797827 - Make SimpleTest.is and variants less verbose on pass

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Simon Montagu :smontagu]

Attachment: 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.

Comment 1

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

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!

Comment 3

[Daniel Holbert [:dholbert]]

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

Comment 4

[Daniel Holbert [:dholbert]]

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

Comment 5

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

dholbert++

Comment 6

[Simon Montagu :smontagu]

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

Comment 7

[Ed Morley [:emorley]]

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

Comment 8

[Ed Morley [:emorley]]

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