Closed
Bug 990289
Opened 11 years ago
Closed 11 years ago
No longer possible to get useful in-browser failing subtest output for mochitests
Categories
(Testing :: Mochitest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla32
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(3 files)
STEPS TO REPRODUCE:
mach mochitest-plain --keep-open dom/tests/mochitest/general/test_interfaces.html
EXPECTED RESULTS: Test is run in the main window, which shows all the failures for the test clearly so I don't have to grovel for them in the console log.
ACTUAL RESULTS: The test is run in the subframe, not in the main window, which means it doesn't give me useful info on failing subtests. The result is that something that would normally have taken 5-6 minutes took 30 because finding the test failures repeatedly took so much longer. :(
Local bisect says this is fallout from bug 987398.
Comment 1•11 years ago
|
||
I would help fix my regression, but I don't know what "test is run in the subframe" means or how I would go about investigating this. I fear someone more familiar with the JS/browser side of mochitests will need to help out here.
Assignee | ||
Comment 2•11 years ago
|
||
Let me post before and after screenshots. Unless you mean what it means in terms of test harness internal mechanics?
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 years ago
|
||
And in particular, looking at testing/mochitest/runtests.py there's all sorts of stuff in there that looks at options.testPath, which is precisely what the patch for bug 987398 stopped setting. So for example getTestPath looks at that. And that's called by buildTestURL(). So when getTestPath() started returning "", we get a url like "http://mochi.test:8888/tests/?gunk" as opposed to "http://mochi.test:8888/tests/dom/tests/mochitest/general/test_interfaces.html?gunk".
Assignee | ||
Comment 6•11 years ago
|
||
So just adding back this line:
options.testPath = test_path
right after "options.manifestFile = manifest" makes the case I care about work like it used to. Presumably that breaks something else, though?
Assignee | ||
Comment 7•11 years ago
|
||
OK, this is really making it _very_ hard to debug failing tests. Can we please get this fixed? What would the change in comment 6 break?
Flags: needinfo?(ted)
Flags: needinfo?(gps)
Flags: needinfo?(ahalberstadt)
Comment 8•11 years ago
|
||
If we scoped that change so that we only did it in the case where we had a single test file that would be an acceptable change. Presumably just reverting that will break running things from manifests properly.
(Humorously your complaint is that we accidentally fixed the long-standing bug 508664.)
Flags: needinfo?(ted)
Assignee | ||
Comment 9•11 years ago
|
||
Bug 508664 is about running in automation, mostly, right? I guess bug 979467 wasn't...
But yes, this bug is specifically about the single-file case.
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #8418181 -
Flags: review?(ted)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 11•11 years ago
|
||
No, we never run a single file in automation. If you look at the dupes of that bug you can see that lots of people expect the "run a single test" and "run a bunch of tests" cases to work the same in the test runner (which is not unreasonable). I can also see the appeal of the prior "run a test and keep it loaded" behavior though (especially since that behavior has existed for all of Mochitest's existence).
Assignee | ||
Comment 12•11 years ago
|
||
As a note, the bug on the non-single-file case is bug 988169, and it's also an incredible productivity-sink. :(
Assignee | ||
Comment 13•11 years ago
|
||
Maybe we need a different mode for "debug a test" and "run a test"? Trying to treat them identically seems to be leading to this tug-of-war...
Comment 14•11 years ago
|
||
gps is on leave, FWIW.
Agreed, a simple commandline switch to select the behavior here would probably solve everyone's issues.
Flags: needinfo?(gps)
Assignee | ||
Comment 15•11 years ago
|
||
Is there a way to default command-line switches for this stuff, by the way?
Assignee | ||
Comment 16•11 years ago
|
||
By which I mean, stick something in some dotfile so I don't have to keep typing the same switches every time.
Comment 17•11 years ago
|
||
I think there's a bug somewhere to implement a .machrc, but don't think it exists yet. For now your best bet is probably to create an alias :/
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 18•11 years ago
|
||
I already have aliases for some of this stuff; that will just end up increasing the number of aliases I have to maintain and remember. :(
In any case, Ted, is the attached patch OK as a start, or are we going to hold this up for the command-line argument thing?
Flags: needinfo?(ted)
Comment 19•11 years ago
|
||
Comment on attachment 8418181 [details] [diff] [review]
Make running a single mochitest run it in a way where the subtest results can be accessed
Review of attachment 8418181 [details] [diff] [review]:
-----------------------------------------------------------------
This is fine for now, it's just restoring the old behavior. I think ideally we'd have an argument (and a way to persist that argument so you don't have to pass it repeatedly) but I'm not going to block you on that.
Attachment #8418181 -
Flags: review?(ted) → review+
Updated•11 years ago
|
Flags: needinfo?(ted)
Assignee | ||
Comment 20•11 years ago
|
||
Flags: in-testsuite?
Comment 21•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•