Closed Bug 1386130 Opened 3 years ago Closed 2 years ago

Intermittent layout/base/tests/test_scroll_snapping_scrollbars.html | Step 5: Synthesized mouse events move scroll position. (Page scrollbar right, expect scroll snapping.) - didn't expect "(0,500)", but got it

Categories

(Core :: Layout, defect, P5)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: bdahl)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Assignee: nobody → bdahl
This reproduced relatively consistently with headless mochitests. We should probably also visit all the bugs tied to this intermittent failure and all the tests that were disabled because of it too.

Bugs that are probably a dupe: bug 1386130, bug 1379810, and bug 1381932
Blocks: 1399956
Comment on attachment 8915364 [details]
Bug 1386130 - Actually wait for up to 30 frames to test scroll.

https://reviewboard.mozilla.org/r/186558/#review191630

::: layout/base/tests/test_scroll_snapping_scrollbars.html:267
(Diff revision 1)
> -      || testCase.startScroll.x != sc.scrollLeft
> -      || ++stopFrameCount < 30) {
> +    ok(false,
> +      "Step " + step + ": Frame count limit reached without scrolling." +
> +      "(" + testCase.description + ")");

In the failure message here, please add a space after the period in "scrolling.", or equivalently before the open-paren.

Otherwise it looks like this would print out a message "Step N: ...without scrolling.(stuff)" -- with the period and open-paren directly adjacent.
Attachment #8915364 - Flags: review?(dholbert) → review+
Comment on attachment 8915364 [details]
Bug 1386130 - Actually wait for up to 30 frames to test scroll.

https://reviewboard.mozilla.org/r/186558/#review191632

::: commit-message-ee1c4:1
(Diff revision 1)
> +Bug 1386130 - Actually wait for up to 30 frames to test scroll. r?dholbert

Also, please adjust commit message to mention that this is a mochitest -- e.g.:
  Bug 1386130 - Tweak mochitest to actually wait [etc]

That makes it more obvious that this is a test-only commit, which can be skipped over when perusing regression pushlogs looking for the cause of a regression, for example.

(The mention of "...to test scroll" does kinda hint at this already, but it's non-obvious whether it's talking about a testcase vs. some hypothetical "if (scrolling)" check in our Firefox code somewhere.)
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/547580414195
Tweak mochitest to actually wait for up to 30 frames to test scroll. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/547580414195
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.