Closed Bug 732255 Opened 10 years ago Closed 10 years ago

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

Categories

(Testing :: XPCShell Harness, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla15

People

(Reporter: luke, Assigned: sfink)

Details

Attachments

(1 file)

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.
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
(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.
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.
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.
Steve may know something about this!
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).
And I see the confusion, my bug title was backwards (as was, probably, my understanding when filing :).
Summary: can we pass -d when running a single test or a single directory of tests? → -d passed for xpcshell tests that do not need -d
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.
Actually, now that I think about it I'm totally not sure about that.
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.
Actually, bug 692987 would be a better pointer, since that's about adding it as a manifest option.
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.
Oh, that seems much more likely!
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.
Attachment #620428 - Flags: review?(ted.mielczarek)
Attachment #620428 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → sphink
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ab010583f6f
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/3ab010583f6f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.