Closed
Bug 969437
Opened 10 years ago
Closed 9 years ago
add some way to disable mochitest test summarization for locally-run tests
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla31
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(2 files, 1 obsolete file)
7.38 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
9.03 KB,
patch
|
ahal
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
+1 to this. See also bug 973000.
Comment 3•9 years ago
|
||
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.
Flags: needinfo?(ahalberstadt)
![]() |
Assignee | |
Comment 4•9 years ago
|
||
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.
Attachment #8376453 -
Flags: feedback?(jmaher)
Attachment #8376453 -
Flags: feedback?(bobbyholley)
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
Attachment #8376453 -
Flags: feedback?(jmaher) → feedback+
![]() |
Assignee | |
Comment 6•9 years ago
|
||
(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•9 years ago
|
||
(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).
Flags: needinfo?(ahalberstadt)
Comment 8•9 years ago
|
||
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?
Attachment #8376453 -
Flags: feedback?(bobbyholley) → feedback+
![]() |
Assignee | |
Comment 9•9 years ago
|
||
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.
Attachment #8376453 -
Attachment is obsolete: true
![]() |
Assignee | |
Comment 10•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
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
![]() |
Assignee | |
Comment 13•9 years ago
|
||
(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.
![]() |
Assignee | |
Comment 14•9 years ago
|
||
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...
Attachment #8387676 -
Flags: review?(jmaher)
Comment 15•9 years ago
|
||
n.b. Mochitest still uses automationutils.py (I'd like to move that all into Mozbase at some point).
Comment 16•9 years ago
|
||
(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•9 years ago
|
||
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•9 years ago
|
||
I filed bug 981030 for option C.. but if this bug is time sensitive I wouldn't recommend blocking on it.
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!
Attachment #8387676 -
Flags: review?(jmaher) → review+
Comment 20•9 years ago
|
||
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.
![]() |
Assignee | |
Comment 21•9 years ago
|
||
ahal++ for moving these things in-tree! Your reward: reviewing a patch to add --quiet to all these shiny new configs.
Attachment #8397238 -
Flags: review?(ahalberstadt)
Comment 22•9 years ago
|
||
Comment on attachment 8397238 [details] [diff] [review] part 2 - add --quiet option to all mochitest configurations Review of attachment 8397238 [details] [diff] [review]: ----------------------------------------------------------------- Lgtm :)
Attachment #8397238 -
Flags: review?(ahalberstadt) → review+
![]() |
Assignee | |
Comment 23•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7cecd865ea6 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8608d5e26ff
Flags: in-testsuite+
![]() |
Assignee | |
Updated•9 years ago
|
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/c7cecd865ea6 https://hg.mozilla.org/mozilla-central/rev/c8608d5e26ff
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•