732255 - -d passed for xpcshell tests that do not need -d

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Luke Wagner [:luke]]

Running all xpcshell tests (make xpcshell-tests) passes '-d' to xpcshell while running single tests or subsets of tests does not pass -d.  Whenever a developer gets a failure from a test suite, the first thing they always want to do is repro the failure by running the single test.  In my case, the failure required '-d' so I was pretty frustrated when I couldn't reproduce (esp. considering that each xpcshell test is run by itself).  This bug is to pass '-d' always.

Comment 1

[(not currently active) Ted Mielczarek]

You mean "-d" for "enable JSDebugger"? AFAICT we only pass that when a test has "debug = True":
http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py#651

Comment 2

[Luke Wagner [:luke]]

(Oops, sorry I missed your response until now.)

I don't know what '-d' really means, but what I do know is that I printed out argv in xpcshell's main() and '-d' is the only difference between the arguments passed to single-test and 'make xpcshell-tests' runs.

The reason I filed this is that I had a failure that only reproduced when I ran the whole test-suite and it took me a while to figure out that running a single test wasn't passing '-d'.  In the end I just hacked runxpcshelltests.py to always pass -d.  Being able to reproduce a failure by running a single test is really important to developers and speeds up debugging immensely.  Reproducibility with xpcshell tests is generally quite good because each tests runs in its own process, so I really hope we can smooth out this last hole.

Comment 3

[(not currently active) Ted Mielczarek]

I agree with all your assertions, I'm just trying to figure out the root cause here. AFAICT we only want to pass that flag for tests that request it (since it enables JSD). If we're passing it otherwise that seems like a bug.

Comment 4

[Luke Wagner [:luke]]

Ah.  I wonder if it goes something like this: the test harness sets the flag if it intends to run any test that needs jsd.  When you run a single test, it doesn't find any, so doesn't set the flag.  When you 'make xpcshell-tests', it finds the jsd xpcshell tests and, instead of setting -d *just for the jsd tests*, it sets them for all tests.

Comment 5

[Luke Wagner [:luke]]

Steve may know something about this!

Comment 6

[Luke Wagner [:luke]]

For posterity, from irc with sfink, running the command: 

  python -u ../config/pythonpath.py  -I../build  ../testing/xpcshell/runxpcshelltests.py  --build-info=./mozinfo.json  dist/bin/xpcshell  _tests/xpcshell/js/xpconnect/tests/unit --test-path=test_bug604362.js

does not pass -d (the test doesn't want it), but 'make xpcshell-tests' passes -d for this test (which is the bug, I think).

Comment 7

[Luke Wagner [:luke]]

And I see the confusion, my bug title was backwards (as was, probably, my understanding when filing :).

Comment 8

[(not currently active) Ted Mielczarek]

Oh! I know what goes wrong here. We use "debug" to mean "this is a debug build", so that other debug key is totally wrong. We need to rename that to "jsd" or something.

Comment 9

[(not currently active) Ted Mielczarek]

Actually, now that I think about it I'm totally not sure about that.

Comment 10

[Steve Fink [:sfink] [:s:]]

I added the debug stuff in bug 692722. Note that it is really about SM's debugMode, not JSD in particular, and right now I expect gps will be using it for tests using the new debugger API.

Comment 11

[Steve Fink [:sfink] [:s:]]

Actually, bug 692987 would be a better pointer, since that's about adding it as a manifest option.

Comment 12

[Steve Fink [:sfink] [:s:]]

Attachment: Only use -d for tests that want it

I'm still trying to reproduce (I have a run going that probably will, but it hasn't made it to js/jsd yet), but if I were to take a guess, I'd say this patch should fix it. I'll mark it for r? when I can verify that.

Comment 13

[(not currently active) Ted Mielczarek]

Oh, that seems much more likely!

Comment 14

[Steve Fink [:sfink] [:s:]]

Comment on attachment 620428 [details] [diff] [review]
Only use -d for tests that want it

Yep, it appeared to work. With the patch, I saw no -d, then -d for the jsd tests, then back to no -d.

Comment 15

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ab010583f6f

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3ab010583f6f