Closed Bug 970071 Opened 6 years ago Closed 6 years ago

Print something more useful after the filename when a jittest fails

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: philor, Assigned: terrence)

Details

Attachments

(1 file)

When a jittest fails, we print "TEST-UNEXPECTED-FAIL | filename.js | --commandline-options".

I understand that sometimes the commandline options are a significant factor in the failure, that it's always failing with --ion-eager and that's An Clue, but...

* often it isn't, and putting them there says to people starring and filing that they should (though they mostly won't, and mostly shouldn't) file a new bug if there is a bug for "--baseline-eager --no-ti --no-fpu" but their failure was on "--baseline-eager --no-ti".

* tbpl's "I found a filename and should search for all failures in that filename" detection depends on there being something after the second pipe, so "TEST-UNEXPECTED-FAIL | filename.js | --commandline-options" is a failure in filename.js and the suggestions are every intermittent-failure bug with filename.js in the summary, but "TEST-UNEXPECTED-FAIL | filename.js |" (which we also run, no options, so we also hit) is not, it's a no-idea-what-test thing which calls for searching for the full line of the failure, which means that unlike every single other failure which clearly has a filename in it, jittest failure bugs must be filed with the "TEST-UNEXPECTED-FAIL" and the full path in the summary, and they maybe don't work even then.

Judging by the other stuff output around the failure line in the logs, the harness knows whether it was a timeout or some other sort of failure, which seems like a very significant bit of information, and our minds tend to wander pretty quickly while starring, so I'd expect that

TEST-UNEXPECTED-FAIL | filename.js | Timeout (with "--no-ti")
TEST-UNEXPECTED-FAIL | filename.js | Timeout (with "--no-ti --no-fpu")
TEST-UNEXPECTED-FAIL | filename.js | Assertion failure: 0 == rv (with "")

would get the desired result, one bug on filename.js timing out, and one bug on filename.js having an assertion failure.
Assignee: nobody → terrence
Status: NEW → ASSIGNED
Attachment #8373484 - Flags: review?(jorendorff)
Comment on attachment 8373484 [details] [diff] [review]
make_jittest_output_more_useful-v0.diff

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

::: js/src/tests/lib/jittests.py
@@ +383,5 @@
>      # INFO stdout          > baz
>      # INFO stderr         2> TypeError: or something
>      # TEST-UNEXPECTED-FAIL | jit_test.py: Test execution interrupted by user
> +    result = "TEST-PASS" if ok else "TEST-UNEXPECTED-FAIL"
> +    message = "Success" if ok else res.descript_failure()

Spelling ("descript")
Attachment #8373484 - Flags: review?(jorendorff) → review+
Will teach me to make last minute name changes. Retested locally for good measure, should be goodness.

https://hg.mozilla.org/integration/mozilla-inbound/rev/97037c9b3c8c
https://hg.mozilla.org/mozilla-central/rev/97037c9b3c8c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.