Closed Bug 664197 Opened 13 years ago Closed 13 years ago

integrate mozinfo + new manifest parser work into xpcshell harness

Categories

(Testing :: XPCShell Harness, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ted, Assigned: ted)

References

Details

(Whiteboard: fixed-in-bs)

Attachments

(4 files, 3 obsolete files)

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.
OS: Mac OS X → All
Hardware: x86 → All
Blocks: 658928
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
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.
Attachment #539562 - Attachment is obsolete: true
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.
I haven't tested the patch, but it looks very good as stands. WFM
Attachment #541147 - Attachment is obsolete: true
Okay, this seems right. Added a few more tests and fiddled a few things from the last patch. I have two more patches coming.
I needed to add a couple of keys to mozinfo.json: "toolkit" and "crashreporter".
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.
Attachment #541474 - Flags: review?(jmaher)
Attachment #541475 - Flags: review?(jhammel)
Attachment #541476 - Flags: review?(jmaher)
>+    def testDebug(self):
>+        """
>+        Test that crashreporter values are properly detected.
>+        """

That doesn't look right.
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+
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.
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+
(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?
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.
Attachment #541739 - Flags: review?(jhammel)
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.
Attachment #541739 - Flags: review?(jhammel) → review+
Oops, plus a bustage fix to sync js/src/config:
http://hg.mozilla.org/projects/build-system/rev/8f2475ecc0d3
And one more bustage fix to fix xpcshell/selftest.py on Python 2.5:
http://hg.mozilla.org/projects/build-system/rev/9ece40393310
You need to log in before you can comment on or make changes to this bug.