1429744 - Perma failing layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true

Reporter

Description

•

2 years ago

treeherder

Filed by: apavel [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=155588651&repo=autoland

https://queue.taskcluster.net/v1/task/U6smevWAQ5ybyA8zcff0sw/runs/0/artifacts/public/logs/live_backing.log

[task 2018-01-11T10:54:10.072Z] 10:54:10     INFO - TEST-START | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html
[task 2018-01-11T10:54:10.121Z] 10:54:10     INFO - GECKO(1149) | ++DOMWINDOW == 18 (0x7f74a41ec800) [pid = 1149] [serial = 18] [outer = 0x7f74b0985e20]
[task 2018-01-11T10:54:10.245Z] 10:54:10     INFO - TEST-INFO | started process screentopng
[task 2018-01-11T10:54:10.694Z] 10:54:10     INFO - TEST-INFO | screentopng: exit 0
[task 2018-01-11T10:54:10.696Z] 10:54:10     INFO - Buffered messages logged at 10:54:10
[task 2018-01-11T10:54:10.697Z] 10:54:10     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Stylesheet now has 2 rules. 
[task 2018-01-11T10:54:10.698Z] 10:54:10     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Removed expected number of rules. 
[task 2018-01-11T10:54:10.700Z] 10:54:10     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Added expected number of rules. 
[task 2018-01-11T10:54:10.702Z] 10:54:10     INFO - test2Setup: called
[task 2018-01-11T10:54:10.703Z] 10:54:10     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Stylesheet now has 2 rules. 
[task 2018-01-11T10:54:10.705Z] 10:54:10     INFO - styleFirstAddProcessor: called with event @import url("imported_no_op.css");
[task 2018-01-11T10:54:10.705Z] 10:54:10     INFO - Buffered messages finished
[task 2018-01-11T10:54:10.706Z] 10:54:10     INFO - TEST-UNEXPECTED-FAIL | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true
[task 2018-01-11T10:54:10.707Z] 10:54:10     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:312:5
[task 2018-01-11T10:54:10.707Z] 10:54:10     INFO - test2Setup@chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:92:3
[task 2018-01-11T10:54:10.709Z] 10:54:10     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the import rule. 
[task 2018-01-11T10:54:10.710Z] 10:54:10     INFO - GECKO(1149) | MEMORY STAT | vsize 2012MB | residentFast 285MB | heapAllocated 92MB
[task 2018-01-11T10:54:10.715Z] 10:54:10     INFO - TEST-OK | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | took 227ms
[task 2018-01-11T10:54:10.717Z] 10:54:10     INFO - GECKO(1149) | ++DOMWINDOW == 19 (0x7f74a6ab0400) [pid = 1149] [serial = 19] [outer = 0x7f74b0985e20]
[task 2018-01-11T10:54:10.718Z] 10:54:10     INFO - TEST-START | Shutdown
[task 2018-01-11T10:54:10.719Z] 10:54:10     INFO - Passed:  12
[task 2018-01-11T10:54:10.720Z] 10:54:10     INFO - Failed:  1
[task 2018-01-11T10:54:10.721Z] 10:54:10     INFO - Todo:    0
[task 2018-01-11T10:54:10.722Z] 10:54:10     INFO - Mode:    non-e10s
[task 2018-01-11T10:54:10.724Z] 10:54:10     INFO - Slowest: 1148ms - chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html
[task 2018-01-11T10:54:10.725Z] 10:54:10     INFO - TEST-INFO | Ran 2 Loops
[task 2018-01-11T10:54:10.726Z] 10:54:10     INFO - SimpleTest FINISHED

Comment hidden (Intermittent Failures Robot)

30 failures in 149 pushes (0.201 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* mozilla-central: 16
* autoland: 14

Platform breakdown:
* linux64: 10
* windows7-32: 6
* windows10-64: 6
* osx-10-10: 4
* windows7-32-nightly: 1
* windows10-64-nightly: 1
* macosx64-nightly: 1
* linux64-nightly: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1429744&startday=2018-01-11&endday=2018-01-11&tree=all

Stefan Hindli [:stefan_hindli]

Comment 2

•

2 years ago

In the last 7 days this bug has failed 30 times. The fails happen on Linux x64, MacOSX 64 nightly, OSX 10.10, Windows 7, windows 10-64 and windows 7-32 nightly

Here is an example of the most recent log: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-central&job_id=155585821&lineNumber=1624

Here is a relevant snippet from aforementioned log:

02:38:16     INFO - TEST-UNEXPECTED-FAIL | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true
02:38:16     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:312:5
02:38:16     INFO - test2Setup@chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:92:3
02:38:16     INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the import rule. 
02:38:16     INFO - GECKO(753) | MEMORY STAT | vsize 4358MB | residentFast 339MB | heapAllocated 97MB
02:38:16     INFO - TEST-OK | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | took 131ms
02:38:16     INFO - GECKO(753) | ++DOMWINDOW == 18 (0x1147b2400) [pid = 753] [serial = 18] [outer = 0x1217e8e10]
02:38:16     INFO - TEST-START | Shutdown

Flags: needinfo?(bugs)

Stefan Hindli [:stefan_hindli]

Updated

•

2 years ago

Whiteboard: [stockwell needswork]

Jet Villegas (inactive)

Comment 3

•

2 years ago

Brad: the test failures reported above don't reflect the comment here: 

  // Create a Promise to watch for two StyleRuleAdded events. The first invocation should
  // be the style rule, and the second should be the import rule. We use the same processor
  // for both events, but the processor will only return true (completing the Promise) when
  // the import rule has been processed.

from:
https://searchfox.org/mozilla-central/source/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html

Can you double-check the assertions here? Thx!

Flags: needinfo?(bugs) → needinfo?(bwerth)

Brad Werth [:bradwerth]

Assignee

Comment 4

•

2 years ago

This failure is happening because the import rule StyleRuleAdded event is arriving before the style rule StyleRuleAdded event. That's not a test problem; it's a code problem and I'll have to dig a bit. I'll also improve the test so that the failure message in this case states the true cause of the failure.

Brad Werth [:bradwerth]

Assignee

Comment 5

•

2 years ago

Huh, well I can't actually find the bit of code that enforces import rules to be deferred after style rules. The deferral is mediated through a check to StyleSheet::RuleHasPendingChildSheet -- and if the rule has the sheet ready, then the processing of the rule isn't deferred at all. This seems correct. So there's an assumption in the test that isn't enforced by the code.

I'm updating the test to reflect this new understanding. It makes the test less meaningful, but it won't fail for this reason any more.

Flags: needinfo?(bwerth)

Comment hidden (mozreview-request)

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

Brad Werth [:bradwerth]

Assignee

Updated

•

2 years ago

Assignee: nobody → bwerth

Brad Werth [:bradwerth]

Assignee

Updated

•

2 years ago

Attachment #8942279 - Flags: review?(cam)

Comment hidden (Intermittent Failures Robot)

30 failures in 788 pushes (0.038 failures/push) were associated with this bug in the last 7 days.   

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **  

Repository breakdown:
* mozilla-central: 16
* autoland: 14

Platform breakdown:
* linux64: 10
* windows7-32: 6
* windows10-64: 6
* osx-10-10: 4
* windows7-32-nightly: 1
* windows10-64-nightly: 1
* macosx64-nightly: 1
* linux64-nightly: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1429744&startday=2018-01-08&endday=2018-01-14&tree=all

Brad Werth [:bradwerth]

Assignee

Updated

•

2 years ago

Attachment #8942279 - Flags: review?(cam) → review?(emilio)

Emilio Cobos Álvarez (:emilio)

Comment 8

•

2 years ago

mozreview-review

Comment on attachment 8942279 [details]
Bug 1429744: Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules.

https://reviewboard.mozilla.org/r/212566/#review220108

Yeah, this test is racy. The reason the order may change is because of the `RuleHasPendingChildSheet` check here:

  https://searchfox.org/mozilla-central/rev/9f2721b19b10a9a441fa5ca7116720d2fcebcc4a/layout/style/ServoStyleSheet.cpp#323

I think we may want to kill that weird notification order, but maybe the inspector relies on import rules being added already having its associated sheet? We could file a bug to investigate it.

Anyway, there's nothing preventing that sheet to be in the cache and be loaded sync.

Could you double-check / confirm that we actually have a stylesheet there, and that it's loaded?

Also, pointing this out in the commit message would be great.

r=me with that, thanks for fixing this!

Attachment #8942279 - Flags: review?(emilio) → review+

Comment hidden (mozreview-request)

Comment on attachment 8942279 [details]
Bug 1429744: Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/212566/diff/1-2/

Pulsebot

Comment 10

•

2 years ago

Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/922a011fced2
Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules. r=emilio

Code Review Bot [:reviewbot]

Comment 11

•

2 years ago

mozreview-review

Comment on attachment 8942279 [details]
Bug 1429744: Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules.

https://reviewboard.mozilla.org/r/212566/#review220322


Static analysis found 1 defect in this patch.
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint check path/to/file` (Python/Javascript/wpt)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:68
(Diff revision 2)
>  let foundStyle = false;
>  let styleFirstAddProcessor = function(event) {
>    info("styleFirstAddProcessor: called with event "+ event.rule.cssText);
>    if (event.rule.type == CSSRule.IMPORT_RULE) {
>      foundImport = true;
> +    isnot(event.rule.styleSheet, null, "Test 2: import rule has stylesheet loaded.");

Error: 'isnot' is not defined. [eslint: no-undef]

Brad Werth [:bradwerth]

Assignee

Comment 12

•

2 years ago

(In reply to Code Review Bot [:reviewbot] from comment #11)
> ::: layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:68
> (Diff revision 2)
> >  let foundStyle = false;
> >  let styleFirstAddProcessor = function(event) {
> >    info("styleFirstAddProcessor: called with event "+ event.rule.cssText);
> >    if (event.rule.type == CSSRule.IMPORT_RULE) {
> >      foundImport = true;
> > +    isnot(event.rule.styleSheet, null, "Test 2: import rule has stylesheet loaded.");
> 
> Error: 'isnot' is not defined. [eslint: no-undef]

This test should be on the ignore list; not sure why lint triggered on it. Running lint locally shows that all the SimpleTest usages fail. Setting "checkin-needed" to get help on landing it.

Keywords: checkin-needed

Mozilla Autoland

Comment 13

•

2 years ago

We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Ryan VanderMeulen [:RyanVM]

Updated

•

2 years ago

Keywords: checkin-needed

Dorel Luca [:dluca]

Comment 14

•

2 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/922a011fced2

Status: NEW → RESOLVED

Closed: 2 years ago

status-firefox60: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla60

Ryan VanderMeulen [:RyanVM]

Comment 15

•

2 years ago

bugherderuplift

https://hg.mozilla.org/releases/mozilla-beta/rev/0a7b0fd5bf12

status-firefox59: --- → fixed

Joel Maher ( :jmaher ) (UTC-4) (on leave, back October 1st)

Updated

•

2 years ago

Whiteboard: [stockwell needswork] → [stockwell fixed:logic]

Bugzilla

Perma failing layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true

Categories

(Core :: Layout, defect, P5)

Tracking

()

People

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

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:logic])

Crash Data

Security

(public)

User Story

Attachments

(1 file)

Description

Comment 1

Comment 2

Updated

Comment 3

Comment 4

Comment 5

Comment 6

Updated

Updated

Comment 7

Updated

Comment 8

Comment 9

Comment 10

Comment 11

Comment 12

Comment 13

Updated

Comment 14

Comment 15

Updated