Web Inspector: add a sanity test for storage panel
Created attachment 66718 [details] Patch
Created attachment 66720 [details] Patch
Comment on attachment 66718 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66718&action=prettypatch > LayoutTests/inspector/storage-panel-show.html:27 > + function printStorageEntries(storage, entries) { Looks like this function is not used.
Created attachment 66721 [details] Patch
(In reply to comment #3) > (From update of attachment 66718 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=66718&action=prettypatch > > > LayoutTests/inspector/storage-panel-show.html:27 > > + function printStorageEntries(storage, entries) { > Looks like this function is not used. Thanks. Removed the function.
Comment on attachment 66721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66721&action=prettypatch > LayoutTests/inspector/storage-panel-show-expected.txt:1 > +Test that storage panel is present and that it contains correct datat for local and session DOM storages. Typo: "datat" => "data" > LayoutTests/inspector/storage-panel-show.html This tests only the DOM Storage aspect of the Storage panel. Maybe this file name is a bit too general, unless there are immediate plans to test other parts of the panel (Databases, Cookies, Application Caches). > LayoutTests/inspector/storage-panel-show.html:12 > + localStorage["_a" + i] = i; > + sessionStorage["_b" + i] = i+10; These lines are a bit misleading, since they are actually stored as strings. I think eventually they will be stored as Structured Clones. Should be fine, but something to keep in mind. > WebKit/chromium/src/js/Tests.js:-224 > -TestSuite.prototype.testHostIsPresent = function() > -TestSuite.prototype.testElementsTreeRoot = function() > -TestSuite.prototype.testMainResource = function() A bit weird removing these tests, as they have nothing to do with this patch. Probably would have been better to have its own patch. However, there is nothing really wrong with this patch, and its a good test =). Do you agree with any of the comments above?
Created attachment 66849 [details] Patch
Thanks for reviewing this! (In reply to comment #6) > (From update of attachment 66721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=66721&action=prettypatch > > > LayoutTests/inspector/storage-panel-show-expected.txt:1 > > +Test that storage panel is present and that it contains correct datat for local and session DOM storages. > > Typo: "datat" => "data" > Done. > > LayoutTests/inspector/storage-panel-show.html > > This tests only the DOM Storage aspect of the Storage panel. Maybe > this file name is a bit too general, unless there are immediate > plans to test other parts of the panel (Databases, Cookies, > Application Caches). > Renamed the test to storage-panel-dom-storage.html > > LayoutTests/inspector/storage-panel-show.html:12 > > + localStorage["_a" + i] = i; > > + sessionStorage["_b" + i] = i+10; > > These lines are a bit misleading, since they are actually stored as strings. I think > eventually they will be stored as Structured Clones. Should be fine, but something > to keep in mind. > Changed the values to strings. This way ability to store JS objects in the dom storage shouldn't affect this test. > > > WebKit/chromium/src/js/Tests.js:-224 > > -TestSuite.prototype.testHostIsPresent = function() > > -TestSuite.prototype.testElementsTreeRoot = function() > > -TestSuite.prototype.testMainResource = function() > > A bit weird removing these tests, as they have nothing to do with this patch. > Probably would have been better to have its own patch. > Ok, excluded this change from this patch. I was too lazy to file a new bug just too remove obsolete code.
Comment on attachment 66849 [details] Patch Thanks for those making those changes. r=me as long as those key/value pair orderings will always happen. It would be bad if the order changed in some iterations causing a flakey test. You could probably just check using --iterations.
(In reply to comment #9) > (From update of attachment 66849 [details]) > Thanks for those making those changes. > > r=me as long as those key/value pair orderings will always happen. > It would be bad if the order changed in some iterations causing > a flakey test. You could probably just check using --iterations. The order was steady for 100 iterations.
Comment on attachment 66849 [details] Patch Rejecting patch 66849 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 20939 test cases. animations/combo-transform-translate+scale.html -> failed Exiting early after 1 failures. 99 tests run. 20.27s total testing time 98 test cases (98%) succeeded 1 test case (1%) had incorrect layout Full output: http://queues.webkit.org/results/3914322
Comment on attachment 66849 [details] Patch Clearing flags on attachment: 66849 Committed r67067: <http://trac.webkit.org/changeset/67067>
All reviewed patches have been landed. Closing bug.