fix(tests): stop clear_fault_emits_fault_cleared_event racing its siblings - #135
Merged
Conversation
…lings The event sender is global and the test binary runs tests in parallel, so the drain loop sees FaultCleared events from any test that clears a fault — and eleven of them do. The loop's Ok(_) arm skips other event kinds but not other tests' FaultCleared, so whichever arrives first is asserted against this test's id, comparing two unrelated uuids. Guard the arm on the id so the loop skips what its comment always said it skipped. Observed failing in CI; the same code passes on other branches, which is the flake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgdfXBAnSEj5s24gbJyXHi
passcod
pushed a commit
that referenced
this pull request
Aug 1, 2026
Split into #135 against main. It is unrelated to the Windows runtime and fixes a flake that can hit any branch, so it should not wait on this one. The --nocapture change that shared its commit stays: it belongs here, since it is what makes the S6 spike tests' recorded observations reach the log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgdfXBAnSEj5s24gbJyXHi
Contributor
Code Coverage OverviewLanguages: TypeScript, Rust TypeScript / code-coverage/vitestThe overall coverage in commit 01d37b6 in the Rust / code-coverage/rustThe overall coverage in commit 01d37b6 in the Updated |
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky runtime faults test caused by parallel test execution sharing a global event broadcaster, where unrelated FaultCleared events from sibling tests could be incorrectly asserted against this test’s fault id.
Changes:
- Make
clear_fault_emits_fault_cleared_eventonly matchFaultClearedevents whoseidequals the fault id created by the test (via a match guard). - Expand the in-test comment to accurately document why the id guard is necessary given parallel execution and a global sender.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
passcod
marked this pull request as ready for review
August 1, 2026 21:59
Merged
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.
Split out of #107 so it can merge on its own — it is unrelated to that PR's subject and fixes a flake that can hit any branch.
runtime::faults::tests::clear_fault_emits_fault_cleared_eventintermittently fails comparing two unrelated fault ids:The event sender is global and the test binary runs tests in parallel. The drain loop's comment says it skips "any interleaved events from other parallel tests", but the
Ok(_) => continuearm only skips other event kinds — another test'sFaultClearedmatches the first arm and is asserted against this test's id. Eleven tests in that file clear faults, so the collision is routine rather than theoretical.Guarding the arm on the id makes the loop skip what it always claimed to skip; a non-matching
FaultClearednow falls through tocontinue.Observed failing on a branch whose changes go nowhere near faults, while the identical code passed on a sibling branch in the same window — which is the flake.
🤖 Generated with Claude Code
https://claude.ai/code/session_01CgdfXBAnSEj5s24gbJyXHi
Generated by Claude Code