Repository navigation
fix(tests): await parallel cache consumers before assertions - #3352
Daniel Meppiel (danielmeppiel) merged 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 067b35fc-52fb-4db4-b316-c606d23b4b34
There was a problem hiding this comment.
🟢 Approval recommended
The bounded test correction waits for both workers without weakening concurrency or content checks, and the documentation matches the change.
No open issues 🎉
What changed in this PR
This PR fixes a shared-cache regression test that could inspect results before both Git consumers had finished, without changing production cache behavior.
Changes:
- Replace timed thread joins with executor-managed completion while preserving parallel execution and existing assertions.
- Select the test in the existing Windows compatibility gate.
- Document worker-completion requirements for concurrent fixtures.
| File | Description |
|---|---|
tests/unit/deps/test_shared_clone_cache.py |
Await both consumers and add the windows_compat marker. |
docs/src/content/docs/contributing/integration-testing.md |
Explain when concurrent fixtures must await workers. |
Review effort: Balanced
Docs sync advisoryVerdict: no_change * Pages affected: 0 * LLM calls: 0/15 * Took: not timed (deterministic path gate) Both changed paths match the canonical test/docs no-impact gate; no user-facing CLI, policy, dependency format, primitive or TOC surface changed. The contributor guide already explains worker completion before result inspection and cleanup. This is a direct L0 path classification, not a panel review or approval. Generated by docs-sync. This comment is AI-generated and may contain errors. Merge Master assessment: ship_nowQualified exact head All 50 artifact seals, the full completion schema and strict terminal owner-v2 verification passed; zero application owners are touched. Actual source-session requests and terminal receipts match the retained evidence. The unchanged delayed-worker control fails, the identical control on this head passes, missing X still fails the original content assertion, and the restored focused module passes all 45 cases. Canonical lint/static evidence is exact-head and terminal. Both real Git consumers, their barrier and all substantive assertions remain intact. Own-head CI and merge gate are successful, including explicit Windows execution of the repaired node. All 28 currently observed checks are terminal: 25 success, two neutral and one intended deployment skip. The added entries since the source's 26-check snapshot are metadata-only eligibility publications. Actual CodeQL analyses succeeded; the imposed neutral/missing native CodeQL comparison remains an explicit exception, not passed analysis. The original failing run and both failed evidence guards remain preserved. The author released all #3352 write/process surfaces. Fresh identity, unique carrier, empty closing references, body, live reviews, current-main compatibility and admin bypass capability were verified. This bounded issue-less CI repair closes no issue. The wider CI mission remains open. Full-run Generated by autopilot-pr-merge-worker. This comment is AI-generated and may contain errors. |
fix(tests): await parallel cache consumers before assertions
Description
TL;DR
Wait for both real Git consumers to finish before the shared-cache regression test inspects their results. The previous timed joins could return with a consumer still running and report a correctly materialized package as missing. Keep both consumers, their barrier, real Git operations and all content/error assertions; also select this case in the existing Windows compatibility gate.
Important
This is a test-lifetime correction, not a production-cache concurrency fix or a timeout increase. The failed main-push run stays failed; a separate successful execution does not erase it.
Problem (WHY)
AssertionError: assert 'X' in ['Y']. The unit step finished with1 failed, 24835 passed, 624 skipped, 21 xfailed.join(timeout=10)returned whilethread_awas alive. The worker subsequently completed; no production-cache failure was needed to reproduce the false result.The investigation follows Agent Skills' "Read agent execution traces, not just final outputs.": preserve the failed run, inspect the real boundary, and distinguish an incomplete observation from missing content.
Approach (WHAT)
windows_compatto this case only; do not broaden the entire module or edit CI selection.Implementation (HOW)
tests/unit/deps/test_shared_clone_cache.pydocs/src/content/docs/contributing/integration-testing.mdNo production code, credentials, cache keys, package references, workflow caps, marker expressions, security controls, dependencies or release behavior change. No CHANGELOG entry is added for this internal test correction.
Diagrams
The highlighted region owns both worker lifetimes; the real parallel materialization and final assertions remain unchanged.
sequenceDiagram participant T as Cache regression test participant E as ThreadPoolExecutor participant A as thread_a participant B as thread_b rect rgb(255, 247, 200) Note over T,E: NEW: own both worker lifetimes T->>E: Submit both consumers E-->>T: Two futures T->>E: Collect both future results par Existing consumer A E->>A: Run thread_a A->>A: Barrier then real Git clone and checkout A-->>E: Return and Existing consumer B E->>B: Run thread_b B->>B: Barrier then real Git clone and checkout B-->>E: Return end E-->>T: Both consumers completed end T->>T: Assert no errors and X plus Y present T->>T: Delete temporary bare cacheTrade-offs
Benefits
Issue and approved scope
Issue: None. This is a separate bounded CI-recovery carrier under the maintainer's direct task-specific exception, not a claim of accepted issue status.
Human scope-approval comment: No GitHub approval comment is claimed. The standing direct instruction is: "you do it, and do not mandate for these fixes to require an issue and approval, it's not needed. get CI working ASAP". The coordinator explicitly returned exclusive ownership of this newly observed failure.
Does this PR complete the issue, or what remains? No issue is closed. Scope is exactly the two files above. The wider main-CI/nonpublishing-validation mission remains separate; delivered PRs and their historical evidence are not reopened or relabelled.
Type of change
Testing
No new product functionality is introduced. The existing regression is repaired and gains Windows-gate coverage. Local claims remain limited to the focused suite and explicit controls below; this carrier's exact-head hosted checks are now terminal.
Validation
Exact candidate:
9f7c0108bd8012662dd087d711558a6291544e10, sole parent actual public mainabd363ff7bfabbc7984e1bad8cdff373f44219fa. Both paths were frozen before qualification; clean-source guards passed before and after every execution.abdcase, delay consumer A for 12 seconds after successful real checkout9f7c010case, identical delayed-checkout control9f7c010case, remove X after successful real checkout9f7c010abd..9f7c0109f7c010863 passed, 2 skipped, 36088 deselectedin 519.19sBoth actual CodeQL analyses succeeded. The imposed-native CodeQL check remains neutral/missing under the existing explicit maintainer exception, not passed; docs deployment was skipped. Independent bounded source review found no in-scope blocker; Copilot's review had no inline findings. Docs-sync returned deterministic L0
no_change, with no panel spawned.Warning
This carrier does not clear the wider mission. The separate actual-
abdfull validation 39784442659 has 10 Linux ARM integration failures and 6 Intel macOS integration failures; Linux x86 integration was cancelled and Release Validation skipped because of failed dependencies. Those are genuine recovery blockers, not allowed publication skips or results for this head.Exact commands and retained output
The session-only diagnostic harness and its source-bound traces are retained with the maintainer's evidence packet; they are not runtime dependencies or files added to this PR.
Delayed control, exit 0 on the candidate (the same control exited 1 on unchanged
abd):Missing-content control, expected exit 1:
Restored module, exit 0:
Each following canonical command passed with exit 0:
The platform-neutral equivalents of CI's three grep/awk guards passed on their exact covered paths. The diagram also passed the installed Mermaid CLI 12.0.0.
Scenario Evidence
tests/unit/deps/test_shared_clone_cache.py::TestBareCacheRaceCondition::test_parallel_different_subdirs_both_succeed(regression trap for the linked failed main job; delayed-completion and missing-content controls above)This is a real-filesystem/local-Git test located under
tests/unit; the table does not claim a user CLI flow or a mocked production-cache proof.How to test
uv run --frozen --extra dev python -m pytest tests/unit/deps/test_shared_clone_cache.py -q; expect all 45 cases to pass.uv run --frozen --extra dev python -m pytest tests/unit/deps/test_shared_clone_cache.py -m windows_compat -q; expect the repaired case to be selected and pass.Spec conformance (OpenAPM v0.1)
If this PR changes behaviour that an OpenAPM v0.1
req-XXXcovers,confirm the three-step ritual in the
development guide:
docs/src/content/docs/specs/openapm-v0.1.mdupdated(new/changed
<a id="req-XXX"></a>anchor + prose + Appendix Crow).
docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.ymlupdated.
@pytest.mark.req("req-XXX")test undertests/spec_conformance/added or extended.CONFORMANCE.{md,json}regenerated viauv run --extra dev python -m tests.spec_conformance.gen_statementand committed.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com