Closed
Bug 1456119
Opened 6 years ago
Closed 6 years ago
Sort features in test262 update script
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
Details
Attachments
(2 files)
2.43 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
60.42 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
Similar to bug 1408583, we should sort the feature tests to reduce differences between different Python versions.
Assignee | ||
Comment 1•6 years ago
|
||
Use list comprehension instead of sets to avoid non-deterministic sort order of the feature lists.
Attachment #8970282 -
Flags: review?(sphink)
Assignee | ||
Comment 2•6 years ago
|
||
Rerun the python script to update the feature checks.
Attachment #8970286 -
Flags: review?(sphink)
Comment 3•6 years ago
|
||
Comment on attachment 8970282 [details] [diff] [review] bug1456119-part1-sorted.patch Review of attachment 8970282 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/tests/test262-update.py @@ +242,5 @@ > refTestSkip.append("not a test file") > > # Skip tests with unsupported features. > if "features" in testRec: > + unsupported = [f for f in testRec["features"] if f in UNSUPPORTED_FEATURES] What?! You're changing from an O(n) algorithm to an O(n**2)? What if there are 20 features? Do you have *any* idea how long it takes a computer to count up to 400? ...oh, you do, do you? Okay, this is fine, then.
Attachment #8970282 -
Flags: review?(sphink) → review+
Updated•6 years ago
|
Attachment #8970286 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] (PTO Apr9-12) from comment #3) > > if "features" in testRec: > > + unsupported = [f for f in testRec["features"] if f in UNSUPPORTED_FEATURES] > > What?! You're changing from an O(n) algorithm to an O(n**2)? What if there > are 20 features? Do you have *any* idea how long it takes a computer to > count up to 400? > > ...oh, you do, do you? > > Okay, this is fine, then. No worries, UNSUPPORTED_FEATURES is already a |set|, so we should still be in O(n). :-)
Comment 5•6 years ago
|
||
Doh! I'm not sure why I assumed it was a list. The fact that we were previously calling .intersection on it ought to have given me a rather large hint...
Assignee | ||
Comment 6•6 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ebdb48d52da55bfea32c787848b46ab047c7391
Keywords: checkin-needed
Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/12bfc2acf1d3 Part 1: Sort feature checks in test262 update script. r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/b66c3bf69b40 Part 2: Reimport tests after features were sorted. r=sfink
Keywords: checkin-needed
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/12bfc2acf1d3 https://hg.mozilla.org/mozilla-central/rev/b66c3bf69b40
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•