Closed Bug 1264106 Opened 4 years ago Closed 4 years ago

browser_ConsoleAPI_originAttributes.js is going to permafail when Gecko 48 merges to Aurora

Categories

(WebExtensions :: Untriaged, defect, P2, critical)

Unspecified
All
defect

Tracking

(firefox46 unaffected, firefox47+ fixed, firefox48+ fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.3 - Apr 25
Tracking Status
firefox46 --- unaffected
firefox47 + fixed
firefox48 + fixed

People

(Reporter: RyanVM, Assigned: rpl)

References

Details

(Whiteboard: triaged)

Attachments

(1 file)

[Tracking Requested - why for this release]: Permafail when Gecko 48 merges to Aurora.

Interestingly, this appears to be a DevEdition-specific issue. The problem goes away on the Beta simulations for Gecko 48.

https://treeherder.mozilla.org/logviewer.html#?job_id=19232591&repo=try

00:32:44     INFO -  376 INFO TEST-START | dom/tests/browser/browser_ConsoleAPI_originAttributes.js
00:32:44     INFO -  TEST-INFO | started process screenshot
00:32:44     INFO -  TEST-INFO | screenshot: exit 0
00:32:44     INFO -  377 INFO checking window state
00:32:44     INFO -  378 INFO fake webextension docShell created
00:32:44     INFO -  379 INFO TEST-PASS | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | the consoleAPIMessage contains the expected message -
00:32:44     INFO -  380 INFO TEST-PASS | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | the consoleAPImessage contains originattributes -
00:32:44     INFO -  381 INFO TEST-PASS | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | the consoleAPImessage's originAttributes contains the expected addonId -
00:32:44     INFO -  382 INFO TEST-UNEXPECTED-FAIL | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | found one console api messsage as expected - Got 2, expected 1
00:32:44     INFO -  Stack trace:
00:32:44     INFO -  chrome://mochikit/content/browser-test.js:test_is:967
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js:observe:34
00:32:44     INFO -  resource://gre/components/ConsoleAPIStorage.js:CS_recordEvent:137
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js line 75 > eval:null:1
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js:test:75
00:32:44     INFO -  chrome://mochikit/content/browser-test.js:Tester_execTest:810
00:32:44     INFO -  chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:711
00:32:44     INFO -  chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:741
00:32:44     INFO -  Not taking screenshot here: see the one that was previously logged
00:32:44     INFO -  383 INFO TEST-UNEXPECTED-FAIL | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | the consoleAPImessage contains originattributes -
00:32:44     INFO -  Stack trace:
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js:observe:36
00:32:44     INFO -  resource://gre/components/ConsoleAPIStorage.js:CS_recordEvent:137
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js line 75 > eval:null:1
00:32:44     INFO -  chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js:test:75
00:32:44     INFO -  chrome://mochikit/content/browser-test.js:Tester_execTest:810
00:32:44     INFO -  chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:711
00:32:44     INFO -  chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:741
00:32:44     INFO -  JavaScript error: chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js, line 37: TypeError: cachedMessages[0].originAttributes is undefined
00:32:44     INFO -  384 INFO fake webextension page logged a console api message
00:32:44     INFO -  385 INFO Console message: [JavaScript Error: "TypeError: cachedMessages[0].originAttributes is undefined" {file: "chrome://mochitests/content/browser/dom/tests/browser/browser_ConsoleAPI_originAttributes.js" line: 37}]
00:33:29     INFO -  Not taking screenshot here: see the one that was previously logged
00:33:29     INFO -  386 INFO TEST-UNEXPECTED-FAIL | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | Test timed out -
00:33:29     INFO -  MEMORY STAT | vsize 728MB | vsizeMaxContiguous 4337684MB | residentFast 183MB | heapAllocated 62MB
00:33:29     INFO -  387 INFO TEST-OK | dom/tests/browser/browser_ConsoleAPI_originAttributes.js | took 45040ms
Flags: needinfo?(lgreco)
- the test case was failing because of an additional error message which is logged
  only on DevEdition.
- this patch makes the assertions on the cached messages more resilient to this kind of
  changes in the logged messages, by filtering the cached messages and then
  making assertions on the length of the messages found.

Review commit: https://reviewboard.mozilla.org/r/45885/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45885/
Attachment #8740679 - Flags: review?(amarchesini)
Comment on attachment 8740679 [details]
MozReview Request: Bug 1264106 - Fix browser_ConsoleAPI_originAttributes.js permafail on DevEdition. r?baku

https://reviewboard.mozilla.org/r/45885/#review42463

::: dom/tests/browser/browser_ConsoleAPI_originAttributes.js
(Diff revision 1)
> -      let cachedMessages = ConsoleAPIStorage.getEvents();
> -
> -      is(cachedMessages.length, 1, "found one console api messsage as expected");
> +      let cachedMessages = ConsoleAPIStorage.getEvents().filter((msg) => {
> +        return msg.originAttributes && msg.originAttributes.addonId == FAKE_ADDON_ID;
> +      });
>  
> -      ok(cachedMessages[0].originAttributes, "the consoleAPImessage contains originattributes");
> +      is(cachedMessages.length, 1, "found the expected cached console messages from the addon");
> -      is(cachedMessages[0].originAttributes.addonId, FAKE_ADDON_ID,

removing this line, you are removing the main task for this test.
Try a different approach. Can you wait until you receive the correct log message?
Attachment #8740679 - Flags: review?(amarchesini)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #0)
> [Tracking Requested - why for this release]: Permafail when Gecko 48 merges
> to Aurora.
> 
> Interestingly, this appears to be a DevEdition-specific issue. The problem
> goes away on the Beta simulations for Gecko 48.
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=19232591&repo=try

it looks like the console error which appears in the failure logs[1] is generating the failure
and an exception that prevents the test to reach the "finish()" call.

[1]: https://treeherder.mozilla.org/logviewer.html#?job_id=19232591&repo=try#L5206

(In reply to Andrea Marchesini (:baku) from comment #2)
> Comment on attachment 8740679 [details]
> MozReview Request: Bug 1264106 - Fix browser_ConsoleAPI_originAttributes.js
> permafail on DevEdition. r?baku
> 
> https://reviewboard.mozilla.org/r/45885/#review42463
> 
> ::: dom/tests/browser/browser_ConsoleAPI_originAttributes.js
> (Diff revision 1)
> > -      let cachedMessages = ConsoleAPIStorage.getEvents();
> > -
> > -      is(cachedMessages.length, 1, "found one console api messsage as expected");
> > +      let cachedMessages = ConsoleAPIStorage.getEvents().filter((msg) => {
> > +        return msg.originAttributes && msg.originAttributes.addonId == FAKE_ADDON_ID;
> > +      });
> >  
> > -      ok(cachedMessages[0].originAttributes, "the consoleAPImessage contains originattributes");
> > +      is(cachedMessages.length, 1, "found the expected cached console messages from the addon");
> > -      is(cachedMessages[0].originAttributes.addonId, FAKE_ADDON_ID,
> 
> removing this line, you are removing the main task for this test.
> Try a different approach. Can you wait until you receive the correct log
> message?

At this point the message has been already received, I removed the explicit assertion only
because it was implicit in the filtering conditions.

If we prefer, I can add it back as an additional explicit assertion.
Flags: needinfo?(lgreco)
Comment on attachment 8740679 [details]
MozReview Request: Bug 1264106 - Fix browser_ConsoleAPI_originAttributes.js permafail on DevEdition. r?baku

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45885/diff/1-2/
Attachment #8740679 - Flags: review?(amarchesini)
Priority: -- → P2
Whiteboard: triaged
Comment on attachment 8740679 [details]
MozReview Request: Bug 1264106 - Fix browser_ConsoleAPI_originAttributes.js permafail on DevEdition. r?baku

https://reviewboard.mozilla.org/r/45885/#review44469
Attachment #8740679 - Flags: review?(amarchesini) → review+
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Iteration: --- → 48.3 - Apr 25
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9f720f8aea14
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.