Wait on a condition in every scenario that reads after one - #209
Merged
Merged
Conversation
#190 swept six sleeping assertions and its grep was wrong: a one-line lookahead demanding a literal 'expect(' on the very next line, which misses a value read now and asserted two lines down, and anything behind a blank line. Counted properly there were 42. They are replaced with 'eventually(read, matches, what)', file by file, each with the condition that actually applies. Three are kept and say why: an assertion that *nothing* happened is not a condition anything can wait on. The four in '15-preferences-on-disk' now poll the file from Node instead of outwaiting the store plugin's debounce. 'scripts/e2e-waits.test.mjs' holds the rule with a lookahead that works, so the sweep cannot quietly come undone. Two of the rewrites taught something the sweep alone would not have: submitting a dialog and waiting only on the stored value returns sooner than the sleep did, before the dialog has closed and the next scenario opens a panel over it; and a wait on an end state is worth splitting into the steps that lead there, so a timeout names which one failed. Closes #197
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.
Closes #197.
#190 swept six sleeping assertions and its grep was wrong — a one-line lookahead
demanding a literal
expect(on the very next line, which misses a value read nowand asserted two lines down, and anything behind a blank line. Counted properly, 42 of
the 46
browser.pausecalls ine2e/specs/were followed within four lines by a reador an assertion.
What changed
browser.pausein the specsEach one was replaced with
eventually(read, matches, what)and the condition thatactually applies, file by file — a wrong condition is a test that passes for the wrong
reason.
The three kept say why they are kept: an assertion that nothing happened is not a
condition anything can wait on. Two already carried that comment; the third is the tag
manager's cancel.
15-preferences-on-diskno longer outwaits the store plugin's debounce. The fileis the condition, so it is polled from Node — which also drops four fixed 1.5 s waits
to whatever the flush actually costs.
The rule is now executable
scripts/e2e-waits.test.mjs(npm run test:scripts) fails on abrowser.pausefollowed within four lines by a read or an assertion, unless a comment above says the
wait is
deliberatelyone. Confirmed to fail on an injected violation, so the sweepcannot quietly come undone the way #190's did.
Two rewrites taught something
old sleep, before the dialog has closed — and the next scenario opened a settings panel
over it. A duration hides more than the thing being asserted.
and through the DOM. Split into "written at all" then "filed", it passes — and a
timeout now names which step failed.
Verified
E2E 21/21 twice in a row, which is the point of the ticket rather than a formality.
Unit 1354/1354,
test:scripts34/34, lint clean.CLAUDE.mdupdated: the rule wasalready written there, but it said two exceptions and named two script-test files.