Closed Bug 1474569 Opened 2 years ago Closed 2 years ago

Improve output for failing wpt tests in jstests harness

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch Patch v1Splinter Review
No description provided.
Attachment #8990962 - Flags: review?(bbouvier)
Comment on attachment 8990962 [details] [diff] [review]
Patch v1

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

Makes sense. Do you have an example of reporting in a try run? Thanks for the patch.

::: js/src/tests/lib/results.py
@@ +112,5 @@
>                  else:
>                      test_result = (cls.FAIL, test.message)
>                      result = cls.FAIL
> +                    test_output += "expected %s, found %s" % (expected, test.status)
> +                    if test.message:

Is message always guaranteed to exist, even if it's None/empty string? Otherwise, hasattr should be tested for here.
Attachment #8990962 - Flags: review?(bbouvier) → review+
Thanks!

(In reply to Benjamin Bouvier [:bbouvier] from comment #1)
> Comment on attachment 8990962 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 8990962 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Makes sense. Do you have an example of reporting in a try run? Thanks for
> the patch.

No, but locally it looks like this:

REGRESSION - wasm/jsapi/interface.any.js
[0|2|0|0]  40% =====================>                                 |   0.5s
## wasm/jsapi/interface.any.js: rc = 0, run time = 0.087217

Harness status: OK (Reported by harness: None)
Subtest "WebAssembly: property descriptor": expected FAIL, found PASS
Subtest "WebAssembly: constructing": as expected: PASS
Subtest "WebAssembly.validate": expected PASS, found FAIL (with message: "assert_true: enumerable expected true got false")


> ::: js/src/tests/lib/results.py
> @@ +112,5 @@
> >                  else:
> >                      test_result = (cls.FAIL, test.message)
> >                      result = cls.FAIL
> > +                    test_output += "expected %s, found %s" % (expected, test.status)
> > +                    if test.message:
> 
> Is message always guaranteed to exist, even if it's None/empty string?
> Otherwise, hasattr should be tested for here.

wpttest.SubtestResult.__init__ does always set it, yes.
Pushed by Ms2ger@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8c824521e7b
Improve output for failing wpt tests in jstests harness; r=bbouvier
Pushed by Ms2ger@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d11c83f1d68
Improve output for failing wpt tests in jstests harness; r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/9d11c83f1d68
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: needinfo?(Ms2ger)
Blocks: wasm-wpt
You need to log in before you can comment on or make changes to this bug.