Fix flaky global Hydra state leaking between tests (#574)#876
Open
martinez-hub wants to merge 1 commit into
Open
Fix flaky global Hydra state leaking between tests (#574)#876martinez-hub wants to merge 1 commit into
martinez-hub wants to merge 1 commit into
Conversation
The `clean_store` test fixtures (in tests/conftest.py and tests/test_store.py) snapshot and restore Hydra's global ConfigStore. Hydra registers its plugin-provided configs (e.g. `hydra/sweeper/basic`) into that store lazily, upon first plugin discovery. When a fixture snapshots the store *before* discovery occurs and then restores that snapshot on teardown, it strips those plugin configs from the global store. Any later test that composes the Hydra config (e.g. with `return_hydra_config=True`) then fails with `MissingConfigException: Could not find 'hydra/sweeper/basic'`. This is order-dependent, which is why it surfaced as flaky failures that correlated with the Python version (test-collection order differs across versions). Force Hydra plugin discovery before snapshotting in both fixtures so the snapshot includes these configs and the restore preserves them. Adds a deterministic regression test that reproduces the triggering order in an isolated pytest subprocess against the real `clean_store` fixture. Closes mit-ll-responsible-ai#574
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #574 — the intermittent
MissingConfigException: In 'hydra/config': Could not find 'hydra/sweeper/basic'failures intests/test_launch/test_validation.pythat surface as flaky, Python-version-correlated CI failures.Root cause
The
clean_storefixtures (intests/conftest.pyandtests/test_store.py) snapshot and restore Hydra's globalConfigStore. Hydra registers its plugin-provided configs (e.g.hydra/sweeper/basic) into that store lazily, upon first plugin discovery (triggered by the firstcompose/initialize).If a fixture snapshots the store before discovery has occurred and then restores that snapshot on teardown, it strips those plugin configs from the global store — permanently, since
Pluginscaches discovery and won't re-register them. Any later test that composes the Hydra config (e.g. withreturn_hydra_config=True) then fails to findhydra/sweeper/basic.This is order-dependent, which is exactly why it manifested as flaky failures that correlated with the Python version (the set/order of collected tests differs across versions — e.g.
*py310*tests are excluded on 3.9).This can be reproduced deterministically:
Fix
Force Hydra plugin discovery (
Plugins.instance().discover()) before snapshotting in bothclean_storefixtures, so the snapshot includes the plugin configs and the restore preserves them.This is a test-infrastructure fix — there is no user-facing change, so no changelog entry is added. (Note: the approach floated earlier in #574 of clearing
Singleton._instancesinsidelaunch/hydra_mainwas intentionally avoided — Hydra'sConfigStoreis itself stored inSingleton._instances, so clearing it would wipe user-registered configs, and it does not address the actual mechanism above.)Regression test
tests/test_launch/test_global_state_isolation.pyreproduces the triggering order deterministically in an isolatedpytestersubprocess, using the realclean_storefixture: the first test snapshots viaclean_storeand then triggers discovery; the second test then requireshydra/sweeper/basic. Verified to fail without this fix and pass with it.Verification
ruff format --checkandruff checkclean on all changed files.