test(file-tree): wait for the star to change, not for the button to exist - #119
Merged
Conversation
…xist
TestFileTreeE2E flaked roughly one run in three, and failed livetemplate#504's
Docs Examples check.
The star step waited on the wrong thing:
chromedp.Click(star button)
chromedp.WaitVisible(li[data-key=...] button[name=star]) <- already visible
chromedp.Evaluate(textContent.includes('star')) <- races the WS round trip
That button is on screen before the click; only its text changes. So the wait
was satisfied instantly and the assertion read the pre-update DOM. The diagnostic
dump on the CI failure showed the star HAD applied by the time it ran, which is
what pointed at a read race rather than a broken update.
Now waits on the text actually changing, via e2etest.WaitFor.
Only this step was affected: every other wait in the test targets an element the
update creates (li[data-key=/internal/store] and friends), which genuinely does
not exist until the update lands.
Verified 8 consecutive passes, against roughly 1-in-3 failures before.
Confirmed not a livetemplate regression before changing the test: with the real
file-tree template and the real toggle-toggle-toggle-star sequence, the built
tree JSON is byte-identical between livetemplate#504 and its parent, so the
wrapper-tagging change does not affect anything this test exercises.
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
1 participant
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.
TestFileTreeE2Eflaked roughly one run in three, and failed the Docs Examples check on livetemplate#504.The defect
That button is on screen before the click — only its text changes. So the wait was satisfied instantly and the assertion read the pre-update DOM.
The tell was in the CI diagnostic dump: it showed
value="/internal/store/sql/query.go">★— the star had applied by the time diagnostics ran. A broken update would have shown☆. That's a read race, not a failed update.Now waits on the text actually changing, via
e2etest.WaitFor.Only this step was affected. Every other wait targets an element the update creates (
li[data-key="/internal/store"]and friends), which genuinely doesn't exist until the update lands. That asymmetry is why the flake was isolated to starring.Verification
8 consecutive passes, against ~1-in-3 failures before.
Confirmed not a core regression first
livetemplate#504 changes range keying, so a keying regression was the obvious suspect and I checked that before touching the test. With the real
file-treetemplate and the real toggle → toggle → toggle → star sequence, the built tree JSON is byte-identical between #504 and its parent — that change doesn't affect anything this test exercises.Worth noting the test I wrote had the same failure mode I keep flagging in reviews: a gate that doesn't gate. It passed for the right reason most of the time and the wrong reason the rest.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG