b2g_ubuntu64_vm mozilla-inbound opt test gaia-unit on 2014-07-14 19:22:22 PDT for push 6b9c96d0f03d https://tbpl.mozilla.org/php/getParsedLog.php?id=43792566&tree=Mozilla-Inbound&full=1 20:03:00 INFO - gaia-unit-tests TEST-UNEXPECTED-FAIL | system/test/unit/statusbar_test.js | system/Statusbar "after each" hook | TypeError: navigator.mozL10n is undefined (app://system.gaiamobile.org/js/statusbar.js?time=1405393378621:687)
Kevin, can you help us find an owner for this? Pretty longstanding at this point.
Created attachment 8491771 [details] [review] Pull request - Stop clock on teardown Not sure if this is going to work, but I'll run it a bunch on gaia-try to see what it does.
Comment on attachment 8491771 [details] [review] Pull request - Stop clock on teardown Based on this test run here I feel that this is now solved: https://tbpl.mozilla.org/?rev=e3f3a4cd58df172a861a367f94236342eeafb093&tree=Gaia-Try Requesting either Mike or Julien to review this, as Mike knows the statusbar code well, and Julien our test framework - but a single review is probably fine. I tried a number of things playing with the mocks, but for some reason I could not get this passing no matter what I tried. I'd like to land this using the real mozL10n, as several other tests use, and keep an eye on this in master.
Stas, I sense that it will not make you happy wrt. bug 1035385 and bug 1043643. Do you want to try to get it to work with shared mock?
Zibi, Stas - if one of you guys has time to dig into this I'd be happy to take care of this another way. I have a feeling that it's something about the mock lifecycle that's causing this. One option might be to modify the frontend code, but I would prefer to avoid doing that if possible - it can cause problems down the road and mask real errors.
I may be wrong Kevin, but it looks like to me that the issue comes from the fact that you use the real clock.js instead of using a mock for the Clock object. I see "toggleTimeLabel" is called in a lot of places, this calls "this.clock.start" with the "update.time" function as parameter. The error happens in this function. Another way could be to simply use sinon's fake timers to prevent the real clock.js from calling update.time, except where you want it (but actually I don't see any test for this anyway). To quickly fix the issue I'd use the fake timers, but longterm I think it's better to mock clock.js so that it's easier to test Clock and StatusBar separately.
Created attachment 8492144 [details] [review] Use MockL10n in suiteSetup, not setup. There's as many theories as people contributing to this bug :) I like Julien's approach which appears to fix the real issue. In my pull request I moved the assignment of navigator.mozL10n = MockL10n to suiteSetup, and navigator.mozL10n = realMozL10n to suiteTeardown. realMozL10n is undefined in tests, so the later we assign it back, the smaller chance a stray timer will try to call mozL10n.get. https://tbpl.mozilla.org/?rev=c122e513b2c9&tree=Gaia-Try
Comment on attachment 8492139 [details] [review] github PR - use fake timers Looks quite green to me :) Tell me what you think
Attachment #8492139 - Flags: review?(kgrandon)
Comment on attachment 8492139 [details] [review] github PR - use fake timers I still don't like using mocks in tests where possible, but lots of people disagree with me there, but I'm fine with landing this. Thanks for fixing it! :)
Attachment #8492139 - Flags: review?(kgrandon) → review+
I'm waiting for the last tries and then I'm landing :)
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
a=tests v2.1: d558bd001edee9defa017bd67551d43878077932
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
You need to log in before you can comment on or make changes to this bug.