Bug 104423

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	dpranke: review-
Patch	none
Patch	buildbot: commit-queue-
Patch	buildbot: commit-queue-
Patch	none
Patch	none
Patch	none

Michael Pruett

Reported 2012-12-07 18:09:31 PST

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

Attachments
Patch (26.59 KB, patch) 2012-12-08 12:45 PST, Michael Pruett	dpranke: review-	Details Formatted Diff Diff
Patch (25.46 KB, patch) 2012-12-08 14:06 PST, Michael Pruett	no flags	Details Formatted Diff Diff
Patch (25.46 KB, patch) 2012-12-08 14:11 PST, Michael Pruett	buildbot: commit-queue-	Details Formatted Diff Diff
Patch (25.78 KB, patch) 2012-12-08 15:39 PST, Michael Pruett	buildbot: commit-queue-	Details Formatted Diff Diff
Patch (25.50 KB, patch) 2012-12-08 16:18 PST, Michael Pruett	no flags	Details Formatted Diff Diff
Patch (25.68 KB, patch) 2012-12-10 11:02 PST, Michael Pruett	no flags	Details Formatted Diff Diff
Patch (31.31 KB, patch) 2012-12-10 16:21 PST, Michael Pruett	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Michael Pruett

Comment 1 2012-12-08 12:45:05 PST

Created attachment 178372 [details] Patch

Dirk Pranke

Comment 2 2012-12-08 13:32:34 PST

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

Comment 3 2012-12-08 14:06:55 PST

Created attachment 178379 [details] Patch

Michael Pruett

Comment 4 2012-12-08 14:11:52 PST

Created attachment 178380 [details] Patch

WebKit Review Bot

Comment 5 2012-12-08 14:14:54 PST

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 6 2012-12-08 14:22:33 PST

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 7 2012-12-08 14:44:45 PST

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

Michael Pruett

Comment 8 2012-12-08 15:39:13 PST

Created attachment 178382 [details] Patch

WebKit Review Bot

Comment 9 2012-12-08 15:40:47 PST

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 10 2012-12-08 15:41:33 PST

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 11 2012-12-08 16:06:47 PST

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

Michael Pruett

Comment 12 2012-12-08 16:18:36 PST

Created attachment 178385 [details] Patch

Alexey Proskuryakov

Comment 13 2012-12-08 20:29:19 PST

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

Michael Pruett

Comment 14 2012-12-10 11:02:48 PST

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

Comment 15 2012-12-10 11:22:13 PST

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

Comment 16 2012-12-10 16:21:36 PST

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 17 2012-12-11 12:59:41 PST

Comment on attachment 178663 [details] Patch Clearing flags on attachment: 178663 Committed r137346: <http://trac.webkit.org/changeset/137346>

WebKit Review Bot

Comment 18 2012-12-11 12:59:46 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.