Make FakeConfiguration a representative configuration object.#219
Make FakeConfiguration a representative configuration object.#219jonasbardino merged 1 commit intonextfrom
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. |
jonasbardino
left a comment
There was a problem hiding this comment.
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
jonasbardino
left a comment
There was a problem hiding this comment.
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.
00f8989 to
84d8031
Compare
|
This PR in its current form ran for some weeks without issue on dev-erda (appears that I had deployed it a while back and seemingly forgotten about it) and has been running fine. As came up previously in comments, there had been no intent to change behaviour and I believe the adjustments post the last round of review and correct operation have created some assurance there. Some conflict appears to have arisen, so I will be rebasing this and then push to have it folded in. |
84d8031 to
1f56070
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.
1f56070 to
bcee161
Compare
I rebased to latest and had another quick look not spotting any obvious issues. So assuming that you've thoroughly tested on the dev site I'll proceed and merge to get it in before next release (hopefully later this month). |
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.