Bug 28939

Summary: Clean up localStorage.clear() layout test.
Product: WebKit Reporter: Jeremy Orlow <jorlow>
Component: New BugsAssignee: Jeremy Orlow <jorlow>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, beidson, benm, dglazkov, dimich, eric, fishd, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1 eric: review+, eric: commit-queue-

Jeremy Orlow
Reported 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.
Attachments
Patch v1 (8.98 KB, patch)
2009-09-03 02:08 PDT, Jeremy Orlow
eric: review+
eric: commit-queue-
Jeremy Orlow
Comment 1 2009-09-03 02:08:11 PDT
Created attachment 38975 [details] Patch v1
Jeremy Orlow
Comment 2 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!
Eric Seidel (no email)
Comment 3 2009-09-03 02:14:12 PDT
Comment on attachment 38975 [details] Patch v1 Maybe evalAndLog instead? Either way, this looks fine.
Jeremy Orlow
Comment 4 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!!!!!)
Eric Seidel (no email)
Comment 5 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. :)
Jeremy Orlow
Comment 6 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.
Adam Barth
Comment 7 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.
Jeremy Orlow
Comment 8 2009-09-07 22:19:22 PDT
Note You need to log in before you can comment on or make changes to this bug.