Skip to content

Flake batch 38: Windows base-reclaim and call-graph read-bound tests - #1346

Merged
ualtinok merged 8 commits into
mainfrom
land/flake-batch-38
Nov 3, 2026
Merged

ualtinok merged 8 commits into
mainfrom
land/flake-batch-38

Conversation

@aft-alfonso

@aft-alfonso aft-alfonso Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Two test-only fixes for Windows failures. Both turned main red or failed unrelated PRs' runs. No production behaviour changes.

1. View base reclaim test (storage retention)
retention_reclaims_unreferenced_bases_even_when_accounting_is_owed assumed an unused commit base is reclaimed on exactly the second cleanup pass. Reference reclaim runs before the byte-accounting walk and its progress persists across passes. But a pass that spends its whole 250 ms budget opening the registry does no reference sweep, so reclaim lands one pass later. A one-time, test-only registry-setup delay reproduces the CI failure natively on Windows. On main it failed 1 of 19 Windows runs, and never on Linux or macOS. The test now waits up to 16 passes for reclaim. It keeps the 250 ms budget and the 500 ms accounting delay, and still checks after every pass that held and referenced bases are never touched.

2. Call-graph read bound test (#1338)
callgraph_views_rematerialization_bounds_zoom_then_serves_publication asserted end-to-end zoom time under 3 s, with no headroom for runner overhead. On GitHub's Windows runner it saw 4.05 s. Natively on Windows, the bounded waits measured 3.006 s alone and in parallel, and the extra second didn't reproduce. The test now asserts the wait contract that is actually promised: the unchanged absolute deadline, the requested query-lock and receive timeouts, and both timeout outcomes while materialization is held. A new case delays the reply after the timeout to prove reply rendering isn't counted. End-to-end timings stay as diagnostics. The 3 s production bound and the replies are unchanged. The timing hooks are #[cfg(test)] only.

Gate: 264 call-graph view, configure and storage-retention tests on Linux, formatting and the Windows check. Both fixes passed natively on Windows in their workers' runs.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes two test flakes that failed Windows CI runs. No production behaviour changes.

Storage retention regression

  • The base-reclaim test assumed an unused commit base is reclaimed on exactly the second collector visit, but a slow registry open can exhaust the 250 ms visit budget before the reference sweep starts, pushing reclaim a pass later.
  • The test now retries up to 16 collector visits, injects a one-time slow registry open to reproduce the failure, and still checks held and referenced bases after every pass.

Call-graph read bound test

  • The rematerialization bound test asserted end-to-end zoom time under 3 s, which overran on GitHub's Windows runner.
  • It now asserts the wait contract the code promises: unchanged deadlines, per-stage remaining budgets, and timeout outcomes while materialization is held.
  • A new case delays reply delivery 1250 ms past the timeout to prove reply rendering time isn't counted against the bound.
  • End-to-end timings remain as diagnostics; all timing hooks are #[cfg(test)] only and skip when a test context has no cache root, and the acquisition-token test now allows 30 s for positive waits on loaded runners.

Written for commit 05e54d2. Summary will update on new commits.

View guided diff Turn on auto-fix

Follow-ups after the first CI run:

  • Zoom timing hooks: the hooks called canonical_cache_root(), which panics in test contexts that never ran configure, so about fifteen zoom tests failed on all platforms. They now use canonical_cache_root_opt() and skip when there is no root. Confirmed natively on Windows (panic at context.rs:9892 before the fix), and 95 zoom and read-bound tests pass on Linux after it.
  • cancel_reaches_completed_and_running_acquisition_tokens (Views: call-graph reads answer within seconds instead of waiting for a full rebuild #1338): it waited only 2 s for each acquisition token, which a loaded Windows runner missed. Those positive waits are now 30 s. The 250 ms cancellation check is unchanged.

ualtinok and others added 8 commits November 3, 2026 05:27
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
The regression used to require two collector visits to reclaim a base, but lock acquisition and registry setup share the visit budget and can defer a reference sweep entirely. Keep the 250 ms budget and forced owed accounting; retry at most 16 visits, retaining pin/reference checks on every visit and verifying the directory is removed. Inject one slow registry open so the test covers a deferred reference sweep instead of relying on a fast runner.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Record test-only pin, zoom observation, context selection/gap/accounting, query-lock, submission, receiver-budget and completion phases. Keep the existing end-to-end assertion unchanged to diagnose the reported overrun before choosing a fix.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Native Windows solo and parallel configure-module instrumentation did not reproduce the historical extra second: observed reader windows stayed about 3004-3007 ms, with millisecond parsing/rendering overhead. Preserve end-to-end timing as diagnostics, but prove the actual contract through unchanged absolute deadlines, remaining query/receive budgets, and timeout outcomes while the worker stays gated.

Use a cleanup-only watchdog, not a larger production timeout. Add a controlled 1250 ms post-timeout delivery gate: replies deliberately exceed the former wall-clock envelope while wait arguments and Building responses remain correct. All observers and delay controls are test-only.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…s on slow runners

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
@ualtinok
ualtinok added this pull request to the merge queue Nov 3, 2026
@ualtinok
ualtinok removed this pull request from the merge queue due to a manual request Nov 3, 2026
@ualtinok
ualtinok added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 2831b3b Nov 3, 2026
93 of 94 checks passed
@ualtinok
ualtinok deleted the land/flake-batch-38 branch November 3, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant