Close the races behind the suite's intermittent failures - #941
Conversation
PR Summary by QodoEliminate intermittent CI races across test suites
AI Description
Diagram
High-Level Assessment
Files changed (31)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Per-test isolation releases Dispatcher.UIThread and reclaims it on every dispatch, so claiming it once before the suite starts closes only the first window and every later dispatch reopens one. One application for the assembly leaves nothing to reclaim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fake assigned it after resuming through a completion source that publishes continuations asynchronously, leaving a test two thread-pool hops behind the wire with only a wall clock to say when to give up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Auto maintenance and auto gc are on by git's defaults, which an empty global config leaves in force: one writes .git/objects/maintenance.lock under a tree a test is comparing, the other detaches and outlives the TempDir holding the repository it ran in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bridge held a settable delegate no production caller filled, so the one behaviour worth pinning — that a taken port is retried rather than fatal — was only reachable by reaching into the instance under test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e0a2190 to
1f322d3
Compare
realtonyyoung
left a comment
realtonyyoung
left a comment
There was a problem hiding this comment.
Static review complete. I left one actionable inline finding. Per request, I did not build or run tests.
A raw string literal keeps whatever indentation precedes its lines, so a tab inside a space-indented one lands as space-before-tab and `git diff --check` reports it. Git config reads spaces the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
NO FINDINGS |
realtonyyoung
left a comment
realtonyyoung
left a comment
There was a problem hiding this comment.
Static re-review complete. The prior finding is addressed. Per request, I did not build or run tests.
2 participants
Closes #918 — AI-2734
Closes #707 — AI-2370
What & why
Two intermittent CI failures and one harness race, each a test losing a race rather than a product fault. The Avalonia one needed more than claiming
Dispatcher.UIThreadonce at assembly start: per-test isolation releases that claim and retakes it on every dispatch, so a single claim closes only the first window. One application for the assembly leaves nothing to reclaim. Git's own auto maintenance ran inside fixture repositories, writing a lock file into a tree a test was comparing and detaching a gc that outlived theTempDirit ran in.The permission bridge's port seam goes with them: a settable delegate no production caller filled, so the behaviour worth pinning — a taken port is retried, not fatal — was reachable only by reaching into the instance under test. It is a constructor argument now, with the real probe behind it.
Where to look
AvaloniaSessionkeeps the assembly-level claim and changes the isolation level under it.HeadlessUnitTestSession.EnsureIsolatedApplicationcallsDispatcher.ResetBeforeUnitTests, which nulls the process-global the claim lives in, and it runs per dispatch while isolation is per-test. The cost is a sharedApplication: nothing in this suite usesAvaloniaLocator, and each dispatch still ends draining its jobs.GitConfigGlobalSetuppinned an empty global config — hermetic, but git's own defaults (gc.auto6700,maintenance.autotrue) stay in force through an empty file.The ACP fake recorded a server-request reply only after resuming through a completion source that publishes continuations asynchronously, leaving the test two thread-pool hops behind the wire with a wall clock as its only synchronisation.
Verification
Three mutations, each failing the test that claims the behaviour: isolation returned to per-test;
gc.autorestored to git's default; the bridge's port source left to the OS where a collision is arranged.Zero warnings across the solution;
dotnet publish -c Releaseclean of IL2026/IL3050 for the daemon.#916 and #917 were fixed by #924 while this branch was open, and its versions are better than what this branch had: a scripted
HttpMessageHandlerneeds no port at all, and spawningping.exedirectly makes the tracked process the real working-directory owner. Both commits were dropped rather than merged.#543 is closed separately: its deterministic failures no longer reproduce on macOS under the default
TMPDIR, with no test skipped to achieve it. Its production question — the Codex config guard rejecting any symlinked ancestor, so a~/.codexsymlinked into a dotfiles repo is refused — is untouched here and wants its own issue.🤖 Generated with Claude Code