Last Comment Bug 904812 - Make resources 'testing-common' and 'testing' available in Mozmill tests
: Make resources 'testing-common' and 'testing' available in Mozmill tests
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Testing Infrastructure (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 43.0
Assigned To: Joshua Cranmer [:jcranmer]
:
:
Mentors:
: 1107045 (view as bug list)
Depends on:
Blocks: 500753 1204256
  Show dependency treegraph
 
Reported: 2013-08-13 13:20 PDT by Jens Müller (:tessarakt)
Modified: 2015-09-13 12:01 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add resource://testing-common to mozmill (7.49 KB, patch)
2015-04-17 15:38 PDT, Joshua Cranmer [:jcranmer]
philipp: review+
Details | Diff | Splinter Review
Enable testing modules in mozharness (2.32 KB, patch)
2015-04-17 16:23 PDT, Joshua Cranmer [:jcranmer]
philipp: review+
Details | Diff | Splinter Review

Description User image Jens Müller (:tessarakt) 2013-08-13 13:20:25 PDT
Make it possible to load modules from resource://testing-common/..., use that where possible (especially in test-nntp-helpers.js for nntpd.js and imapd.js).
Comment 1 User image Jens Müller (:tessarakt) 2013-10-22 16:20:45 PDT
For mochitests, an entry is added to some manifest file here: http://mxr.mozilla.org/comm-central/source/mozilla/testing/mochitest/runtests.py#478


For xpcshell tests, a substitution is registered in the protocol handler of the IO service: http://mxr.mozilla.org/comm-central/source/mozilla/testing/xpcshell/head.js#329. Basically, the directory has to be available as this._TESTING_MODULES_DIR at this place.

This is apparently set from the start script for the tests: http://mxr.mozilla.org/comm-central/source/mozilla/testing/xpcshell/runxpcshelltests.py#360 by handing it to xpcshell as a command line parameter, which causes a variable to be injected into the JS environment (just guessing here ...).

In the same file:

357             sanitized = self.testingModulesDir.replace('\\', '\\\\')

testingModulesDir can be handed in as a parameter: http://mxr.mozilla.org/comm-central/source/mozilla/testing/xpcshell/runxpcshelltests.py#1161; otherwise the script makes a guess: http://mxr.mozilla.org/comm-central/source/mozilla/testing/xpcshell/runxpcshelltests.py#1161

I guess for Mozmill, the way using the IO Service sounds promising. The rest of the plumbing will probably be Mozmill-specific.
Comment 2 User image Jens Müller (:tessarakt) 2013-10-22 16:56:04 PDT
Mozmill tests are invoked using this command line: http://mxr.mozilla.org/comm-central/source/mail/testsuite-targets.mk#40

The tests are run in this function: http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/runtest.py#310

jsbridge seems to be worth a closer look ...

In addition, URLs like resource://mozmill/... seem to be available in JS code. -> Find out where these resources are defined ...

Anyway: Apparently (?) for Mozmill all needed resources are copied into the directory for the virtualenv -> also copy everyhing from testing-common here?!
Comment 3 User image Joshua Cranmer [:jcranmer] 2013-10-23 08:31:48 PDT
(In reply to Jens Müller (:tessarakt) from comment #2)
> Mozmill tests are invoked using this command line:
> http://mxr.mozilla.org/comm-central/source/mail/testsuite-targets.mk#40

Only for local builds. Release engineering uses their own scripts to run them on the build servers.

> In addition, URLs like resource://mozmill/... seem to be available in JS
> code. -> Find out where these resources are defined ...

Those come from the mozmill extension.

> Anyway: Apparently (?) for Mozmill all needed resources are copied into the
> directory for the virtualenv -> also copy everyhing from testing-common
> here?!

Nope, testing-only modules are already copied into the package tests. No need to include yet another copy.
Comment 4 User image Jens Müller (:tessarakt) 2013-10-23 13:38:46 PDT
(In reply to Joshua Cranmer [:jcranmer] from comment #3)
> Nope, testing-only modules are already copied into the package tests. 

Could you be just a wee bit more verbose here?

"package" in which context? Where do they end up in the filesystem?
Comment 5 User image Jens Müller (:tessarakt) 2013-10-23 13:58:57 PDT
via IRC: make package-tests - that builds the test package we ship out to extensions
Comment 6 User image Jens Müller (:tessarakt) 2013-10-24 13:55:57 PDT
Hi Henrik,

maybe you have some thougts as to how to do this properly in Mozmill ...

BR, Jens
Comment 7 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2013-10-24 22:57:22 PDT
Sorry, but I have nearly no knowledge about the Thunderbird tests. Can you describe it in a small example? What are you seeking for?
Comment 8 User image Jens Müller (:tessarakt) 2013-11-11 15:19:29 PST
Hi Henrik,

this is about making libraries through (require and) resource URLs, instead of relative paths, like here:

http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-nntp-helpers.js#57
(which invokes http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-folder-display-helpers.js#2767, which actually loads the script using local files etc.)

I hope I explained the goal sufficiently now.

Jens
Comment 9 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2013-11-18 14:11:08 PST
Well, Mozmill supports the require() method for a long time. Not sure which specifics your request should have. We cannot include your custom libraries to Mozmill as we also can't include ours for Firefox. But as of now you can't set a specific root path AFAIK. Can you give me an example which combinations you would like to have? Thanks.
Comment 10 User image Jens Müller (:tessarakt) 2013-11-23 04:19:30 PST
(In reply to Henrik Skupin (:whimboo) from comment #9)
> Well, Mozmill supports the require() method for a long time. Not sure which
> specifics your request should have. We cannot include your custom libraries
> to Mozmill as we also can't include ours for Firefox. But as of now you
> can't set a specific root path AFAIK. Can you give me an example which
> combinations you would like to have? Thanks.

Not sure I understand everything you are writing here, but in http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-nntp-helpers.js#57, I'd like to do something like 

require("resource://testing-common/nntpd.js");

And resource://testing-common/ should only be defined when running the Mozmill tests.
Comment 11 User image Jens Müller (:tessarakt) 2013-11-23 11:00:04 PST
I just found https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Using ...

"Programmatically adding aliases

Custom aliases to paths that can be represented as an nsILocalFile can be programmatically added as well."

I guess that is what we want to do at some appropriate place in Mozmill.
Comment 12 User image Henrik Skupin (:whimboo) [away 02/18 - 02/27] 2013-11-24 12:43:17 PST
(In reply to Jens Müller (:tessarakt) from comment #10)
> require("resource://testing-common/nntpd.js");

I don't know our module loading system that well. We included it a long time ago. But I don't think that it was written to load resource modules. What you want here is most likely Components.utils.import(), which is able to load those shared code modules. require() works differently, and means that you wouldn't need the resource specifier. But for anything like that it would be better to file a specific bug in the mozmill component. That way we can figure out if we can improve our module system.
Comment 13 User image Philipp Kewisch [:Fallen] 2014-02-05 14:52:04 PST
I stumbled upon this while investigating something else. I personally see two options to fix this:


1) Generate a stub extension that adds the resource
Contains two files: install.rdf to make it an extension and a chrome.manifest with this line:
> resource testing-common /absolute/path/to/base/directory 

Using an absolute path works and requires the extension to be generated on the fly when mozmill is run. I've tested this on Mac, it will probably also work on Linux, I don't know about windows. Another option is to copy the modules into the extension and use the usual resource registration.

Then the extension can be added to mozmill using the profile args, I didn't check how we actually call it but I know mozmill allows passing profile_args, which can take a list of extensions.



2) Make some slight changes to mozmill:

* Add a function to the extension's frame.js 

> function registerModule(name, path) {
>     let protocolHandler = Services.io.getProtocolHandler("resource")
>                                   .QueryInterface(Components.interfaces.nsIResProtocolHandler);
>
>     let modulesFile = Components.classes["@mozilla.org/file/local;1"]
>                                 .createInstance(Components.interfaces.nsILocalFile);
>     modulesFile.initWithPath(path);
>     protocolHandler.setSubstitution(name, ios.newFileURI(modulesFile));
> }

* Add a parameter to the python MozMill class called testingModulesDir, similar to runxpcshelltests.
* Use this parameter around the part where the http server is started, calling frame.registerModule("testing-common", self.testingModulesDir);


I think (2) would be cleanest, it is generic enough that it could also benefit Firefox. I probably won't have time to do it myself though.
Comment 14 User image Joshua Cranmer [:jcranmer] 2014-12-06 22:39:14 PST
*** Bug 1107045 has been marked as a duplicate of this bug. ***
Comment 15 User image Hiroyuki Ikezoe (:hiro) 2014-12-07 02:02:25 PST
Thanks Joshua. I did not notice this bug.

A patch in bug 117045 is similar to (2) in comment #13. I will revise the patch.
Comment 16 User image Joshua Cranmer [:jcranmer] 2015-04-17 15:38:19 PDT
Created attachment 8594241 [details] [diff] [review]
Add resource://testing-common to mozmill

After analyzing how this was handled in other mozharness suites, the easiest way is to add a --testing-modules-dir option to the suite runner and thread that through the harness. The path is normalized to an absolute path, since it looks like someone changes the cwd deep in mozmill and I didn't want to deal with that.

This doesn't actually add any testing-common resources yet, but I tested it by removing the try/catch around the import in messageInjection.js. Try results can be found here: <https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8b4ee0f309f3>.
Comment 17 User image Joshua Cranmer [:jcranmer] 2015-04-17 16:23:09 PDT
Created attachment 8594252 [details] [diff] [review]
Enable testing modules in mozharness

Gee, a few months ago, this would have required a change to buildbot-configs or something. Now it's just a change to a file in mozilla-central.
Comment 18 User image Philipp Kewisch [:Fallen] 2015-04-19 13:34:10 PDT
Comment on attachment 8594252 [details] [diff] [review]
Enable testing modules in mozharness

Review of attachment 8594252 [details] [diff] [review]:
-----------------------------------------------------------------

You sure you want me to review this patch? It seems to be a m-c patch?
Comment 19 User image Joshua Cranmer [:jcranmer] 2015-09-12 12:03:43 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #18)
> Comment on attachment 8594252 [details] [diff] [review]
> Enable testing modules in mozharness
> 
> Review of attachment 8594252 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You sure you want me to review this patch? It seems to be a m-c patch?

It's a change to mozharness configs, so it doesn't really impact m-c at all.
Comment 20 User image Joshua Cranmer [:jcranmer] 2015-09-12 15:03:36 PDT
Pushing the c-c changes, given a green(-ish) try build:
https://hg.mozilla.org/comm-central/rev/200c5cc46353
Comment 21 User image Joshua Cranmer [:jcranmer] 2015-09-12 15:06:38 PDT
And the m-c changeset has been pushed to m-i:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b5fec1df4d
Comment 22 User image Phil Ringnalda (:philor) 2015-09-13 12:01:08 PDT
https://hg.mozilla.org/mozilla-central/rev/e0b5fec1df4d

Note You need to log in before you can comment on or make changes to this bug.