bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.
Test agent does not currently expose to test writers the ability to plug in a custom test loader. This was an issue for us in migrating our gaia calendar codebase to use amd modules since we wanted to write our test cases/files as amd modules but couldn't load them with an amd-compliant module loader. James Lal sent me a patch that introduces various hacks to expose the ability to plug in a custom loader like this one https://github.com/gaye/gaia/blob/calendar-amd/apps/calendar/test/unit/setup.js#L76 and my hope is that we can land it in test-agent proper before landing our calendar amd conversion.
Summary: Support alternate module loaders as first-class citizens → Support alternate test loaders as first-class citizens
Created attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 This is James Lal's work, but I am happy to work with you (julien) to make any changes it needs in order to get it in while James is on PTO. Thanks a billion for your help!
Comment on attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 comments left on github basically, this doesn't work with apps that don't use the special loader. We need to provide a default loader that can be overwritten for apps that need it.
Comment on attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 Updated to work with existing apps!
Comment on attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 Let's do another review cycle :) But this looks better !
Comment on attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 Tests passing now and responded to second round of feedback. Thanks for all your help!
Comment on attachment 8490320 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/js-test-agent/pull/82 r=me thanks for your patience :) Of course please squash before merging!
Attachment #8490320 - Flags: review?(felash) → review+
Thank you for the thorough reviews julien!
Just making a note that I bumped the package.json versions to the latest here: https://github.com/mozilla-b2g/gaia/commit/8f452cbfb9049ffab7adc5c9283d373fbbbedafd We need to do this when updating modules or TBPL might get things over the internet.
(In reply to Kevin Grandon :kgrandon from comment #8) > Just making a note that I bumped the package.json versions to the latest > here: > https://github.com/mozilla-b2g/gaia/commit/ > 8f452cbfb9049ffab7adc5c9283d373fbbbedafd > > We need to do this when updating modules or TBPL might get things over the > internet. sorry had to revert this since we had test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=49186183&tree=B2g-Inbound after this push
Hi kevin, You shouldn't remove istanbul and watch here. There are used in build system now.
(In reply to Ricky Chien [:rickychien] from comment #10) > Hi kevin, > > You shouldn't remove istanbul and watch here. There are used in build system > now. If this is the case then they should be updated in gaia-node-modules. I'll bump these in another bug.
Unfortunately I had to revert the test-agent bump for causing problems when applying the package.json changes to master and pushing to gaia-try. This was causing errors such as: 09:07:14 INFO - TEST-UNEXPECTED-FAIL | sharedtest/test/unit/l10n/lib/context/resources_test.js | Missing resources should get ready - assert.strictEqual is not a function 09:07:14 INFO - TypeError: assert.strictEqual is not a function 09:07:14 INFO - at (anonymous) (app://sharedtest.gaiamobile.org/test/unit/l10n/lib/context/resources_test.js:31:5) Revert: https://github.com/mozilla-b2g/gaia-node-modules/commit/9302b558fcfa7a8dcf797f5311bd719a33a799e8 To avoid this, you can submit a pull request to gaia-node-modules, and update the tarball path and gaia node_modules revision in another gaia pull request. Feel free to flag me for review if you desire. I'm tracking syncing package.json with the latest gaia_node_modules in bug 1074921.
I noticed that you fixed the test agent in dev_apps, so I think everything is good here. I've restored the latest test-agent in gaia-node-modules: https://github.com/mozilla-b2g/gaia-node-modules/commit/388b4ea51463246c09399a9b36dd539daa14d8ae Please keep the bugs updated if possible. For example I don't see where https://github.com/mozilla-b2g/gaia/pull/24527 or commit a6879b79ba4c465426152e71d431c095dc8bac68 is mentioned in this bug.
Sorry Kevin and thanks for the help here.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Hi guys, chiming in late here because I hadn't seen this bug earlier. I noticed Gareth changed the setup code in apps/sharedtest/test/unit/l10n/lib: https://github.com/mozilla-b2g/gaia/commit/e6207091cf045581be27a7f4adc3f66e601ceca8 Please note that this code is maintained in a separate repository (https://github.com/l20n/l20n.js) where it is also expected to run in node.js. I landed a change in our repo which ports Gareth's commit: https://github.com/l20n/l20n.js/commit/1b199dee1c29f41359a78dd0fdc74d8540043025 I'll merge this into Gaia later this week. In the future, please keep Gandalf or me in the loop for changes to apps/sharedtest/test/unit/l10n/lib. Thanks!
I'm not sure where this change comes from anyway, I don't know if it's been reviewed by anybody...
Those were some changes I needed to make while downstreaming the test-agent changes to gaia. The problem was that the l10n tests were doing `var assert = require('assert') || window.assert;` The call to require was (I gather) meant for the node environment but it didn't play well with rjs in the browser environment (which of course was failing with "module 'assert' not found"). I didn't know that the l10n tests were also run elsewhere in nodejs. I'll make sure to send a note to you and Gandalf in the future (and sorry for the suprise). Anyway, I think we should continue in our efforts to only check code into gaia if that code is written and maintained in gaia. If l10n is written / maintained elsewhere, then its unit tests should live outside the gaia tree and we should fetch it from somewhere like npm.
The only changes I made, ftr, were to remove all of the assert assignments which are unnecessary in test-agent since assert is a global.
Sorry I actually think the failures didn't have to do with requirejs but rather changes in the way that calls to require in test-agent are handled. I think my point about running tests where software is developed and not in downstream locations still stands, but just wanted to clarify.
You need to log in before you can comment on or make changes to this bug.