Make SimpleTest.is and variants less verbose on pass

RESOLVED FIXED in Firefox 17

Status

Testing
Mochitest
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smontagu, Assigned: smontagu)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 667925 [details] [diff] [review]
Don't output expected results on pass

A large percentage of mochitest logs consist of "foo should equal foo" (where foo is often quite lengthy) outputted by SimpleTest.is(). We certainly don't need the two identical copies of the test result, and I question whether we even need one when the test has passed.

A mochitest-4 log with this patch took up ~36MB, as opposed to ~50MB without the patch.
Attachment #667925 - Flags: review?(jmaher)
Comment on attachment 667925 [details] [diff] [review]
Don't output expected results on pass

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

Is there anyway we could make this configurable?  It is nice to know that we are passing something and by removing the description of what we are testing, the output becomes useless.  What is of most concern in a log file is finding what went wrong and obtaining enough information from the log itself to diagnose the problem.  Since this patch allows us to do that, I am fine going with an r+.   

We should ask ourselves if we need to output anything on a pass (even the "XXXXXX TEST-PASS | ...") as we could really trim up our logs if needed.  A slippery slope and making it configurable would help ensure we are not forgetting about the pros and cons of doing this.

As a bonus this should make our IO for android testing less which is a really good thing!
Attachment #667925 - Flags: review?(jmaher) → review+
Duplicate of this bug: 797645

Updated

5 years ago
Assignee: nobody → smontagu
(In reply to Joel Maher (:jmaher) from comment #1)
> We should ask ourselves if we need to output anything on a pass (even the
> "XXXXXX TEST-PASS | ...") as we could really trim up our logs if needed.

FWIW -- in my experience, the TEST-PASS lines make it easier to establish the context for crashes/hangs, so I'd advocate against removing TEST-PASS lines entirely.

e.g. suppose a test tests something on all of our CSS properties, and when it gets to a particular one, we occasionally will hang/crash.  In the logs that experience the hang/crash, it's much more useful to see something like

>  TEST-INFO | starting test test_allTheThings.html
>  [...]
>  TEST-PASS | property 'n' behaved like we expected
> ***CRASH*** (or hang/no more output/whatever)

...rather than just...

>  TEST-INFO | starting test test_allTheThings.html
> ***CRASH*** (or hang/no more output/whatever)
(in that case, property 'n+1' would presumably be the buggy one)

I suppose if the TEST-PASS presence/complete-absence were configurable, we could turn it off in the majority of cases, and then turn it back on eagerly if we need it.  The downside is, our output would be mysterious (like the latter chunk in comment 3) the first time we hit a sporadic failure, and we'd need someone to take action (configure it on) in order to figure out more... but maybe that's worth it for reduced log-sizes.
dholbert++
Depends on: 798062
Blocks: 798062
No longer depends on: 798062
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d30e007d711f

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/d30e007d711f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Comment 8

5 years ago
Landed on mozilla17 with the blanket a=test-only to reduce log size for esr17, which will be with us until December 2013 (yey):
https://hg.mozilla.org/releases/mozilla-beta/rev/e94f91b6473c
status-firefox17: --- → fixed
You need to log in before you can comment on or make changes to this bug.