1016429 - [Settings] Fix unit tests with a newer mocha

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Julien Wajsberg [:julienw]]

In Bug 874510, we try to upgrade mocha and we faced some issues in existing tests.

In Settings, here are the failing tests:

 2) [settings-test/unit/sim_settings_helper_test.js] SimSettingsHelper > SimSettingsHelper._setToSettingsDB > setToSettingsDB is called successfully:

Error: expected undefined to equal 0

at chaiAssert (http://settings.gaiamobile.org:8080/common/test/helper.js:33:1)

at equal (http://settings.gaiamobile.org:8080/common/vendor/chai/chai.js:1250:1)

at (anonymous) (http://settings.gaiamobile.org:8080/test/unit/sim_settings_helper_test.js:87:1)

at wrapper (http://settings.gaiamobile.org:8080/common/test/mocha_generators.js:62:13)

at run (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4311:7)

at runTest (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4728:5)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4806:7)

at next (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4653:7)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4663:7)

at next (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4601:16)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4630:5)

at timeslice (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:5763:5)



 14) [settings-test/unit/panels/app_permissions/app_permissions_list_test.js] app permission list > Start test appPermissionList module confirmGoClicked:

ReferenceError: MockNavigatorSettings is not defined

at set (http://settings.gaiamobile.org:8080/shared/test/unit/mocks/mock_settings_listener.js?bust=1401207306112:13:5)

at pl_confirm_go_clicked (http://settings.gaiamobile.org:8080/js/panels/app_permissions_list/app_permissions_list.js?bust=1401207306112:66:9)

at (anonymous) (http://settings.gaiamobile.org:8080/test/unit/panels/app_permissions/app_permissions_list_test.js:128:7)

at wrapper (http://settings.gaiamobile.org:8080/common/test/mocha_generators.js:62:13)

at run (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4311:7)

at runTest (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4728:5)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4806:7)

at next (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4653:7)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4663:7)

at next (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4601:16)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4625:7)

at done (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4300:5)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4314:9)

at (anonymous) (http://settings.gaiamobile.org:8080/common/test/mocha_generators.js:46:13)

at wrapper (http://settings.gaiamobile.org:8080/common/test/mocha_generators.js:73:15)

at run (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4311:7)

at next (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4613:5)

at (anonymous) (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4630:5)

at timeslice (http://settings.gaiamobile.org:8080/common/vendor/mocha/mocha.js:5763:5)

Comment 1

[Julien Wajsberg [:julienw]]

Actually, the first one seems to happen intermittently on master.

Comment 2

[Julien Wajsberg [:julienw]]

And the first one is actually not happening anymore. I'll have a fix for the second one shortly.

Comment 3

[Julien Wajsberg [:julienw]]

Attachment: github PR

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 8442012 [details] [review]
github PR

Hey Arthur,

the fix is not really nice, if you have a better idea I'd be really happy to do it. Otherwise, it seems to fix the issue for me.

I think the issue could happen in current master too, but because the newer mocha is faster the issue is happening more often.

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8442012 [details] [review]
github PR

r=me. It seems we can only force to load the mock settings as it couldn't be required by alemeda.

Actually I should modify the test to make it use a simpler mock of settings listener that is not depend on MockNavigatorSettings.

Comment 6

[Julien Wajsberg [:julienw]]

master: 672342bdb2a1bb7738cca840fc00b4846110d2e1

Thanks !

Comment 7

[Arthur Chen [:arthurcc][inactive after 6/30]]

Hi Julien, how did you check if the patch does fix the issue? I have a idea that by using "deps" in setup.js maybe we could make requirejs load MockNavigatorSettings when loading MockSettingsListener.

Comment 8

[Julien Wajsberg [:julienw]]

It definitely fixes the issue I had in Bug 874510 but I'm open with other ways of doing the same thing.

I don't know requirejs at all, that's the problem for me ;)

Comment 9

[Julien Wajsberg [:julienw]]

Sorry, I read "did you check" instead of "how did you check".

So I applied the patch from bug 874510 and ran it several times. Without the patch it fails sometimes, with the patch I couldn't make it fail.