Replace per-app mock_l10n with /shared/test/unit/mock_l10n.js
Apps like bluetooth, collection, email, ftu, search, settings, verticalhome, video, wappush use their own mock_l10n. We should migrate all of those to shared/test/unit/mock_l10n.js There are also tests the create their own MockL10n within the test file. We should clean that too.  https://github.com/mozilla-b2g/gaia/blob/62eedafb0657bbec8941f5bdaa919b5a59f25db1/apps/settings/test/unit/findmydevice_panel_test.js#L35-L41 It's a fairly easy first bug. I'm happy to mentor
please assign this bug to me, i am new here and would like to contribute
Hey Aniket, thanks for stepping in! Don't hesitate to ping me or gandalf on IRC if you have questions, we'll be more than happy to help!
Created attachment 8479429 [details] [review] PR request
Comment on attachment 8479429 [details] [review] PR request That looks good! Please, remove the test update from video_test.js. The reason why it doesn't work is because the test is written in a way that depends on mozL10n.once not being fired in the test (while shared mock_l10n does fire it). We should file a separate bug to update the whole test file to use shared mock_l10n. I left some comment notes in the patch, but overall it looks great! I'm settings Stas as the second reviewer, since he knows about tests more than I do :)
Also, please look here: https://tbpl.mozilla.org/?rev=28c4705343cb5b30f8e7276b22e8e20ce624f51f&tree=Gaia-Try and clean the Li errors (linting errors).
Comment on attachment 8479429 [details] [review] PR request Hey Aniket -- the patch looks good but before I give it a final r+, please remove npm-debug.log from it. I also noticed that the following mock_l10n.js files are still present on your branch: apps/video/test/unit/mock_l10n.js apps/emergency-call/test/unit/mock_l10n.js apps/collection/test/unit/mock_l10n.js apps/bookmark/test/unit/mock_l10n.js apps/communications/contacts/test/unit/mock_l10n.js apps/homescreen/test/unit/mock_l10n.js apps/settings/test/unit/mock_l10n.js Are they required? Can we remove them?
The one in emergency-call has been removed, the rest of the apps have some tests which do not sync well with the commoon mock_l10n, and hence they use their own. I have created a PR
Hi Aniket! Please rebase your patch onto the current master. 2.1 branched off yesterday, and we should be using the latest master code as the base. (In reply to Staś Małolepszy :stas from comment #6) > apps/video/test/unit/mock_l10n.js > apps/collection/test/unit/mock_l10n.js > apps/communications/contacts/test/unit/mock_l10n.js > apps/homescreen/test/unit/mock_l10n.js Can you file four bugs, one for each of these mocks in their respective components, blocking bug 1035385? > apps/bookmark/test/unit/mock_l10n.js > apps/settings/test/unit/mock_l10n.js I was able to remove these two files, I'll attach a patch for you to apply.
Created attachment 8483403 [details] [diff] [review] Remove mock_l10n from apps/bookmark and apps/settings Aniket, feel free to use this patch in your pull request.
Comment on attachment 8479429 [details] [review] PR request Canceling the review for now. Please re-request when you address my comment 8 and comment 9. Thanks!
(In reply to Zibi Braniecki [:gandalf] from comment #11) > Ping? Sorry for the late reply internet in my institute was down due to some problem, it is under repair hence communication problem
Hi Aniket -- are you still working on this? I feel this is very close to being landable, just needs a final touch from comment 9. Would you like to finish this? Do you need my help? Just let me know!
(In reply to Staś Małolepszy :stas from comment #8) > > apps/video/test/unit/mock_l10n.js > > apps/collection/test/unit/mock_l10n.js > > apps/communications/contacts/test/unit/mock_l10n.js > > apps/homescreen/test/unit/mock_l10n.js > > Can you file four bugs, one for each of these mocks in their respective > components, blocking bug 1035385? I filed bug 1078183, bug 1078187, bug 1078212 and bug 1078214.
I took Aniket's pull request, applied my patch and rebased it on top of the current master: https://github.com/mozilla-b2g/gaia/pull/24810 https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=80241643adea I think this is good for landing now.
Master: https://github.com/mozilla-b2g/gaia/commit/a6c29cc2f048dc08a2f008322699368b7e5cd9b0 Thanks, Aniket, for all your hard work on this!
I broke one test when I rebased my branch. I'll have a follow up in a second.
If anyone's looking for this, here's the broken Gu test: INFO - TEST-UNEXPECTED-FAIL | settings/test/unit/panels/root/bluetooth_item_test.js | BluetoothItem "before all" hook - Error: Load failed: unit/mock_l10n: app://settings.gaiamobile.org/test/unit/mock_l10n.js?bust=1412759831948 (app://settings.gaiamobile.org/js/vendor/alameda.js?time=1412759831850:893)