Summary: | WebAssembly: import GCC torture tests | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | JF Bastien <jfbastien> | ||||||||||||
Component: | JavaScriptCore | Assignee: | JF Bastien <jfbastien> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, fpizlo, jfbastien, keith_miller, mark.lam, msaboff, saam | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 159775, 170970 | ||||||||||||||
Attachments: |
|
Description
JF Bastien
2017-04-11 10:59:24 PDT
Created attachment 307429 [details]
patch
Attachment 307429 [details] did not pass style-queue:
ERROR: Tools/Scripts/update-wasm-gcc-torture.py:10: trailing whitespace [pep8/W291] [5]
ERROR: Tools/Scripts/update-wasm-gcc-torture.py:13: trailing whitespace [pep8/W291] [5]
ERROR: Tools/Scripts/update-wasm-gcc-torture.py:148: indentation is not a multiple of four [pep8/E111] [5]
Total errors found: 3 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 307430 [details]
patch
Fix style.
Created attachment 307432 [details]
patch
Rebase.
Comment on attachment 307432 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=307432&action=review > Tools/Scripts/run-jsc-stress-tests:1212 > + prepareExtraRelativeFiles([Pathname('..') + wasm], $collection) Should this be "[(Pathname('..') + wasm).to_s]" instead of "[Pathname('..') + wasm]"? (In reply to Mark Lam from comment #5) > Comment on attachment 307432 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307432&action=review > > > Tools/Scripts/run-jsc-stress-tests:1212 > > + prepareExtraRelativeFiles([Pathname('..') + wasm], $collection) > > Should this be "[(Pathname('..') + wasm).to_s]" instead of "[Pathname('..') > + wasm]"? What I have now works because, IIUC, `Pathname + String => Pathname` and prepareExtraRelativeFiles expects at least the extra files or the destination to be a Pathname. As long as one of them is a Pathname we're fine: it concatenates both! Emscripten was being noisy in its print error log. This fix should help: https://github.com/kripken/emscripten/pull/5144 Comment on attachment 307432 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=307432&action=review Nice. r=me > Tools/ChangeLog:3 > + WebAssembly: import GCC torture tests Nit: I'd make the bug title reflect what the patch actually does. > Tools/Scripts/update-wasm-gcc-torture.py:96 > +def waterfall_known_failures(args): What determines if something is a failure? If it's just that v8 fails that test, perhaps that's not the metric we want to use. Created attachment 307485 [details] patch > > Tools/ChangeLog:3 > > + WebAssembly: import GCC torture tests > > Nit: I'd make the bug title reflect what the patch actually does. Fixed. > > Tools/Scripts/update-wasm-gcc-torture.py:96 > > +def waterfall_known_failures(args): > > What determines if something is a failure? If it's just that v8 fails that > test, perhaps that's not the metric we want to use. The waterfall runs all these tests on V8, spec interpreter, and jsc. The expected failures file lists which test flavor it expects to fail and on which engine. The Mac waterfall turns red if jsc tests start failing. We just happen to be on par with V8 :) The line `if 'emwasm' not in attributes` is the one that distinguishes this particular build of the torture tests. IMO the other two flavors (bare and musl) are kinda obsolete now and have served their prior purpose wells I just ignored them here. If jsc and V8 diverged then the attributes would also say "jsc" or "V8", but then we'd need to differentiate which engine fails on which build flavor. Let's just not diverge instead ;) Comment on attachment 307485 [details] patch Rejecting attachment 307485 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 307485, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Tools/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/3564373 Created attachment 307487 [details]
patch
One day I won't forget to update the OOPS. Today is not that day.
Comment on attachment 307487 [details] patch Clearing flags on attachment: 307487 Committed r215519: <http://trac.webkit.org/changeset/215519> All reviewed patches have been landed. Closing bug. |