Bug 104423 – [JSC] Add tests for explicit serialization values

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Michael Pruett]

Tests for explicit serialization values for V8 were added in r135022. Similar tests should be added for JSC's serialization format.

[Michael Pruett]

Created attachment 178372 [details]
Patch

[Dirk Pranke]

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.

[Michael Pruett]

Created attachment 178379 [details]
Patch

[Michael Pruett]

Created attachment 178380 [details]
Patch

[WebKit Review Bot]

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.

[Build Bot]

Comment on attachment 178380 [details]
Patch

Attachment 178380 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15219276

[Build Bot]

Comment on attachment 178380 [details]
Patch

Attachment 178380 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15218371

[Michael Pruett]

Created attachment 178382 [details]
Patch

[WebKit Review Bot]

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.

[Build Bot]

Comment on attachment 178382 [details]
Patch

Attachment 178382 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15218385

[Build Bot]

Comment on attachment 178382 [details]
Patch

Attachment 178382 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15191802

[Michael Pruett]

Created attachment 178385 [details]
Patch

[Alexey Proskuryakov]

JavaScript tests go to fast/js, please don't add another directory for those.

[Michael Pruett]

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.

[Alec Flett]

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.

[Michael Pruett]

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.

[WebKit Review Bot]

Comment on attachment 178663 [details]
Patch

Clearing flags on attachment: 178663

Committed r137346: <http://trac.webkit.org/changeset/137346>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.