Skip to content

Make FakeConfiguration a representative configuration object.#219

Merged
jonasbardino merged 1 commit intonextfrom
test/enhance-fake-configuration
Jan 12, 2026
Merged

Make FakeConfiguration a representative configuration object.#219
jonasbardino merged 1 commit intonextfrom
test/enhance-fake-configuration

Conversation

@albu-diku
Copy link
Copy Markdown
Contributor

@albu-diku albu-diku commented Apr 4, 2025

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.

@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch from 40c209e to b451cff Compare April 9, 2025 15:44
@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch from b451cff to 219c4b4 Compare July 23, 2025 14:09
@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch 4 times, most recently from 8819c98 to 8bb1fc6 Compare September 2, 2025 14:51
@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch from 8bb1fc6 to 403e09e Compare September 3, 2025 10:03
@jonasbardino jonasbardino requested review from a team and removed request for jonasbardino September 3, 2025 15:26
@jonasbardino
Copy link
Copy Markdown
Contributor

Reassigned the review to all ops to avoid unnecessary bottlenecks.

Comment thread mig/shared/configuration.py
Comment thread mig/shared/configuration.py Outdated
Comment thread mig/shared/configuration.py Outdated
Comment thread tests/support/configsupp.py Outdated
Comment thread tests/support/configsupp.py
Comment thread tests/support/configsupp.py Outdated
Comment thread tests/test_mig_shared_configuration.py
Comment thread tests/test_tests_support_configsupp.py Outdated
Comment thread tests/test_tests_support_configsupp.py Outdated
Copy link
Copy Markdown
Contributor

@jonasbardino jonasbardino left a 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.

@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch 6 times, most recently from e44d190 to f7e4c8d Compare October 17, 2025 13:24
Comment thread mig/shared/configuration.py Outdated
Comment thread mig/shared/configuration.py Outdated
Comment thread tests/support/configsupp.py Outdated
Copy link
Copy Markdown
Contributor

@jonasbardino jonasbardino left a 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.

@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch 3 times, most recently from 00f8989 to 84d8031 Compare October 20, 2025 13:38
@albu-diku
Copy link
Copy Markdown
Contributor Author

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.

@albu-diku albu-diku force-pushed the test/enhance-fake-configuration branch from 84d8031 to 1f56070 Compare December 11, 2025 12:06
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.
@jonasbardino jonasbardino force-pushed the test/enhance-fake-configuration branch from 1f56070 to bcee161 Compare January 12, 2026 14:16
@jonasbardino
Copy link
Copy Markdown
Contributor

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.

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).

@jonasbardino jonasbardino merged commit 2596267 into next Jan 12, 2026
10 checks passed
@jonasbardino jonasbardino deleted the test/enhance-fake-configuration branch January 12, 2026 14:46
@jonasbardino jonasbardino added enhancement New feature or request test-only Improvements or additions solely for better test coverage - without functionality changes labels Jan 12, 2026
@jonasbardino jonasbardino removed their assignment Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test-only Improvements or additions solely for better test coverage - without functionality changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants