test: stop fixtures leaking config overrides and stop retrying a failed Prefect setup - #10329
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Restore config overrides in test fixtures to prevent state leakage; test-only fix with no production impact.
Re-trigger cubic
fatih-acar
force-pushed
the
fac/fix-test-cache-bus-override-leak
branch
from
August 19, 2026 14:30
1351f0c to
a0587bf
Compare
fatih-acar
marked this pull request as ready for review
August 19, 2026 15:12
ajtmccarty
approved these changes
Aug 19, 2026
gmazoyer
approved these changes
Aug 19, 2026
…ig overrides
`memory_cache` and `bus_simulator` each set a `config.OVERRIDE` field and never
put it back. The `dependency_provider.scope(...)` unwinds on teardown, but
`build_cache()` and `build_message_bus()` consult `config.OVERRIDE` *first*, so
the override outlives the class that installed it and every later resolution in
that xdist worker gets the previous class's throwaway adapter.
For the cache that surfaces as
ResourceNotFoundError: Diff summary for pipeline <uuid> was not found in the cache
in `TestProposedChange::test_run_generators_validate_requested_jobs`. The test
writes the diff summary through a cache built from `config.SETTINGS.cache.driver`
(Redis) and `run_generators` reads it back via `get_cache()`. Once the override
leaks, the write goes to Redis and the read goes to the leftover MemoryCache.
It reads as flaky but it is scheduling: it fails exactly when xdist puts
`test_artifact_regen_e2e.py`, which uses `memory_cache`, on the same worker
earlier in the session. Runs 32238651760 and 32153198842 had both files on gw3
and failed; run 32251034448 had them on gw3 and gw1 and passed.
For the message bus nothing fails today — a stale BusSimulator swallows messages
instead of raising — so it is fixed here before it costs a debugging session.
Save and restore in a `finally`, matching the neighbouring `workflow_local`
fixture and every other override site in the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…class `setup_task_manager_once` recorded only success, so a Prefect test server that came up and then stopped responding was retried by every later test class in that xdist worker. The retry is not cheap. The setup does not fail fast against an unreachable server — it blocks on the API until the pytest timeout fires — so each retry cost the full 300s. In run 32238651760 that turned one broken worker into 46 `Failed: Timeout >300.0s` errors across five test files and pushed the session into its 1800s limit, with the original httpx.ReadTimeout buried under 45 identical copies. Remember the failure alongside the success and re-raise it, chained, on every later call. The worker still fails, but once, in seconds, with the cause attached to the first error rather than the forty-sixth. `except BaseException` is deliberate: the pytest timeout raises `Failed`, which does not derive from `Exception`, and that is exactly the failure worth remembering. The once-per-process state moves onto a `TaskManagerSetup` object that takes the setup callable as a constructor argument, so the tests drive it with recording and failing doubles instead of patching the module — the adapter pattern the testing guidelines ask for. `setup_task_manager_once()` keeps its signature and callers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fatih-acar
force-pushed
the
fac/fix-test-cache-bus-override-leak
branch
from
August 19, 2026 21:31
a0587bf to
d28c010
Compare
fatih-acar
marked this pull request as draft
August 19, 2026 21:32
fatih-acar
marked this pull request as ready for review
August 19, 2026 22:17
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.
Why
Two independent faults in
backend-tests-integration, both found chasing the CI failure onrun 32238651760
(
1 failed, 445 passed, 46 errors). Neither is caused by the PR that run belonged to.1. A leaked cache override — the
1 failedIt is not flaky. It fails exactly when pytest-xdist happens to schedule
backend/tests/integration/proposed_change/test_artifact_regen_e2e.pyonto the same worker,earlier in the session:
test_artifact_regen_e2e.pytest_proposed_change.pymemory_cachesetsconfig.OVERRIDE.cacheto aMemoryCacheand never puts it back. Thedependency_provider.scope(build_cache, ...)unwinds on teardown, butbuild_cache()consultsconfig.OVERRIDE.cachefirst, so the override outlives the class that installed it and everylater
get_cache()in that worker returns the dead MemoryCache.test_run_generators_validate_requested_jobswrites the diff summary through a cache it buildsfrom
config.SETTINGS.cache.driver(Redis) andrun_generatorsreads it back viaget_cache().Once the override leaks, the write goes to Redis and the read goes to the leftover MemoryCache.
bus_simulator, immediately above it, leaksconfig.OVERRIDE.message_busthe same way. Nothingfails from it today — a stale
BusSimulatorsilently swallows messages rather than raising —which is why it is worth closing now. Every other override site in the suite
(
component/api/conftest.py,component/telemetry/test_tasks.py,component/git/test_sync_repository.py,component/api/test_50_internals.py) already saves and restores; these two fixtures were the outliers.2. A retried Prefect setup — the
46 errorsOn gw0 the worker's Prefect testcontainer answered
wait_for_prefectand then stopped responding(
httpx.ReadTimeout, dead events websocket) for the rest of the session. The container fault isinfrastructure. The 46 errors are ours:
setup_task_manager_oncerecorded only success, so everylater test class retried the dead server — and since the setup does not fail fast against an
unreachable server, each retry cost the full 300s pytest timeout.
One broken container became 46
Failed: Timeout >300.0serrors across five test files, pushed thesession into its 1800s limit, and buried the original
httpx.ReadTimeoutunder 45 identical copies.What changed
Three commits, test-only. No production code, so no changelog fragment.
memory_cache/bus_simulator— save the previousconfig.OVERRIDEvalue and restore it ina
finally, matching the neighbouringworkflow_localfixture.setup_task_manager_once— remember the failure alongside the success and re-raise it,chained, on every later call. The worker still fails, but once, in seconds, with the cause
attached to the first error rather than the forty-sixth.
except BaseExceptionis deliberate:the pytest timeout raises
Failed, which does not derive fromException.backend/tests/unit/helpers/test_task_manager.py— guards the once-on-success,no-retry-on-failure and BaseException cases. The once-per-process state moved onto a
TaskManagerSetupobject taking the setup callable as a constructor argument, so these drive itwith recording and failing doubles rather than patching the module.
Verification
The cache leak, reproduced by running the two files in one process in the poisoning order:
1 failed, 6 passed—ResourceNotFoundError: Diff summary for pipeline ... was not found in the cache7 passedThe retry guard: the three new unit tests pass, and dropping the short-circuit from
setup_task_manager_oncefails two of them.🤖 Generated with Claude Code