Bug 39748 - Clean up IndexedDB layout tests
Summary: Clean up IndexedDB layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-26 10:08 PDT by Jeremy Orlow
Modified: 2010-05-27 06:42 PDT (History)
2 users (show)

See Also:

Attachments
Patch (14.12 KB, patch)
2010-05-26 10:11 PDT, Jeremy Orlow 		no flags Details | Formatted Diff | Diff
Patch (14.10 KB, patch)
2010-05-27 05:41 PDT, Jeremy Orlow 		no flags Details | Formatted Diff | Diff
Patch (14.82 KB, patch)
2010-05-27 06:33 PDT, Jeremy Orlow 		steveblock: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2010-05-26 10:08:20 PDT 
Clean up IndexedDB layout tests
Comment 1 Jeremy Orlow 2010-05-26 10:11:28 PDT 
Created attachment 57111 [details]
Patch
Comment 2 Andrei Popescu 2010-05-27 04:37:24 PDT 
I think you have some indentation errors in LayoutTests/storage/indexeddb/resources/shared.js : lines 34, 43 and 52.

Also, do you need a new line at the end of file?
Comment 3 Jeremy Orlow 2010-05-27 05:41:42 PDT 
Created attachment 57227 [details]
Patch
Comment 4 Andrei Popescu 2010-05-27 05:47:12 PDT 
LGTM
Comment 5 Steve Block 2010-05-27 06:10:56 PDT 
Comment on attachment 57227 [details]
Patch

LayoutTests/storage/indexeddb/resources/shared.js:4
 +          layoutTestController.dumpAsText();
This isn't needed - it's in js-test-pre.js

LayoutTests/storage/indexeddb/resources/shared.js:67
 +  \ No newline at end of file
newline

LayoutTests/storage/indexeddb/resources/shared.js:8
 +      shouldBeTrue("'indexedDB' in window");
Is this needed here? It seems like it should be tested in the fast/dom/Window/window-properties* tests.

If we remove this, we can remove the init() function and just call layoutTestController.waitUntilDone() directly.
Comment 6 Jeremy Orlow 2010-05-27 06:19:10 PDT 
(In reply to comment #5)
> (From update of attachment 57227 [details])
> LayoutTests/storage/indexeddb/resources/shared.js:4
>  +          layoutTestController.dumpAsText();
> This isn't needed - it's in js-test-pre.js

done

 
> LayoutTests/storage/indexeddb/resources/shared.js:67
>  +  \ No newline at end of file
> newline

done
 
> LayoutTests/storage/indexeddb/resources/shared.js:8
>  +      shouldBeTrue("'indexedDB' in window");
> Is this needed here? It seems like it should be tested in the fast/dom/Window/window-properties* tests.

Since it's behind a compile flag, the window properties tests skip over indexedDB..and that'll be true until the majority of the tests enable it.  Also tests like dom storage run this check every single time.  But I guess we could split it into its own test. 
 
> If we remove this, we can remove the init() function and just call layoutTestController.waitUntilDone() directly.

...with the if statement, but OK...we can do that.
Comment 7 Jeremy Orlow 2010-05-27 06:33:58 PDT 
Created attachment 57236 [details]
Patch
Comment 8 Steve Block 2010-05-27 06:37:10 PDT 
Comment on attachment 57236 [details]
Patch

LayoutTests/storage/indexeddb/script-tests/idb-database-request.js:3
 +      layoutTestController.waitUntilDone();
Actually, I meant that you could leave this call in shared.js, but not in a function, so as to avoid the need to call that function from each test's JS file.
Comment 9 Jeremy Orlow 2010-05-27 06:42:56 PDT 
Committed r60299: <http://trac.webkit.org/changeset/60299>