Check that the output of all JITFLAGS is the same on jit-tests

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: terrence, Assigned: jj)

Tracking

Trunk
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: [lang=python])

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

3 years ago
As was done for the jsreftest suite in bug 637400, but for the jit-test suite.
Assignee

Updated

3 years ago
Whiteboard: [lang=python]
Assignee

Comment 1

3 years ago
If we look at jit-tests (js/src/jit-test/jit_tests.py) it calls jittests.run_test_remote and jittests.run_tests so we need to do check in the both the functions separately that the output for different flag is not different. Am I correct?
Flags: needinfo?(terrence)
Assignee

Comment 2

3 years ago
Posted patch try_v1.diff (obsolete) — Splinter Review
How about this one?
Attachment #8780642 - Flags: review?(terrence)
Reporter

Comment 3

3 years ago
Comment on attachment 8780642 [details] [diff] [review]
try_v1.diff

The patch file is empty. Don't forget to qref.
Flags: needinfo?(terrence)
Attachment #8780642 - Flags: review?(terrence)
Assignee

Comment 4

3 years ago
Posted patch try_v1.diff (obsolete) — Splinter Review
Attachment #8780642 - Attachment is obsolete: true
Attachment #8780710 - Flags: review?(terrence)
Assignee

Comment 5

3 years ago
I forgot to refresh the patch earlier :P
Reporter

Comment 6

3 years ago
Comment on attachment 8780710 [details] [diff] [review]
try_v1.diff

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

::: js/src/tests/lib/jittests.py
@@ +513,5 @@
> +                if res.test.path in output_dict.keys():
> +                    if output_dict[res.test.path] != res.out:
> +                        pb.message("FAIL - OUTPUT DIFFERS {}".format(res.test.relpath_tests))
> +                else:
> +                    output_dict[res.test.path] = res.out

This is almost perfect. However, by putting this above where we print the program output, it might not get seen by a reader. Instead let's move it down below to where we emit errors with pb.message. How about just above |doing = 'after {}'.format(res.test.relpath_tests)|?
Attachment #8780710 - Flags: review?(terrence)
Assignee

Comment 7

3 years ago
Posted patch try_v2.diffSplinter Review
Fixed it :)
Attachment #8780710 - Attachment is obsolete: true
Attachment #8781172 - Flags: review?(terrence)
Reporter

Comment 8

3 years ago
Comment on attachment 8781172 [details] [diff] [review]
try_v2.diff

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

Excellent!
Attachment #8781172 - Flags: review?(terrence) → review+
Reporter

Updated

3 years ago
Keywords: checkin-needed

Comment 9

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/772ec8c912ac
Check that the output of all JITFLAGS is the same on jit-tests. r=terrence
Keywords: checkin-needed

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/772ec8c912ac
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.