Almost certainly fallout from bug 976920 merging over from mozilla-beta. Not sure if this is exposing bugs that should be fixed or if we should just backout bug 976920 from b2g30 and get on with life. https://tbpl.mozilla.org/php/getParsedLog.php?id=40084747&tree=Mozilla-B2g30-v1.4 b2g_ubuntu64_vm mozilla-b2g30_v1_4 opt test gaia-unit on 2014-05-21 02:04:22 PDT for push 12fe2b67a099 slave: tst-linux64-spot-182 gaia-unit-tests TEST-UNEXPECTED-FAIL | communications/dialer/test/unit/mmi_test.js | dialer/mmi Should handle network initiated messages properly | global leak detected: postMessage gaia-unit-tests TEST-UNEXPECTED-FAIL | communications/contacts/test/unit/image_loader_test.js | Image Loader Test Suite > imgsLoading balance > "before each" hook | global leak detected: stop gaia-unit-tests TEST-UNEXPECTED-FAIL | camera/test/unit/controllers/preview-gallery_test.js | controllers/preview-gallery PreviewGalleryController() Should add and remove a resize handler when opening and closing view | global leaks detected: addEventListener, removeEventListener gaia-unit-tests TEST-UNEXPECTED-FAIL | sms/test/unit/attachment_test.js | attachment_test.js view attachment with open activity Activity errors > "before each" hook | global leak detected: alert gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/hardware_buttons_test.js | system/HardwareButtons press and release home (screen enabled) | global leak detected: dispatchEvent gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/radio_test.js | Radio > set enabled to true conn0 is enabling, conn1 is enabled "before each" hook | global leak detected: dispatchEvent gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/fxa_manager_test.js | system/FxAccountManager > "before each" hook | global leak detected: addEventListener gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/fxa_manager_test.js | system/FxAccountManager > On openFlow mozFxAccountsRPChromeEvent "before each" hook | global leak detected: dispatchEvent gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/update_manager_test.js | system/UpdateManager "after each" hook | TypeError: self.toaster is null (http://system.gaiamobile.org:8080/js/update_manager.js?time=1400664443911:406) gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/fxa_client_test.js | system/FxAccountsClient > "before each" hook | global leaks detected: addEventListener, removeEventListener, dispatchEvent gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/app_window_factory_test.js | system/AppWindowFactory handle event classic app launch | global leak detected: dispatchEvent

We don't want to back bug 976920 out (except temporarily), because there are known web compat issues with the state without that bug. Peter, this sounds similar to the issue you ran into with the Window patches on inbound, right?

Ah, yes. See bug 789261 comment 10. Basically, the gaia unit tests are making busted assumptions. We should just add every single one of these to the globals whitelist, imho. Going to do a try run to verify this.

Also, who the heck owns this stuff? Trying some people who have blame for the relevant files, because while I can add this stuff to the whitelist, any time someone starts using a new function it will need to be added there too, which doesn't scale. So we really need to fix the tests to not be broken.

I don't own these tests but I put together some of the bits used to write the tests... We are using some off the shelf test framework (mocha) which is doing some assertions after each test to ensure no new globals are exposed... We can disable the global checks and upstream a patch to make these globally acceptable... I guess there was some change that effects the visibility of window properties which is why this is cropping up.

> which is doing some assertions after each test to ensure no new globals are exposed... Right, but you're also using sinon.js, which adds window properties all the time, is the point. > I guess there was some change that effects the visibility of window properties More precisely, such a change was reverted for now on beta.

I was only updating sinon to 1.9.1 (bug 995425). I don't think I changed any code in tests.

So I guess the problem is fundamentally with any test that does sinon.spy(window, foo) where foo comes from the proto chain, not the window object itself. The smart thing to do if we want to keep this mocha "find props that got randomly added" feature would be to just teach sinon.spy to tell mocha about the properties it's adding... or to have sinon remove the props it adds at the end of the test.

Gaia now uses JSHint which does a pretty good job of finding unintentional globals. I think we should just remove this feature.

see also bug 1013816

Yep, that's the trunk version.

https://tbpl.mozilla.org/?tree=Try&rev=688db6ec9aaf is green. Can we get it reviewed and landed soon please? :)

Created a pull request at https://github.com/mozilla-b2g/gaia/pull/19512 Not sure how to request review on that...

Comment on attachment 8426677 [details] Link to pull request Ah right - if this is for 1.4 I think something like this is fine. Going forward I'll try to land bug 1014180, or get this patched upstream. I'll give F+, but someone like Julien or James should probably put the official R+ on the patch.

I think we'll try to land bug 1014180 on the branch instead. Kevin, you can take care of this too?

This requires uplifting all of our node_module dependencies to 1.4. I'm not sure how feasible this is, but I will open a pull request to see what kind of breakage we get. We'll likely need to disable a bunch of tests in 1.4

Another possibility could be to branch node_modules ? (BTW that's what Travis does when caching stuff) Although I'm optimistic here :) If we need to disable tests to uplift 1014180, then I'd rather go with Boris' simple solution.

Ok, let's try it. Branched node_modules in 1.4 and updated test-agent: https://github.com/mozilla-b2g/gaia-node-modules/commit/00b8ace9941a74105597beefdbed8b0bb9c3fc00 And here is the pull request to gaia: https://github.com/mozilla-b2g/gaia/pull/19532 It's pretty self-contained. If it errors out, let's go with the simple approach.

Talked it over with Julien and Kevin and decided to go with bz's PR for now to at least get b2g30 green again. Leaving the bug open until we see if comment 19 is viable or not. v1.4: https://github.com/mozilla-b2g/gaia/commit/8990a6b8ee6a2c8f9668ef24e6d86e37aecad750

Turns out we can't do a straight update of test-agent for 1.4, and I don't want to spend too much longer investigating. I'll use this patch for master, and we can let 1.4 ship as-is.

Comment on attachment 8426677 [details] Link to pull request This was irc-reviewed.