Make resources 'testing-common' and 'testing' available in Mozmill tests

RESOLVED FIXED in Thunderbird 43.0

Status

Thunderbird
Testing Infrastructure
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: tessarakt, Assigned: jcranmer)

Tracking

(Blocks: 2 bugs)

Trunk
Thunderbird 43.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
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).
(Reporter)

Updated

4 years ago
See Also: → bug 748490
(Reporter)

Comment 1

4 years ago
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.
(Reporter)

Updated

4 years ago
Assignee: nobody → blog
(Reporter)

Comment 2

4 years ago
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?!
(Assignee)

Comment 3

4 years ago
(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.
(Reporter)

Comment 4

4 years ago
(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?
(Reporter)

Comment 5

4 years ago
via IRC: make package-tests - that builds the test package we ship out to extensions
(Reporter)

Updated

4 years ago
Summary: Make testing-common modules available in Mozmill tests → Make resources 'testing-common' and 'testing' available in Mozmill tests
(Reporter)

Comment 6

4 years ago
Hi Henrik,

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

BR, Jens
Flags: needinfo?(hskupin)
Sorry, but I have nearly no knowledge about the Thunderbird tests. Can you describe it in a small example? What are you seeking for?
Flags: needinfo?(hskupin)
(Reporter)

Comment 8

4 years ago
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
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.
(Reporter)

Comment 10

4 years ago
(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.
(Reporter)

Updated

4 years ago
Flags: needinfo?(hskupin)
(Reporter)

Comment 11

4 years ago
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.
(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.
Flags: needinfo?(hskupin)
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.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1107045
(Assignee)

Updated

3 years ago
Blocks: 500753
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.
(Assignee)

Comment 16

2 years ago
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>.
Assignee: blog → Pidgeot18
Status: NEW → ASSIGNED
Attachment #8594241 - Flags: review?(philipp)
(Assignee)

Comment 17

2 years ago
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.
Attachment #8594252 - Flags: review?(philipp)
Attachment #8594241 - Flags: review?(philipp) → review+
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?
Attachment #8594252 - Flags: review?(philipp) → review+
(Assignee)

Comment 19

2 years ago
(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.
(Assignee)

Updated

2 years ago
Blocks: 1204256
(Assignee)

Comment 20

2 years ago
Pushing the c-c changes, given a green(-ish) try build:
https://hg.mozilla.org/comm-central/rev/200c5cc46353
(Assignee)

Comment 21

2 years ago
And the m-c changeset has been pushed to m-i:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b5fec1df4d
https://hg.mozilla.org/mozilla-central/rev/e0b5fec1df4d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
You need to log in before you can comment on or make changes to this bug.