1132028 - browser_hiddenOneOffs_cleanup.js is going to permafail when Gecko 38 merges to Aurora

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Test permafail when Gecko 38 merges to Aurora.

Merge day is less than two weeks away, so this needs urgent attention. Note that Aurora doesn't have NIGHTLY_BUILD set, so it's likely you're depending on a feature that isn't enabled outside of trunk builds.

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

23:32:15 INFO - 1474 INFO TEST-START | browser/components/search/test/browser_hiddenOneOffs_cleanup.js
23:32:16 INFO - *************************
23:32:16 INFO - A coding exception was thrown and uncaught in a Task.
23:32:16 INFO - Full message: TypeError: hiddenOneOffs.includes is not a function
23:32:16 INFO - Full stack: test_remove@chrome://mochitests/content/browser/browser/components/search/test/browser_hiddenOneOffs_cleanup.js:41:1
23:32:16 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:667:9
23:32:16 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:593:7
23:32:16 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:752:59
23:32:16 INFO - *************************
23:32:16 INFO - 1475 INFO checking window state
23:32:16 INFO - 1476 INFO Entering test test_remove
23:32:16 INFO - 1477 INFO Waiting for engine to be added: testEngine_dupe.xml
23:32:16 INFO - 1478 INFO Search engine added: testEngine_dupe.xml
23:32:16 INFO - 1479 INFO Waiting for engine to be added: testEngine.xml
23:32:16 INFO - 1480 INFO Search engine added: testEngine.xml
23:32:16 INFO - 1481 INFO Removing testEngine_dupe.xml
23:32:16 INFO - 1482 INFO TEST-PASS | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | hiddenOneOffs has the correct engine count post removal.
23:32:16 INFO - 1483 INFO TEST-UNEXPECTED-FAIL | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/search/test/browser_hiddenOneOffs_cleanup.js:41 - TypeError: hiddenOneOffs.includes is not a function
23:32:16 INFO - Stack trace:
23:32:16 INFO - test_remove@chrome://mochitests/content/browser/browser/components/search/test/browser_hiddenOneOffs_cleanup.js:41:1
23:32:16 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:667:9
23:32:16 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:593:7
23:32:16 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:752:59
23:32:16 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:667:9
23:32:16 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:593:7
23:32:16 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:752:59
23:32:16 INFO - 1484 INFO Leaving test test_remove
23:32:16 INFO - 1485 INFO Entering test test_add
23:32:16 INFO - 1486 INFO Waiting for engine to be added: testEngine.xml
23:32:16 INFO - 1487 INFO TEST-UNEXPECTED-FAIL | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | addEngine failed with error code 2 -
23:32:16 INFO - Stack trace:
23:32:16 INFO - chrome://mochitests/content/browser/browser/components/search/test/browser_hiddenOneOffs_cleanup.js:promiseNewEngine/</<.onInitComplete/<.onError:19
23:32:16 INFO - resource://gre/components/nsSearchService.js:SRCH_SVC_addEngine/engine._installCallback:4248
23:32:16 INFO - resource://gre/components/nsSearchService.js:onError:1586
23:32:16 INFO - resource://gre/components/nsSearchService.js:SRCH_ENG_onLoad:1662
23:32:16 INFO - resource://gre/components/nsSearchService.js:SRCH_loadStopR:359
23:32:16 INFO - null:null:0
23:32:16 INFO - 1488 INFO TEST-UNEXPECTED-FAIL | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | Uncaught exception - expected PASS
23:32:16 INFO - 1489 INFO Leaving test test_add
23:32:16 INFO - 1490 INFO Removing testEngine.xml
23:32:16 INFO - 1491 INFO Removing testEngine_dupe.xml
23:32:16 INFO - JavaScript error: resource://gre/components/nsSearchService.js, line 278: NS_ERROR_ILLEGAL_VALUE: no engine passed to removeEngine!
23:32:16 INFO - 1492 INFO TEST-UNEXPECTED-FAIL | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | Cleanup function threw an exception - [Exception... "no engine passed to removeEngine!" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource://gre/components/nsSearchService.js :: FAIL :: line 278" data: no]
23:32:16 INFO - 1493 INFO MEMORY STAT vsize after test: 637730816
23:32:16 INFO - 1494 INFO MEMORY STAT residentFast after test: 172040192
23:32:16 INFO - 1495 INFO MEMORY STAT heapAllocated after test: 79380180
23:32:16 INFO - 1496 INFO TEST-OK | browser/components/search/test/browser_hiddenOneOffs_cleanup.js | took 142ms

Comment 1

[Florian Quèze [:florian]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #0)

> 23:32:16 INFO - A coding exception was thrown and uncaught in a Task.
> 23:32:16 INFO - Full message: TypeError: hiddenOneOffs.includes is not a
> function
> 23:32:16 INFO - Full stack:
> test_remove@chrome://mochitests/content/browser/browser/components/search/
> test/browser_hiddenOneOffs_cleanup.js:41:1

Arg, when reading https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes I misinterpreted "Available in Nightly channel only" as meaning "landed in the current cycle".

Chris, are you going to provide a patch to stop using Array.includes in that test, or should I? Sorry for the trouble :-(.

Comment 2

[Chris]

Attachment: bug-1132028.patch

I went ahead and changed the calls to use filter instead of contains.  Let me know if there's anything else that needs to be fixed.

Comment 3

[Florian Quèze [:florian]]

Comment on attachment 8563229 [details] [diff] [review]
bug-1132028.patch

Review of attachment 8563229 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the quick reaction here :-).

::: browser/components/search/test/browser_hiddenOneOffs_cleanup.js
@@ +37,5 @@
>      Services.prefs.getCharPref("browser.search.hiddenOneOffs").split(",");
>  
>    is(hiddenOneOffs.length, 1,
>       "hiddenOneOffs has the correct engine count post removal.");
> +  is(hiddenOneOffs.filter(x => x == "FooDupe"), "",

Array.filter returns an array, and when comparing it to a string you are relying on it being implicitly converted to a string. I guess that works, but I think using https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some would be closer to what we want here.

Comment 4

[Chris]

Attachment: bug-1132028.patch

Fixed to use some instead.

Comment 5

[Florian Quèze [:florian]]

Comment on attachment 8563473 [details] [diff] [review]
bug-1132028.patch

Review of attachment 8563473 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks! Can you push it to try including https://hg.mozilla.org/try/rev/64d4769c787e to verify it makes the test pass?

Comment 6

[Chris]

Sorry for asking this, but what is the right way to do this?  I can't seem to find any documentation on pushing to a specific try revision.  Do I just use https://hg.mozilla.org/try/rev/64d4769c787e in my push command?  Is there something else I need to do to push to a specific revision?  Or do I just need to import the contents of that patch into my queue?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to Chris from comment #6)
> Or do I just need to import the contents of that patch into my queue?

Yep, just import it into your queue (using e.g. "hg qimport https://hg.mozilla.org/try/raw-rev/64d4769c787e") and then push both changesets to try together.

Comment 8

[Chris]

Thanks for the info Gavin.

Push to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa8e5c59c1eb

Comment 9

[Chris]

Attachment: bug-1132028.patch

It doesn't look like this test caused any failures in the try run, marking it for checkin.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8564221 [details] [diff] [review]
bug-1132028.patch

Just a checkin-needed will suffice :). Thanks for the patch, I'll land it soon!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/9dde2d05baa7

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9dde2d05baa7

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Untracking.