1386130 - 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

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=119856742&repo=mozilla-inbound

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win64-debug/mozilla-inbound_win8_64-debug_test-mochitest-e10s-4-bm119-tests1-windows-build475.txt.gz

Comment 1

[Intermittent Failures Robot]

12 failures in 888 pushes (0.014 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 9
* mozilla-inbound: 3

Platform breakdown:
* windows8-64: 10
* windows7-32: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386130&startday=2017-07-31&endday=2017-08-06&tree=all

Comment 2

[Intermittent Failures Robot]

1 failures in 924 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 1

Platform breakdown:
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386130&startday=2017-09-04&endday=2017-09-10&tree=all

Comment 3

[Intermittent Failures Robot]

2 failures in 885 pushes (0.002 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* autoland: 2

Platform breakdown:
* macosx64-stylo-disabled: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386130&startday=2017-09-25&endday=2017-10-01&tree=all

Comment 4

[Brendan Dahl [:bdahl]]

Attachment: Bug 1386130 - Actually wait for up to 30 frames to test scroll.

The frame count test was inverted (should be '>' not '<'), causing
the test to never actually wait up to 30 frames to test the scroll
position. The test will also now explicitly fail if the frame count
is reached to make finding this issue easier in the future.

Review commit: https://reviewboard.mozilla.org/r/186558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186558/

Comment 5

[Brendan Dahl [: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

Comment 6

[Daniel Holbert [:dholbert]]

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.

Comment 7

[Daniel Holbert [:dholbert]]

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.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/547580414195
Tweak mochitest to actually wait for up to 30 frames to test scroll. r=dholbert

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/547580414195

Comment 10

[Intermittent Failures Robot]

1 failures in 824 pushes (0.001 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* autoland: 1

Platform breakdown:
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1386130&startday=2017-10-02&endday=2017-10-08&tree=all