969437 - add some way to disable mochitest test summarization for locally-run tests

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Nathan Froyd [:froydnj]]

Honza Bambas mentioned yesterday that being able to see all the TEST-PASS messages is a useful thing, because you can correlate what tests are passing with, e.g. NSPR logging output.  We should add some sort of capability of doing this, probably through mach (Honza mentioned he used |mach -v| and verboseness seems appropriate for this sort of thing).

Comment 1

[Bobby Holley (:bholley)]

+1 to this. See also bug 973000.

Comment 3

[Bobby Holley (:bholley)]

Andrew, do you plan bundle the fix for bug 973000 into this bug? They're two separate feature requests, which is why I filed it separately.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: always output log messages for a single mochitest run

I thought dup'ing the bugs was a little odd too, but no matter.

Here's a patch for the scenario Bobby described.  I don't feel great about
reaching into the guts of parentRunner, but I don't think there's another good
option.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8376453 [details] [diff] [review]
always output log messages for a single mochitest run

Review of attachment 8376453 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/tests/SimpleTest/SimpleTest.js
@@ +22,5 @@
>  // In normal test runs, the window that has a TestRunner in its parent is
>  // the primary window.  In single test runs, if there is no parent and there
>  // is no opener then it is the primary window.
> +var isSingleTestRun = (parent == window && !opener)
> +var isPrimaryTestWindow = !!parent.TestRunner || isSingleTestRun;

this makes more sense now

@@ +322,5 @@
>          var outputCoalescedMessage = numCoalescedMessages == coalesceThreshold;
>  
> +        var runningSingleTest = ((parentRunner &&
> +                                  parentRunner._urls.length == 1)
> +                                 || isSingleTestRun);

can't we just say:
if runningSingleTest: outputCoalescedMessage = False

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Joel Maher (:jmaher) from comment #5)
> @@ +322,5 @@
> >          var outputCoalescedMessage = numCoalescedMessages == coalesceThreshold;
> >  
> > +        var runningSingleTest = ((parentRunner &&
> > +                                  parentRunner._urls.length == 1)
> > +                                 || isSingleTestRun);
> 
> can't we just say:
> if runningSingleTest: outputCoalescedMessage = False

I think you mean outputCoalescedMessage = True?

Simpler would be good, but outputCoalescedMessage == True causes us to append verbiage about elided messages, which would be inaccurate and misleading.

Comment 7

[Andrew Halberstadt [:ahal]]

(In reply to Bobby Holley (:bholley) from comment #3)
> Andrew, do you plan bundle the fix for bug 973000 into this bug? They're two
> separate feature requests, which is why I filed it separately.

Sorry, I misread the title of this bug. I thought it said "Disable test summarization for locally run tests" instead of "Add some way...". Feel free to re-open the other bug if that doesn't get fixed here. (Once this is in place the other bug is just a simple change to the mach target).

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8376453 [details] [diff] [review]
always output log messages for a single mochitest run

Works locally. Thanks for doing this!

Maybe this patch should move to bug 973000?

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8376453 [details] [diff] [review]
always output log messages for a single mochitest run

This patch is no longer applicable to this bug.

Comment 10

[Nathan Froyd [:froydnj]]

Bill mentioned this to me on IRC the other day, too, also in the context of logging things across several tests.  He argued that any silencing of things should be opt-in, ala MOZ_QUIET.

The only problem is that I'm not sure how to satisfy conflicting desires here:

- automation wants as few messages as possible, given that B2G requires it to avoid timing out;
- (some?) local developers want as many messages as possible, to facilitate debugging.

Both of these paths go through automationutils et al, so we can't set anything there.  The easiest course I can see is to have a --log-all-messages flag (wordshedding welcome), but that's more typing for people.  Anybody else have ideas that I'm missing here?

Comment 11

[Andrew Halberstadt [:ahal]]

Actually mochitests don't use automation.py/automationutils anymore. Some of the functionality that used to live there is now in runtests.py, but most of it is in the various components of mozbase.

Comment 12

[Andrew Halberstadt [:ahal]]

Also, it would be really easy to opt-in to summarization in automation if there was a command line argument to do it. We'd just need to add it to the various mozharness scripts, e.g:

http://mxr.mozilla.org/build/source/mozharness/configs/unittests/linux_unittest.py#54
http://mxr.mozilla.org/build/source/mozharness/configs/b2g/emulator_automation_config.py#59
http://mxr.mozilla.org/build/source/mozharness/configs/android/androidarm.py#202

Comment 13

[Nathan Froyd [:froydnj]]

(In reply to Andrew Halberstadt [:ahal] from comment #12)
> Also, it would be really easy to opt-in to summarization in automation if
> there was a command line argument to do it. We'd just need to add it to the
> various mozharness scripts, e.g:
> 
> http://mxr.mozilla.org/build/source/mozharness/configs/unittests/
> linux_unittest.py#54
> http://mxr.mozilla.org/build/source/mozharness/configs/b2g/
> emulator_automation_config.py#59
> http://mxr.mozilla.org/build/source/mozharness/configs/android/androidarm.
> py#202

Ah, ok, didn't know about those.  Forgot that there are separate "run tests" for automation and developers.  Should we worry about those getting used for developers in Some Great Automation Unification down the road, or should we just cross that bridge when we come to it?  I guess we can just go ahead and do it and somebody will yell if it breaks down the road.

Comment 14

[Nathan Froyd [:froydnj]]

Attachment: introduce a --quiet option for mochitests

OK, here's a patch that adds --quiet and makes not-quiet (that is, print all
TEST-PASS, TEST-INFO, etc.) the default.

What's the magic for updating mozharness along with this?  I guess Andrew
hasn't backed out the B2G-only mochitest summarization bits, so landing this
and having it run without the mozharness bits (and/or riding the trains with
it?) won't cause anything to fall over, just run slightly slower...

Comment 15

[(not currently active) Ted Mielczarek]

n.b. Mochitest still uses automationutils.py (I'd like to move that all into Mozbase at some point).

Comment 16

[Andrew Halberstadt [:ahal]]

(In reply to Nathan Froyd (:froydnj) from comment #14)
> Created attachment 8387676 [details] [diff] [review]
> introduce a --quiet option for mochitests
> 
> OK, here's a patch that adds --quiet and makes not-quiet (that is, print all
> TEST-PASS, TEST-INFO, etc.) the default.
> 
> What's the magic for updating mozharness along with this?  I guess Andrew
> hasn't backed out the B2G-only mochitest summarization bits, so landing this
> and having it run without the mozharness bits (and/or riding the trains with
> it?) won't cause anything to fall over, just run slightly slower...

Unfortunately I did back it out. I'll whip up a mozharness patch that we can land before landing your patch that should prevent any bustage.

Comment 17

[Andrew Halberstadt [:ahal]]

Bah, forgot this is going to be difficult to do since the --quiet command won't exist on older branches. We really need to get those mozharness configs in-tree :(.

I think our best options are to either:
A) land a dummy --quiet option that does nothing on all the older branches
B) switch back to the opt-out method and make the mach targets pass in the out-out by default
C) block on getting configs in mozharness and spend some time nailing this down

(In reply to Nathan Froyd (:froydnj) from comment #13)
> Should we worry about those getting used for
> developers in Some Great Automation Unification down the road, or should we
> just cross that bridge when we come to it?  I guess we can just go ahead and
> do it and somebody will yell if it breaks down the road.

Yeah, we eventually want to merge the mach/mozharness methods of running tests together. I would say let's cross that road when we come to it though.

Comment 18

[Andrew Halberstadt [:ahal]]

I filed bug 981030 for option C.. but if this bug is time sensitive I wouldn't recommend blocking on it.

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8387676 [details] [diff] [review]
introduce a --quiet option for mochitests

Review of attachment 8387676 [details] [diff] [review]:
-----------------------------------------------------------------

that is a lot of places to add code just for a single parameter to the test runner!

Comment 20

[Andrew Halberstadt [:ahal]]

Bug 981030 is finished now!

You can just add --quiet to all of the 'mochitest_options' keys in these files:
http://mxr.mozilla.org/mozilla-central/source/testing/config/mozharness/

No need for landing on older branches anymore.

Comment 21

[Nathan Froyd [:froydnj]]

Attachment: part 2 - add --quiet option to all mochitest configurations

ahal++ for moving these things in-tree!  Your reward: reviewing a patch
to add --quiet to all these shiny new configs.

Comment 22

[Andrew Halberstadt [:ahal]]

Comment on attachment 8397238 [details] [diff] [review]
part 2 - add --quiet option to all mochitest configurations

Review of attachment 8397238 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm :)

Comment 23

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7cecd865ea6
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8608d5e26ff

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c7cecd865ea6
https://hg.mozilla.org/mozilla-central/rev/c8608d5e26ff