-
Notifications
You must be signed in to change notification settings - Fork 4
Make FakeConfiguration a representative configuration object. #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
40c209e to
b451cff
Compare
b451cff to
219c4b4
Compare
8819c98 to
8bb1fc6
Compare
8bb1fc6 to
403e09e
Compare
|
Reassigned the review to all ops to avoid unnecessary bottlenecks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but there are some mostly smaller issues as outlined in comments. I think the most important thing here is that it needs thorough testing on at one of our test sites to make sure configuration.py changes don't break user pages or services.
e44d190 to
f7e4c8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work 👍 we're getting really close and after the planned deployment testing I'm sure we can merge it.
I added a couple of comments you can address or comment on at your leisure.
ae9cae2 to
00f8989
Compare
As of this commit fake configuration instances are populated with the same properties, including the same default values, as a genuine Configuration object instance. This ensures that logic under test is going to behave far more as it would with a real configuration object which means a great deal more assurance in the tests. Achieve this by using the dictionary of defaults that were split out and made statically defined some time ago. FakeConfiguration becomes a SimpleNamespace thereby ensuring both that normal attribute lookup works correctly but also disallows unknown properties being attached to configuration arbitrarily. This keeps us honest in the properties we expose and prevents accidental additions. Since FakeConfiguration needs to track the real Configuration the only permissible keys are those of a genuine object. Unfortunately it seems that beyond the specified defaults many properties are set dynamically when loading a configuration. Lay the first steps for these objects becoming regular by making a couple of properties used by existing tests static; either existing defaults are re-used to avoid functional change or properties are explicitly defined with auto load-time behaviour. Of particular note is the use of keyword_auto for new_user_default_ui. Finally, use the opportunity to improve the integration of the various fake objects into test cases. The provided fake configuration will now be passed the fake logger attached provided by the MiG testcase, which means that it will be included in the logger message detection logic.
00f8989 to
84d8031
Compare
As of this commit fake configuration instances are populated with
the same properties, including the same default values, as a genuine
Configuration object instance. This ensures that logic under test is
going to behave far more as it would with a real configuration object
which means a great deal more assurance in the tests.
Achieve this by using the dictionary of defaults that were split out
and made statically defined some time ago. FakeConfiguration becomes
a SimpleNamespace thereby ensuring both that normal attribute lookup
works correctly but also disallows unknown properties being attached
to configuration arbitrarily. This keeps us honest in the properties
we expose and prevents accidental additions.
Since FakeConfiguration needs to track the real Configuration the only
permissible keys are those of a genuine object. Unfortunately it seems
that beyond the specified defaults many properties are set dynamically
when loading a configuration. Lay the first steps for these objects
becoming regular by making a couple of properties used by existing
tests static; either existing defaults are re-used to avoid functional
change or properties are explicitly defined with auto load-time behaviour.
Of particular note is the use of keyword_auto for new_user_default_ui.
Finally, use the opportunity to improve the integration of the various
fake objects into test cases. The provided fake configuration will now
be passed the fake logger attached provided by the MiG testcase, which
means that it will be included in the logger message detection logic.