Bug 39748

Summary: Clean up IndexedDB layout tests
Product: WebKit Reporter: Jeremy Orlow <jorlow>
Component: New BugsAssignee: Jeremy Orlow <jorlow>
Status: RESOLVED FIXED    
Severity: Normal CC: andreip, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
Patch steveblock: review+

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>