1150818 - Need to load mozinfo.json into xpcshell test

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: mozinfo_in_xpcshell_test.patch

So we can easily skip test on certain conditions.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfebe25bb031

Comment 1

[(not currently active) Ted Mielczarek]

Comment on attachment 8587858 [details] [diff] [review]
mozinfo_in_xpcshell_test.patch

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

::: testing/xpcshell/runxpcshelltests.py
@@ +421,5 @@
>              '-a', self.appPath,
>              '-r', self.httpdManifest,
>              '-m',
>              '-s',
> +            '-e', 'const MOZINFO = Object.freeze(%s);' % json.dumps(self.mozInfo),

It might be nicer to write out a temp file containing this on startup and pass it with -f instead of serializing this on the command line for every xpcshell invocation.

::: testing/xpcshell/selftest.py
@@ +289,5 @@
>  
> +# A test for mozinfo.
> +LOAD_MOZINFO = '''
> +function run_test() {
> +    do_check_neq(MOZINFO, null);

This doesn't seem like it'd actually fail if MOZINFO were completely missing. Maybe do_check_true(typeof MOZINFO != "undefined") ?

@@ +290,5 @@
> +# A test for mozinfo.
> +LOAD_MOZINFO = '''
> +function run_test() {
> +    do_check_neq(MOZINFO, null);
> +    do_check_neq(MOZINFO.os, null);

Same here.

@@ +903,5 @@
> +    def testLoadMozInfo(self):
> +        """
> +        Check that mozinfo.json is loaded
> +        """
> +        self.writeFile("test_loadMozInfo.js", LOAD_MOZINFO)

I'd just include the JS test body inline here, it's not needed by other tests.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Output mozinfo into a js file

This patch might be worth to attach to bug 852207.

Anyway, this patch just adds a method to mozinfo.py to output mozinfo in the following format:

const mozinfo = Object.freeze({"foo": "bar", ...});

So the mozinfo variable can be used in any test frameworks with same manner if the framework uses this method.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Load mozinfo from a temporary file

This patch outputs mozinfo into a temporary js file and loads it as Ted suggested.

The temporary file path is:

On b2g and android:
 /data/local/xpcshell/tmp/mozinfo.js

On others:
 tests-root-dir
 or
 testharnessdir

I don't think the testharnessdir is the best place the tests-root-dir is not defined on try server so I had no choice.

Rest of part of the patch is addressed comment #1.

Thanks Ted!

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Try server result with above two patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b305032eb531

Comment 5

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> On others:
>  tests-root-dir
>  or
>  testharnessdir
> 
> I don't think the testharnessdir is the best place the tests-root-dir is not
> defined on try server so I had no choice.

I meant 'is the best place *but* the tests-root-dir'.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8589333 [details] [diff] [review]
Load mozinfo from a temporary file

Clearing feedback flag.

After some tries to load mozinfo into mochitest framework, I realized including 'const mozinfo = ...' in the mozinfo file is not good idea. In mochitest with e10s, the mozinfo file has to be loaded in chrome process and has to be redefined as a new variable in content process. So the 'const mozinfo' is useless there.

I will attach a new patch against mozbase which just outputs mozinfo *as is* to bug 852207. And I will attach a patch against xpcshell to this bug.

I am sorry for the confusion.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Load mozinfo into xpcshell test

This patch defines mozinfo as a global variable by using defineProperty something like lazy getter, there is no overhead if test does not use mozinfo at all.

I am going to request a review after bug 852207 is closed.

Try server result seems good for now:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79db7a9a5b4b

Comment 8

[:Paolo Amadini]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> This patch defines mozinfo as a global variable by using defineProperty
> something like lazy getter, there is no overhead if test does not use
> mozinfo at all.

I'd really expect this overhead to be negligible, and moreover the file has already been read to process the manifest and is cached by the OS. I think you can simplify the code by doing the I/O asynchronously in the main xpcshell task, before tests are run:

mozinfo = JSON.parse(yield OS.File.read(_MOZINFO_JS_PATH, { encoding: "utf-8" }));

Comment 9

[(not currently active) Ted Mielczarek]

We're probably splitting hairs here, honestly, it's not likely to make much difference either way.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: Output mozinfo into a JSON file

Came back from bug 852207.

This patch is almost the same as attachment 858936. The only difference is the file format. This patch outputs mozinfo as JSON.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Attachment: Load mozinfo into xpcshell test v2

Just unrotted.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f22a9bbda55

(In reply to :Paolo Amadini from comment #8)

> mozinfo = JSON.parse(yield OS.File.read(_MOZINFO_JS_PATH, { encoding:
> "utf-8" }));

I tried it but it needs other works to use osfile.jsm in testing/xpshell/head.js. Please file a new bug if you need it. I will care it later.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8609957 [details] [diff] [review]
Output mozinfo into a JSON file

Sorry. I accidentally marked this patch as obsolete.

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 8609958 [details] [diff] [review]
Load mozinfo into xpcshell test v2

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

::: testing/xpcshell/head.js
@@ +1510,5 @@
> +  let json = Components.classes["@mozilla.org/dom/json;1"]
> +    .createInstance(Components.interfaces.nsIJSON);
> +  let mozinfo = json.decodeFromStream(stream, stream.available());
> +  stream.close();
> +  return mozinfo;

Wow, I had forgotten how terrible XPCOM File I/O is. I wish we could use OS.File here, but that'd require making this async, which seems like more of a hassle.

::: testing/xpcshell/runxpcshelltests.py
@@ +603,5 @@
>          # Create a profile and a temp dir that the JS harness can stick
>          # a profile and temporary data in
>          self.profileDir = self.setupProfileDir()
>          self.tempDir = self.setupTempDir()
> +        self.setupMozinfoJS()

As per the lines above, it would be nice if setupMozinfoJS returned the path, and you assigned it here.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Attachment: Part 1: Output mozinfo into a JSON file

Rebased to master.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Attachment: Part 2: Load mozinfo.json into xpcshell tests

Rebased to master. And setupMozInfoJS returns the path.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Pushed try now.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d8b3f3cf7cc

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5c3c5566eec
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0d464df0f0b

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5c3c5566eec
https://hg.mozilla.org/mozilla-central/rev/b0d464df0f0b

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/69fd3aae80f2
https://hg.mozilla.org/releases/mozilla-aurora/rev/4613a8921664

Comment 20

[Carsten Book [:Tomcat]]

seems this cause conflicts when moving to beta :

grafting 258858:a5c3c5566eec "Bug 1150818 - Part 1: Output mozinfo into a file. r=ted"
merging testing/mozbase/mozinfo/mozinfo/mozinfo.py
3 files to edit
merging testing/mozbase/mozinfo/mozinfo/mozinfo.py failed!
merging testing/mozbase/mozinfo/tests/test.py                                                                                                   


Fallen could you take a look,thanks!

Comment 21

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Part 1 (for beta): Output mozinfo into a JSON file

Here is part 1 applied to beta. It conflicted because of the not yet existing StringVariables code.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b375ef68b89a
https://hg.mozilla.org/releases/mozilla-beta/rev/d69897abeddc