Closed
Bug 1506352
Opened 7 years ago
Closed 7 years ago
Stop running web-platform-tests in jstests.py until we have a working strategy for updates
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: jgraham, Assigned: Ms2ger)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
9.72 KB,
patch
|
bbouvier
:
review+
jgraham
:
review+
|
Details | Diff | Splinter Review |
We started running some wasm tests from web-platform-tests in the jsshell in Bug 1333800. However we didn't properly consider how things would work when the tests are updated; a wpt update introducing a test that doesn't pass currently breask the build. There are two things that would help:
* Start producing a wptreport.json log and run the jsshell tests as part of the wpt update process, so we are able to update the metadata to match the observed failures on import.
* Don't run the jsshell tests as part of the build, but as a seperate job. IN particular make the wpt part of this tier 2 so that failures can be fixed asynchronously rather than requiring a backout.
Before we can do one or both of those things we need to reconsider running these tests by default.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → Ms2ger
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #9026169 -
Flags: review?(james)
Attachment #9026169 -
Flags: review?(bbouvier)
Reporter | ||
Comment 2•7 years ago
|
||
Comment on attachment 9026169 [details] [diff] [review]
Patch v1
Review of attachment 9026169 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/tests/lib/results.py
@@ +56,5 @@
> CRASH = 'CRASH'
>
> """Classified result from a test run."""
>
> + def __init__(self, test, result, results, wpt_results=None):
I wonder if it's possible to just pass the formatter in here rather than have the handler abstraction, but I don't have a strong opinion.
@@ +127,5 @@
> results.append(test_result)
> stdout.append(test_output)
>
> output.out = "\n".join(stdout) + "\n"
> +
Whitespace
::: js/src/tests/lib/wptreport.py
@@ +51,5 @@
> + * "status": the actual status of the whole test;
> + * "expected": the expected status of the whole test;
> + * "subtests": a list of dicts with keys "test",
> + "subtest", "status" and "expected".
> + :param float duration: the runtime of the test
Also whitespace
Attachment #9026169 -
Flags: review?(james) → review+
Comment 3•7 years ago
|
||
Comment on attachment 9026169 [details] [diff] [review]
Patch v1
Review of attachment 9026169 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/tests/jstests.py
@@ +202,5 @@
> type='choice', choices=['automation', 'none'],
> help='Output format. Either automation or none'
> ' (default %default).')
> + output_og.add_option('--log-wptreport', dest='wptreport', action='store',
> + help='Path to write a wptreport output file')
nit: Can you explicit `wptreport` into `a Web Platform test report`, so there's no doubt for people who read this for the first time?
::: js/src/tests/lib/results.py
@@ +245,5 @@
> self.n += 1
> else:
> result = TestResult.from_output(output)
> +
> + if self.wptreport and result.wpt_results:
Check with `is not None`, to avoid surprising results?
@@ +335,5 @@
> self.slog.suite_end()
> else:
> self.list(completed)
>
> + if self.wptreport:
ditto?
Attachment #9026169 -
Flags: review?(bbouvier) → review+
Pushed by Ms2ger@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5931ccb19e50
Add a --log-wptreport option to jstests; r=bbouvier,jgraham
Pushed by Ms2ger@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55d9c8696101
Followup: Add missing blank line to placate flake8.
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5931ccb19e50
https://hg.mozilla.org/mozilla-central/rev/55d9c8696101
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•