integrate mozinfo + new manifest parser work into xpcshell harness

RESOLVED FIXED

Status

Testing
XPCShell Harness
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-bs)

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

7 years ago
bug 616999 integrated ManifestDestiny into the xpcshell harness, but it is lacking a bit in terms of test conditions. jhammel did some follow-up work to integrate my boolean expression evaluator into ManifestDestiny, as well as writing a mozinfo package in bug 606524 to provide conditions that can be used in expressions. The recently fixed bug 663180 provides a JSON file in the objdir that can be fed to mozinfo to conditionally disable tests via ManifestDestiny. This bug will track gluing all that work together to make xpcshell manifests able to use real conditional disabling.
(Assignee)

Updated

7 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

7 years ago
Blocks: 658928
(Assignee)

Comment 1

7 years ago
Created attachment 539270 [details] [diff] [review]
integrate mozinfo + ManifestDestiny update into xpcshell harness (WIP)

WIP, most of this patch is updating ManifestDestiny (and importing its tests) + importing mozinfo. It successfully runs xpcshell tests, though.

TODO:
1) Hook up the ManifestDestiny tests
2) Package mozinfo.json with test package
3) Write some tests for the xpcshell test harness to verify behavior
4) Fixup manifests
(Assignee)

Comment 2

7 years ago
Created attachment 539562 [details] [diff] [review]
integrate mozinfo + ManifestDestiny update into xpcshell harness (WIP 2)

Fixes #1 & #2 above. jhammel: kind of sucks that the ManifestDestiny tests produce no output on success. Made me question whether they were actually doing anything.
Attachment #539270 - Attachment is obsolete: true
in working with this patch, I was unable to get the unittests to run during make or make check.  I put success output in the tests, but never saw them run.
(Assignee)

Updated

7 years ago
Attachment #539562 - Attachment is obsolete: true
(Assignee)

Comment 4

7 years ago
Created attachment 541147 [details] [diff] [review]
integrate mozinfo + ManifestDestiny update into xpcshell harness

This patch does a few things:
* Handles skipped/known fail tests
* Adds a test suite to test runxpcshelltests.py
* Some minor changes to runxpcshelltests.py to make it easier to test:
** Use the logging module everywhere instead of print
** Allow passing mozInfo in as a dict instead of a filename

I still need to take a pass through all the existing manifests and see if their conditions need to be changed, but I think this is pretty close. I'm going to give it another once-over tomorrow with slightly more rested eyes and then it will probably be ready for review.

Comment 5

7 years ago
I haven't tested the patch, but it looks very good as stands. WFM
(Assignee)

Updated

7 years ago
Attachment #541147 - Attachment is obsolete: true
(Assignee)

Comment 6

7 years ago
Created attachment 541474 [details] [diff] [review]
integrate mozinfo + ManifestDestiny update into xpcshell harness

Okay, this seems right. Added a few more tests and fiddled a few things from the last patch. I have two more patches coming.
(Assignee)

Comment 7

7 years ago
Created attachment 541475 [details] [diff] [review]
extend writemozinfo.py to add a few more keys

I needed to add a couple of keys to mozinfo.json: "toolkit" and "crashreporter".
(Assignee)

Comment 8

7 years ago
Created attachment 541476 [details] [diff] [review]
update xpcshell manifests with new conditions

This patch updates the manifest conditions. We removed --disable-ipc, so I got rid of all the ipc conditions.

I need to run these all past try before they land anywhere, obviously.
(Assignee)

Updated

7 years ago
Attachment #541474 - Flags: review?(jmaher)
(Assignee)

Updated

7 years ago
Attachment #541475 - Flags: review?(jhammel)
(Assignee)

Updated

7 years ago
Attachment #541476 - Flags: review?(jmaher)

Comment 9

7 years ago
>+    def testDebug(self):
>+        """
>+        Test that crashreporter values are properly detected.
>+        """

That doesn't look right.

Comment 10

7 years ago
Comment on attachment 541475 [details] [diff] [review]
extend writemozinfo.py to add a few more keys

looks good except for the testDebug function name (I think?).  for standalone mozinfo users, I'm guessing there's no real way of detecting the toolkit?
Attachment #541475 - Flags: review?(jhammel) → review+
(Assignee)

Comment 11

7 years ago
Yeah. The best you could do is just guess the default for the platform, since our official builds only ever use the default toolkit. The only time it wouldn't match would be weird things like Linux Qt builds or OS X GTK2 builds.

Also, this patch failed on tryserver because the OS X Universal build couldn't unify mozinfo.json. That's...going to require some thought.
(Assignee)

Comment 12

7 years ago
Try run, FWIW:
http://tbpl.mozilla.org/?tree=Try&rev=7bb86ce50346

jhammel: your manifest parser tests fail on tinderbox. :-( Looks like maybe you're relying on namedtuple, which is a Python 2.6 feature, and we only have 2.5?
Comment on attachment 541476 [details] [diff] [review]
update xpcshell manifests with new conditions

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

looks good!
Attachment #541476 - Flags: review?(jmaher) → review+
Comment on attachment 541474 [details] [diff] [review]
integrate mozinfo + ManifestDestiny update into xpcshell harness

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

I haven't ran this patch locally, but assume all the unittests work.

::: build/mozinfo.py
@@ +134,5 @@
> +update({})
> +
> +choices = {'os': ['linux', 'win', 'mac', 'unix'],
> +           'bits': [32, 64],
> +           'processor': ['x86', 'x86_64', 'ppc']}

x86_64 and 32 bits?  Not sure I see the need for x86_64.

@@ +160,5 @@
> +        except ImportError:
> +            try:
> +                from simplejson import loads
> +            except ImportError:
> +                def loads(string):

do we have loads() support in python 2.5?  We are doing some hacking in runxpcshelltests.py to read json.

::: testing/xpcshell/example/unit/test_skip.js
@@ +1,2 @@
> +function run_test() {
> +  // This test expects to fail.

expects to skip

::: testing/xpcshell/runxpcshelltests.py
@@ +383,5 @@
>                 manifest=None, testdirs=[], testPath=None,
>                 interactive=False, verbose=False, keepGoing=False, logfiles=True,
>                 thisChunk=1, totalChunks=1, debugger=None,
>                 debuggerArgs=None, debuggerInteractive=False,
> +               profileName=None, mozInfo=None):

this variable list is pretty long with 18 variables!!  Maybe a followup bug to put most of this information into an object (or just use the options dict)?

@@ +608,5 @@
>                      type = "string", dest="profileName", default=None,
>                      help="name of application profile being tested")
> +    self.add_option("--build-info-json",
> +                    type = "string", dest="mozInfo", default=None,
> +                    help="path to a mozinfo.json including information about the build configuration")

is this required?  It looks like it isn't, but this could be confusing.  Maybe the default could be testdir+mozinfo.json?
Attachment #541474 - Flags: review?(jmaher) → review+
(Assignee)

Comment 15

7 years ago
(In reply to comment #14)
> I haven't ran this patch locally, but assume all the unittests work.

xpcshell tests are green on all platforms except for OS X universal as mentioned in comment 11. "make check" is currently failing manifestparser tests as mentioned in comment 12.

> ::: build/mozinfo.py
> @@ +134,5 @@
> > +update({})
> > +
> > +choices = {'os': ['linux', 'win', 'mac', 'unix'],
> > +           'bits': [32, 64],
> > +           'processor': ['x86', 'x86_64', 'ppc']}
> 
> x86_64 and 32 bits?  Not sure I see the need for x86_64.

x86_64 is the name of the CPU architecture. I think the "bits" thing is a bit redundant, myself, but I didn't care that much.

> do we have loads() support in python 2.5?  We are doing some hacking in
> runxpcshelltests.py to read json.

We don't, but I don't think it matters in this case, since that's only used if you run mozinfo as a script from the commandline, which we're not doing here.

> ::: testing/xpcshell/example/unit/test_skip.js
> @@ +1,2 @@
> > +function run_test() {
> > +  // This test expects to fail.
> 
> expects to skip

No, it does expect to fail, just the harness should skip it so that won't be seen.


> this variable list is pretty long with 18 variables!!  Maybe a followup bug
> to put most of this information into an object (or just use the options
> dict)?

I'll file a bug. I don't have a really great idea, but it's worth thinking about.

> > +    self.add_option("--build-info-json",
> > +                    type = "string", dest="mozInfo", default=None,
> > +                    help="path to a mozinfo.json including information about the build configuration")
> 
> is this required?  It looks like it isn't, but this could be confusing. 
> Maybe the default could be testdir+mozinfo.json?

runxpcshelltests.py will look for mozinfo.json in the same directory as it's running from, and will error if that's not present and it wasn't specified on the commandline. Should I document that in the command help?
(Assignee)

Comment 16

7 years ago
Created attachment 541739 [details] [diff] [review]
support universal builds in mozinfo/writemozinfo

This fixes the universal build issue. I'm testing a local universal build and I've also pushed this entire patch queue to try again.

This pulls in my mozinfo patch:
http://k0s.org/mozilla/hg/mozinfo/rev/4c11b9a7c441

As well as some writemozinfo.py changes to match.
(Assignee)

Updated

7 years ago
Attachment #541739 - Flags: review?(jhammel)
(Assignee)

Comment 17

7 years ago
Okay, that last patch should fix universal builds, and jhammel checked in a ManifestDestiny fix that should fix those tests on tinderbox:
http://hg.mozilla.org/automation/ManifestDestiny/rev/3b0098faf8a6

I've pulled the updated manifestparser into my patch and re-pushed all these patches to try.

Updated

7 years ago
Attachment #541739 - Flags: review?(jhammel) → review+
(Assignee)

Comment 18

7 years ago
http://hg.mozilla.org/projects/build-system/rev/4edf5956780b
http://hg.mozilla.org/projects/build-system/rev/6feef416f752
http://hg.mozilla.org/projects/build-system/rev/20dc5833b2a5
http://hg.mozilla.org/projects/build-system/rev/57ee54e41ef3
http://hg.mozilla.org/projects/build-system/rev/a3b7bdf2e5e8
Whiteboard: fixed-in-bs
(Assignee)

Comment 19

7 years ago
Oops, plus a bustage fix to sync js/src/config:
http://hg.mozilla.org/projects/build-system/rev/8f2475ecc0d3
(Assignee)

Comment 20

7 years ago
And one more bustage fix to fix xpcshell/selftest.py on Python 2.5:
http://hg.mozilla.org/projects/build-system/rev/9ece40393310
(Assignee)

Comment 21

7 years ago
http://hg.mozilla.org/mozilla-central/rev/4edf5956780b
http://hg.mozilla.org/mozilla-central/rev/6feef416f752
http://hg.mozilla.org/mozilla-central/rev/20dc5833b2a5
http://hg.mozilla.org/mozilla-central/rev/57ee54e41ef3
http://hg.mozilla.org/mozilla-central/rev/8f2475ecc0d3
http://hg.mozilla.org/mozilla-central/rev/9ece40393310 
http://hg.mozilla.org/mozilla-central/rev/e831c1712cf6
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.