Bug 81440 - extract logic of idb tests
Summary: extract logic of idb tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Grogan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-16 18:31 PDT by David Grogan
Modified: 2012-03-26 20:51 PDT (History)
3 users (show)

See Also:


Attachments
Patch (683.49 KB, patch)
2012-03-16 18:32 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch (672.32 KB, patch)
2012-03-22 17:35 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch (1023.14 KB, patch)
2012-03-23 13:31 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch (1023.79 KB, patch)
2012-03-23 13:52 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch (991.02 KB, patch)
2012-03-23 15:48 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch for landing (939.83 KB, patch)
2012-03-26 15:37 PDT, David Grogan
no flags Details | Formatted Diff | Diff
Patch for landing (939.83 KB, patch)
2012-03-26 16:49 PDT, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2012-03-16 18:31:23 PDT
extract logic of idb tests
Comment 1 David Grogan 2012-03-16 18:32:14 PDT
Created attachment 132434 [details]
Patch
Comment 2 David Grogan 2012-03-16 18:36:01 PDT
not ready for review yet, just FYI
Comment 3 David Grogan 2012-03-22 17:35:35 PDT
Created attachment 133399 [details]
Patch
Comment 4 David Grogan 2012-03-23 13:31:07 PDT
Created attachment 133549 [details]
Patch
Comment 5 David Grogan 2012-03-23 13:52:39 PDT
Created attachment 133551 [details]
Patch
Comment 6 David Grogan 2012-03-23 13:55:03 PDT
Comment on attachment 133551 [details]
Patch

Josh, this is ready for a look.
Comment 7 Joshua Bell 2012-03-23 14:47:26 PDT
Comment on attachment 133551 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133551&action=review

> LayoutTests/ChangeLog:14
> +        The first three simulate mouse events and the last isn't really a test.

structured-clone.html uses mouse events too (to create File/FileList objects, although those tests are currently disabled). Should it be skipped too?

That test also uses a few more things like ImageData that workers won't be able to create. We may want to split that test up or make it validate both window and worker cases, passing the cloned values to the worker. (values-odd-types.html does similar work, but is probably a redundant subset now)

> LayoutTests/storage/indexeddb/createObjectStore-name-argument-required.html:-1
> -<!DOCTYPE html>

Just removing the DOCTYPE preamble for consistency? In IE this is used to trigger "standards mode", I don't know if WebKit actually does anything with it. It shouldn't matter either way for the test. It's strictly required by the HTML5 spec, though - http://dev.w3.org/html5/spec/single-page.html#syntax-doctype

> LayoutTests/storage/indexeddb/list-ordering.html:-7
> -<input type="file" id="fileInput" multiple></input>

Oops, thanks for deleting this.
Comment 8 Joshua Bell 2012-03-23 14:47:43 PDT
... and otherwise lgtm
Comment 9 David Grogan 2012-03-23 15:35:23 PDT
(In reply to comment #7)
> (From update of attachment 133551 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133551&action=review
> 
> > LayoutTests/ChangeLog:14
> > +        The first three simulate mouse events and the last isn't really a test.
> 
> structured-clone.html uses mouse events too (to create File/FileList objects, although those tests are currently disabled). Should it be skipped too?
> 
> That test also uses a few more things like ImageData that workers won't be able to create. We may want to split that test up or make it validate both window and worker cases, passing the cloned values to the worker. (values-odd-types.html does similar work, but is probably a redundant subset now)

Structured cloning on workers seems ripe for corner cases so I want to do one of your suggestions, or something intelligent, but will skip it for now.

> > LayoutTests/storage/indexeddb/createObjectStore-name-argument-required.html:-1
> > -<!DOCTYPE html>
> 
> Just removing the DOCTYPE preamble for consistency?

Just for consistency.  All the html files were generated from a single template.

In IE this is used to trigger "standards mode", I don't know if WebKit actually does anything with it. It shouldn't matter either way for the test. It's strictly required by the HTML5 spec, though - http://dev.w3.org/html5/spec/single-page.html#syntax-doctype
> 
> > LayoutTests/storage/indexeddb/list-ordering.html:-7
> > -<input type="file" id="fileInput" multiple></input>
> 
> Oops, thanks for deleting this.
Comment 10 David Grogan 2012-03-23 15:48:12 PDT
Created attachment 133576 [details]
Patch
Comment 11 David Grogan 2012-03-23 17:55:20 PDT
Tony, could you review this?

I'm not sure why win-ews can't apply the patch.  win_layout_rel applied it without problems.
http://build.chromium.org/p/tryserver.chromium/builders/win_layout_rel/builds/152/steps/update/logs/stdio
Comment 12 Tony Chang 2012-03-26 09:24:04 PDT
Comment on attachment 133576 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133576&action=review

rs=me

> LayoutTests/ChangeLog:29
> +        * storage/indexeddb/constants.html:
> +        * storage/indexeddb/create-and-remove-object-store.html:
> +        * storage/indexeddb/create-object-store-options.html:
> +        * storage/indexeddb/createObjectStore-name-argument-required.html:
> +        * storage/indexeddb/createObjectStore-null-name.html:
> +        * storage/indexeddb/cursor-continue.html:
> +        * storage/indexeddb/cursor-delete.html:

It's OK to trim this.  You could just list the important files (e.g., the two -expected.txt files where line numbers changed).
Comment 13 David Grogan 2012-03-26 15:37:55 PDT
Created attachment 133907 [details]
Patch for landing
Comment 14 WebKit Review Bot 2012-03-26 16:41:21 PDT
Attachment 133907 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/stor..." exit_code: 1
LayoutTests/ChangeLog:26:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 245 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 WebKit Review Bot 2012-03-26 16:43:39 PDT
Comment on attachment 133907 [details]
Patch for landing

Rejecting attachment 133907 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12148089
Comment 16 David Grogan 2012-03-26 16:49:24 PDT
Created attachment 133931 [details]
Patch for landing
Comment 17 WebKit Review Bot 2012-03-26 20:51:27 PDT
Comment on attachment 133931 [details]
Patch for landing

Clearing flags on attachment: 133931

Committed r112202: <http://trac.webkit.org/changeset/112202>
Comment 18 WebKit Review Bot 2012-03-26 20:51:37 PDT
All reviewed patches have been landed.  Closing bug.