Repository navigation
test(engine): run the non-PNG probe test on a fake clock - #6907
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Approving at 12b3c6b9. I found no blockers.
- Root cause matches.
probeBeginFrameSupportreadsDate.now()against a 120 ms deadline. Under a real stall,awaitBeforeDeadlineseesremainingMs <= 0and throwsBeginFrameProbeTimeout, so the result istimedOut=true. That timeout path wins over the loop's normal non-PNG exit, which throws a plainError. - The fix works. Vitest 4 fake timers mock
Dateas well assetTimeout, so the deadline only moves when the test advances the clock. Advancing 200 ms (more than the 120 ms deadline) lets the loop hit itsDate.now() + 10 >= deadlinebreak and end with the 8-byte non-PNGError.pendingsettles becausepage.close()resolves at once, and thefinallyputs real timers back. - Fails without the fix (checked). I made the mocked
sendstall for 150 ms on every call:- Old test:
expected true to be false, which matches the line in the PR body. - New test: passes. I used
Atomics.wait, because a busy-wait onDate.now/hrtimespins forever under fake timers.
- Old test:
- Suite still green. The unchanged test file has 117/117 passing. The
BeginFrame capability probeblock passed 3/3 runs on Linux.
Not blocking: the other probe tests in this block still use real short deadlines. They look safe from stalls: the "empty screenshot" test expects timedOut=true, and a stall only pushes it that way. If one of them flakes later, it can get the same fake-clock change.
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.
What
The test "reports frames that are not a PNG as unsupported, not as a slow start" ran the probe against a real 120 ms deadline. On a loaded Windows runner the event loop stalled past it, the probe reported a slow start (
timedOut=true) and the merge queue dropped an unrelated PR.It now runs on fake timers and advances the clock by hand, so the result no longer depends on how busy the machine is. No production code changes.
fails without the fix: browserManager.test.ts › "reports frames that are not a PNG as unsupported, not as a slow start" (with the old test, a 150 ms stall in the mocked CDP call gives
expected true to be false)Tests
That test passes 3 runs in a row on Linux.
Size
Under 100 changed lines.