Bug 97051 - IndexedDB: Rewrite confusing call sequence layout tests
Summary: IndexedDB: Rewrite confusing call sequence layout 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: Joshua Bell
URL:
Keywords:
Depends on:
Blocks: 97368 97482
  Show dependency treegraph
 
Reported: 2012-09-18 16:07 PDT by Joshua Bell
Modified: 2012-09-27 12:56 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.60 KB, patch)
2012-09-18 16:07 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (54.79 KB, patch)
2012-09-19 14:07 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-09-18 16:07:32 PDT
IndexedDB: Rewrite confusing call sequence layout tests
Comment 1 Joshua Bell 2012-09-18 16:07:50 PDT
Created attachment 164634 [details]
Patch
Comment 2 Joshua Bell 2012-09-18 16:14:16 PDT
This is a work in progress - I want to go through and rewrite two of the layout tests that push IDB through a series of states. The tests (which I wrote when I was a n00b!) are written in a fairly confusing style using a wrapper object around the IDB API. This makes them difficult to maintain and understand.

I plan to leave the intent of the tests intact and ensure that the tests still push IDB through the same set of states (opening/closing/versioning/deleting) databases, but with simpler, more "idiomatic" code, and eventually break them out into separate files.

The two tests are:

factory-deletedatabase-interactions.html
open-close-version.html

I've tackled the first sub-test of open-close-version.html, attached. The output differs by tweaking the database name (a change), using evalAndLog() for API calls (more spammy), adding assertions about sequence (the ++state thingy), and removing the summary "concate the states together and make sure the strings match" which was confusing and redundant.

If this looks acceptable I'll do the rest to the other sub-tests, land, then do a clean-up pass removing a lot of the extraneous debug() calls, land, then finally split the tests up into separate files.
Comment 3 Joshua Bell 2012-09-19 14:07:13 PDT
Created attachment 164774 [details]
Patch
Comment 4 Joshua Bell 2012-09-19 14:32:19 PDT
The latest patch goes ahead and converts all of those tests over. The *-expected.txt diffs are minimized to show that the tests themselves are doing the same thing as previously. Once this has landed I'll split them into separate files, update the output, and eliminate some of the nested functions to make them easier to read.
Comment 5 Joshua Bell 2012-09-19 14:32:33 PDT
dgrogan@, alecflett@ - please take a look
Comment 6 Alec Flett 2012-09-19 15:18:19 PDT
lgtm
Comment 7 Joshua Bell 2012-09-19 16:39:31 PDT
tony@ - test only changes - r? cq?
Comment 8 WebKit Review Bot 2012-09-20 10:59:58 PDT
Comment on attachment 164774 [details]
Patch

Clearing flags on attachment: 164774

Committed r129142: <http://trac.webkit.org/changeset/129142>
Comment 9 WebKit Review Bot 2012-09-20 11:00:01 PDT
All reviewed patches have been landed.  Closing bug.