990289 - No longer possible to get useful in-browser failing subtest output for mochitests

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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

[Gregory Szorc [:gps]]

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Let me post before and after screenshots.  Unless you mean what it means in terms of test harness internal mechanics?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Before

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: After

Comment 5

[Boris Zbarsky [:bzbarsky]]

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".

Comment 6

[Boris Zbarsky [:bzbarsky]]

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?

Comment 7

[Boris Zbarsky [:bzbarsky]]

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?

Comment 8

[(not currently active) Ted Mielczarek]

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.)

Comment 9

[Boris Zbarsky [:bzbarsky]]

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.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Make running a single mochitest run it in a way where the subtest results can be accessed

Comment 11

[(not currently active) Ted Mielczarek]

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).

Comment 12

[Boris Zbarsky [:bzbarsky]]

As a note, the bug on the non-single-file case is bug 988169, and it's also an incredible productivity-sink.  :(

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

[(not currently active) Ted Mielczarek]

gps is on leave, FWIW.

Agreed, a simple commandline switch to select the behavior here would probably solve everyone's issues.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Is there a way to default command-line switches for this stuff, by the way?

Comment 16

[Boris Zbarsky [:bzbarsky]]

By which I mean, stick something in some dotfile so I don't have to keep typing the same switches every time.

Comment 17

[Andrew Halberstadt [:ahal]]

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 :/

Comment 18

[Boris Zbarsky [:bzbarsky]]

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?

Comment 19

[(not currently active) Ted Mielczarek]

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.

Comment 20

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3d739f77aaa

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e3d739f77aaa