jit_test.py and jstests.py don't show any output from their tests by default, even failed ones, and unfortunately this also swallows any assertion failure that a test may have reported before crashing. Although test failures are reported, this isn't very helpful if you can't reproduce them locally. The following patches 1) extend jit_test.py so that it is able to display the output of only failed tests (like jstests.py with -o -F), 2) switch jit_test.py and jstests.py to display the output of failed tests by default, and add a '--no-show-failed' switch to disable this behavior.

This adds '--failed-only' and '--no-show-failed' to js/src/jit-test/jit_test.py and its backend js/src/tests/lib/jittests.py and changes the default behavior to display the output of failed tests. Since its check_output() reports a binary result, this doesn't differentiate between regressions and timeouts. I don't know if that's a problem or if showing the output for timeouts is desirable as well (I figure we mostly care about assertion failures, but maybe knowing how far a test got before being killed could be useful too).

This changes the behavior of jstests.py in the same way, except that it doesn't display output for timeouts by default (--show-output --failed-only still does). The conditionals got pretty hairy; I wanted to make sure you could still get the previous behavior. In particular, --show-cmd --failed-only --no-show-failed is the new --show-cmd --failed-only.

Oh, part 1 also adds some missing pb.beginline() so that the output doesn't start right after the progress bar.

Let's see how badly this blows up on try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c7f791a983f4

\o/

jstests in the browser (jsreftest) use a different harness, so won't be affected by this bug. If and when we start running them in the shell (bug 1101662), that will be affected.

Comment on attachment 8525599 [details] [diff] [review] Part 1: Show the output of failed jit-tests unless '--no-show-failed' is passed. Review of attachment 8525599 [details] [diff] [review]: ----------------------------------------------------------------- So I only looked at the stdout/stderr logic. I'm assuming that the command line output for --failed-only --show-cmd is correct? (As in, it should only display command lines for failures?) Even if not, r=me for this patch. ::: js/src/tests/lib/jittests.py @@ +647,5 @@ > + show_output = False > + if options.show_output and (not ok or not options.failed_only): > + show_output = True > + if not ok and not options.no_show_failed: > + show_output = True The one weird case is if you pass both --show-output and --no-show-failed. Does that mean you want to see failures' output or not? If not, then I think this would be simpler as: if ok: show_output = options.show_output and not options.failed_only else: show_output = not options.no_show_failed but I think it's probably better to show in that case (it doesn't seem useful to see only successful output), so matching your logic I'd probably say: if ok: show_output = options.show_output and not options.failed_only else: show_output = options.show_output or not options.no_show_failed Then again, I could also see: if options.show_output: show_output = not (ok and options.fail_only) elif not ok: show_output = not options.no_show_failed Hopefully I got all that right. :-)

Comment on attachment 8525602 [details] [diff] [review] Part 2: Show the output of failed jstests unless '--no-show-failed' is passed. Review of attachment 8525602 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/tests/lib/results.py @@ +130,5 @@ > self.groups.setdefault(dev_label, []).append(result.test.path) > > + show_output = False > + if self.options.show_output and (dev_label in ('REGRESSIONS', 'TIMEOUTS') or not self.options.failed_only): > + show_output = True This reads better to me the other way around: if self.options.show_output and (not self.options.failed_only or dev_label in ('REGRESSIONS', 'TIMEOUTS')): mostly because "failed_only" is understandable immediately and so it's easier to figure out that the dev_label part means "if it failed". I guess you could do that explicitly: if self.options.show_output: if not self.options.failed_only or dev_label in...: but I don't like that as well. @@ +136,5 @@ > + show_output = True > + > + show_cmd = False > + if self.options.show_cmd and (dev_label in ('REGRESSIONS', 'TIMEOUTS') or not self.options.failed_only): > + show_cmd = True Flip this one around too.

Carrying forward r=sfink. (In reply to Steve Fink [:sfink, :s:] from comment #7) > So I only looked at the stdout/stderr logic. I'm assuming that the command > line output for --failed-only --show-cmd is correct? (As in, it should only > display command lines for failures?) As discussed on IRC, --show-cmd is handled up front for jit_test.py, so it doesn't factor into this logic. I adjusted the comment for --failed-only to reflect this. jit_test.py does have --show-failed-cmd, which should be unaffected by this change as well. > but I think it's probably better to show in that case (it doesn't seem > useful to see only successful output), so matching your logic I'd probably > say: > > if ok: > show_output = options.show_output and not options.failed_only > else: > show_output = options.show_output or not options.no_show_failed Done, I like that much better (especially how the conditionals line up).

Carrying forward r=sfink. I flipped the conditionals around to depend on the failure state, as in part 1 v2, which cleaned them up very nicely. (*) The default is to show the output only for 'regressions' (not timeouts), without showing the commandline. (*) To also show the commandline for both regressions and timeouts, run with --show-cmd --failed-only. (*) To show output for both regressions and timeouts, run with --show-output --failed-only. (*) To suppress all output, run with --no-show-failed. (*) To show only commandlines for regressions and timeouts, run with --show-cmd --failed-only --no-show-failed. (*) If --show-output is passed, --no-show-failed does nothing.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d9503e64da https://hg.mozilla.org/integration/mozilla-inbound/rev/74b738dc7837

https://hg.mozilla.org/mozilla-central/rev/d4d9503e64da https://hg.mozilla.org/mozilla-central/rev/74b738dc7837