| Summary: | [JSC] Add tests for explicit serialization values | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Pruett <michael> | ||||||||||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | abarth, alecflett, ap, dbates, dpranke, jsbell, oliver, philn, webkit.review.bot, xan.lopez | ||||||||||||||||
| Priority: | P2 | ||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Michael Pruett
2012-12-07 18:09:31 PST
Created attachment 178372 [details]
Patch
Comment on attachment 178372 [details]
Patch
JSC isn't a platform, so it feels wrong to me to put this stuff under LayoutTests/platform. I suggest just using LayoutTests/jsc and modifying platform/chromium/TestExpectations to include a line like "jsc [ WontFix ]".
In particular, we don't want to modify default_baseline_search_path() to include "platform/jsc" at all, since it's not part of the search path in the sense of the other platform dirs.
Created attachment 178379 [details]
Patch
Created attachment 178380 [details]
Patch
Attachment 178380 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:4233: Path does not exist. [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:4234: Path does not exist. [test/expectations] [5]
Total errors found: 2 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178380 [details] Patch Attachment 178380 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15219276 Comment on attachment 178380 [details] Patch Attachment 178380 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15218371 Created attachment 178382 [details]
Patch
Attachment 178382 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:4233: Path does not exist. [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:4234: Path does not exist. [test/expectations] [5]
Total errors found: 2 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178382 [details] Patch Attachment 178382 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15218385 Comment on attachment 178382 [details] Patch Attachment 178382 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15191802 Created attachment 178385 [details]
Patch
JavaScript tests go to fast/js, please don't add another directory for those. Created attachment 178593 [details]
Patch
The V8 equivalent of this test is in platform/chromium/fast/storage, so I've placed the JSC test in fast/storage.
I don't suppose there's any way to share the code that's common in the tests between JSC and V8? Of course that makes port-specific fixes hard. Maybe have cross-references in the comments? i.e. "See LayoutTests/platform/chromium/fast/storage/serialized-script-value.js"? I defer to WebKit reviewers here, but as the author of the V8 side of the tests, I'd like to encourage collaboration and improvement over time here. Created attachment 178663 [details]
Patch
I've moved the utility functions common to both the V8 and JSC tests to a common source file. I've also added comments to both the V8 and JSC layout tests mentioning the existence of the other test.
Comment on attachment 178663 [details] Patch Clearing flags on attachment: 178663 Committed r137346: <http://trac.webkit.org/changeset/137346> All reviewed patches have been landed. Closing bug. |