Repository navigation
Fix the causes of intermittent test failures - #43
Merged
Merged
Conversation
StartAsync_ConcurrentOnSameBrowser_SerializesViaGate queued the answer for the second Tracing.start only after awaiting the first StopAsync. The stop releases the browser-wide gate in its finally block, and the second StartAsync resumes from that release on a thread-pool thread and sends Tracing.start straight away. When it won that race the fake socket had nothing queued, the command went unanswered, and the test waited out the transport's 60 second command timeout before failing with a TaskCanceledException, which is what the ubuntu leg of main hit. Both answers are now queued before the stop. The gate itself keeps them in order, since nothing can send Tracing.start until Tracing.end has been answered, and the test now also checks that order on the wire. Signed-off-by: Torrey Betts <torrey.betts@gmail.com>
Firefox_ScrollIntoViewScript_ScrollsOnlyWhenNeeded read window.scrollY straight after the script had centered the button, then asserted it was unchanged after a second call. Centering puts the button's middle at half the viewport height, so whenever that height is odd the scroll lands on a half pixel (4677.5 locally with a 683px viewport; 4678.5 on the runner, which read 4679 and then 4678). Firefox may round that half pixel some time after the scroll, so the position moved by a pixel with nothing scrolling it. The baseline is now a whole-pixel position the test scrolls to itself, with the button in view but off center. That removes the half pixel, and it also lets the assertion catch a script that scrolls when it should not: re-centering an element that is already centered never moved the page, so the old check could not have seen it. Signed-off-by: Torrey Betts <torrey.betts@gmail.com>
Signed-off-by: Torrey Betts <torrey.betts@gmail.com>
The probe showed that on the Windows runner the default proxy answers loopback lookups at once and a cold test host's first request to a local HttpListener takes tens of milliseconds, so neither explains the intermittent connect-timeout failure. It has done its job. Signed-off-by: Torrey Betts <torrey.betts@gmail.com>
TorreyBetts
marked this pull request as ready for review
October 24, 2026 21:49
SummarySummary
CoverageMotus - 80.9%
Motus.Abstractions - 87.7%
Motus.Analyzers - 91%
Motus.Cli - 82.2%
Motus.Codegen - 89.8%
Motus.Mcp - 91.9%
Motus.Mcp.Http - 98.6%
Motus.Recorder - 77.8%
Motus.Testing - 75.4%
Motus.Testing.MSTest - 79%
Motus.Testing.MSTest.v4 - 22.8%
Motus.Testing.xUnit - 63.6%
|
Contributor
Author
|
Test runs and the flakes left alone. Verification
Left alone, cause not found
Other failures in the last 60 runs
|
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.
A merge queue ejects a PR whenever a check fails, so a test that fails for reasons of its own costs a whole queue cycle. This fixes the two intermittent failures whose cause could be pinned down. Both were in the tests; no product code changes.
TracingTests.StartAsync_ConcurrentOnSameBrowser_SerializesViaGate queued the fake socket's answer for the second Tracing.start only after awaiting the first StopAsync. The stop releases the browser-wide gate in its finally block, and the second StartAsync resumes from that release on a thread-pool thread and sends Tracing.start at once. When it won that race nothing was queued, the command went unanswered, and the test sat out the transport's 60 second command timeout before failing with a TaskCanceledException, as it did on main's ubuntu leg. Both answers are now queued before the stop. The gate keeps them in order, since nothing can send Tracing.start until Tracing.end has been answered, and the test now also asserts that order on the wire.
Firefox_ScrollIntoViewScript_ScrollsOnlyWhenNeeded read window.scrollY straight after the script had centered a button, then asserted it was unchanged after a second call. Centering puts the button's middle at half the viewport height, so whenever that height is odd the scroll lands on a half pixel (4678.5 on the runner, which read 4679 and then 4678), and Firefox can round that half pixel after the scroll has finished. The baseline is now a whole-pixel position the test scrolls to itself, with the button in view but off center. That removes the half pixel and lets the assertion catch a script that scrolls when it should not: re-centering an element that is already centered never moves the page, so the old check could not have seen one.
The Windows connect-timeout failure and the BrowserAttachIntegrationTests timeouts are left unchanged, because their cause was not found; what was ruled out is in the PR discussion.
Signed-off-by: Torrey Betts torrey.betts@gmail.com