Print something more useful after the filename when a jittest fails

RESOLVED FIXED in mozilla30

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: philor, Assigned: terrence)

Tracking

Trunk
mozilla30
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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)

Comment 1

5 years ago
Created attachment 8373484 [details] [diff] [review]
make_jittest_output_more_useful-v0.diff
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+
(Assignee)

Comment 3

5 years ago
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
(Reporter)

Comment 4

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