Bug 28939 - Clean up localStorage.clear() layout test.
Summary: Clean up localStorage.clear() layout test.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-03 01:57 PDT by Jeremy Orlow
Modified: 2009-09-07 22:19 PDT (History)
8 users (show)

See Also:


Attachments
Patch v1 (8.98 KB, patch)
2009-09-03 02:08 PDT, Jeremy Orlow
eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2009-09-03 01:57:17 PDT
This is the first step of https://bugs.webkit.org/show_bug.cgi?id=27897 which is a pretty major cleanup of all the dom storage layout tests.  I'm just changing this one to begin with to get buy-in on the approach.

This also adds a new method to the fast/js/js-test-pre.js that runs a command and prints it to the screen.  This should really help with making the output of the tests readable without looking at the source code.
Comment 1 Jeremy Orlow 2009-09-03 02:08:11 PDT
Created attachment 38975 [details]
Patch v1
Comment 2 Jeremy Orlow 2009-09-03 02:11:38 PDT
I've cc'ed everyone that I (think I) remember commenting about the dom storage layout tests at some point or another.  Please do take a quick look and let me know if you'd be happy r+ing patches of this style in the future.  Once we can all agree on it, I'll go ahead and convert the rest in subsequent patches/bugs.

Thanks!
Comment 3 Eric Seidel (no email) 2009-09-03 02:14:12 PDT
Comment on attachment 38975 [details]
Patch v1

Maybe evalAndLog instead?  Either way, this looks fine.
Comment 4 Jeremy Orlow 2009-09-03 02:28:03 PDT
evalAndLog seems better.  I think I might prefix error messages with something as well.  Maybe "Ran: " or "Executed: " or something?

I'll hold off committing until tomorrow evening in case there's more feedback.  (So please don't put this in the commit queue!!!!!)
Comment 5 Eric Seidel (no email) 2009-09-03 02:32:15 PDT
Comment on attachment 38975 [details]
Patch v1

Not ending up in the commit-queue is an easily solved problem. :)
Comment 6 Jeremy Orlow 2009-09-07 08:30:18 PDT
All right.  Hopefully silence means agreement.  :-)

I'll submit this (with the function name change) and start converting others tomorrow.  I'll probably do them in small chunks (1-3 tests per patch) to keep things easily reviewable.
Comment 7 Adam Barth 2009-09-07 08:46:19 PDT
One thing you might try is to commit this while folks are on the IRC channel.  That can be a good way to flush out opinions from folks who might not comment on the bug beforehand.
Comment 8 Jeremy Orlow 2009-09-07 22:19:22 PDT
Committed r48143: <http://trac.webkit.org/changeset/48143>