test: make the reset-during-processing case actually test its race - #14
Merged
Merged
Conversation
The flaky test turned out to be the smaller of two problems. Removing every runRef guard from App.tsx left it green: dirty.jpg and clean.jpg are both JPEGs and shared one delay, so the abandoned run finished before the newer one and could not have overwritten it whatever the product did. The test carried the name of a race without ever arranging one. The delay now keys on which call it is rather than on the format, so the abandoned run finishes last, and the same mutation turns it red along with its three siblings. All four cases in describe B asserted at a fixed point on the wall clock, 900 ms after the second upload. On an idle machine that left about 400 ms of slack. They now wait for the runs themselves to settle, which is stabler and also stricter: a wall-clock wait that expires early does not fail, it quietly stops testing the race. What actually flaked was a line that asserted nothing - a click on the drop zone commented as a no-op, which is where the failure landed under load. It is gone rather than made tolerant. The micro-race behind it is not fully explained; a probe immediately before it still found the button. Since the line carried no assertion and the scenario reads the same without it, the surface was removed instead of the cause pursued further. No product code. Verified by mutation rather than by hope: with the runRef guards removed, three of the four cases failed before this change and all four fail after it. Under the load that reproduced the flake - CPU pressure plus a running preview server - the suite went from two failures in three runs to five green runs out of five. Closes #11. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #11. Test-only:
git diff -- src/is empty.1. The named test never produced the race it claims
This is the finding, and it is larger than the flakiness that started the investigation.
Removing every
runRefguard fromApp.tsxleft the test green:describe Bdirty.jpgandclean.jpgare both JPEGs, so a delay keyed on the format slowed them equally.The abandoned run therefore finished before the newer one and could not have overwritten it
whatever the product did. The test carried the name of a race without ever arranging one.
The delay now keys on which call it is rather than on the format, so the abandoned run finishes
last — the ordering the other three cases already arranged for themselves.
2. Completion-based synchronisation instead of a 900 ms wall clock
All four cases asserted at a fixed point 900 ms after the second upload. Measured on an idle
machine, that left roughly 400 ms of slack. They now wait for the runs themselves to settle.
That is stabler, and it is also stricter: a wall-clock wait that expires early does not
fail, it quietly stops testing the race. The old form could pass without ever exercising what
it was written for — which is exactly what case 2 turned out to be doing.
3. Validated by mutation, not by hope
With the
runRefguards removed fromApp.tsx:And under the load that reproduced the flake — CPU pressure plus a running
vite previewserver:
What actually flaked
A line that asserted nothing: a click on the drop zone, commented in the source as a no-op. It
opened nothing under jsdom and was where the failure landed. It has been removed rather than
made tolerant.
Stated plainly because it is a loose end: the micro-race behind that click is not fully
explained. A probe immediately before it still found the button present. Since the line
carried no assertion and the scenario reads the same without it, the surface was removed
instead of the cause pursued further. If a
getByRolein a busy state ever flakes again, thatis the thread to pull.
Two hypotheses were tested and dropped along the way: CPU starvation of the 500 ms timer (24
busy processes on 4 cores moved the slack from 402 ms to 420 ms — no effect), and a real race
in the shipped UI (the guard holds; it was verified in Chromium during #8, where a second file
dropped 150 ms into a slow run took over and the abandoned result never appeared).
Scope
Beyond the single test named in #11. All four cases in
describe Bshare the same wall-clockconstruction, and leaving one stabilised case beside three identical ones would be the same
mistake as leaving a stale paragraph beside a corrected one. Say the word and it trims to one.
Gates
tsc --noEmitvite buildnpm audit --audit-level=highgit diff --checkgit diff -- src/describe Balso runs faster: no case waits out a fixed 900 ms block any more.No tag, release or deploy proposed.
🤖 Generated with Claude Code