Bug 44251

Summary: Web Inspector: add DOM breakpoints test
Product: WebKit Reporter: Pavel Podivilov <podivilov>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch.
yurys: review-
Proposed patch.
none
All comments addressed. none

Description Pavel Podivilov 2010-08-19 05:09:51 PDT
Web Inspector: add DOM breakpoints test
Comment 1 Pavel Podivilov 2010-08-19 05:11:32 PDT
Created attachment 64829 [details]
Proposed patch.
Comment 2 Yury Semikhatsky 2010-08-20 07:06:07 PDT
Comment on attachment 64829 [details]
Proposed patch.

LayoutTests/platform/qt/Skipped:109
 +  inspector/dom-breakpoints.html
It should be skipped on Gtk too, right?

LayoutTests/http/tests/inspector/inspector-test2.js:54
 +  InspectorTest.waitUntilPaused = function(callback)
Would be nice if we could install these methods only for tests that need them like we currently do with console/elements/debugger... tests

LayoutTests/http/tests/inspector/inspector-test2.js:209
 +  function runTestInFrontend(initializeInspectorTest, completeTestCallId, testFunction)
Please move it inside runTest to not confuse people that could try to invoke this function in the inspected page.

LayoutTests/inspector/dom-breakpoints.html:20
 +          InspectorTest.addResult("Found div element.");
This test is going to fail id debugger is not always enabled, r- for that.
Comment 3 Pavel Podivilov 2010-08-20 09:47:10 PDT
Created attachment 64962 [details]
Proposed patch.
Comment 4 Pavel Podivilov 2010-08-20 09:49:52 PDT
(In reply to comment #2)
> (From update of attachment 64829 [details])
> LayoutTests/platform/qt/Skipped:109
>  +  inspector/dom-breakpoints.html
> It should be skipped on Gtk too, right?
> 
> LayoutTests/http/tests/inspector/inspector-test2.js:54
>  +  InspectorTest.waitUntilPaused = function(callback)
> Would be nice if we could install these methods only for tests that need them like we currently do with console/elements/debugger... tests
> 
> LayoutTests/http/tests/inspector/inspector-test2.js:209
>  +  function runTestInFrontend(initializeInspectorTest, completeTestCallId, testFunction)
> Please move it inside runTest to not confuse people that could try to invoke this function in the inspected page.
> 
> LayoutTests/inspector/dom-breakpoints.html:20
>  +          InspectorTest.addResult("Found div element.");
> This test is going to fail id debugger is not always enabled, r- for that.

all done
Comment 5 Yury Semikhatsky 2010-08-22 23:58:13 PDT
Comment on attachment 64962 [details]
Proposed patch.

LayoutTests/inspector/debugger-breakpoints-not-activated-on-reload.html:46
 +          WebInspector.panels.scripts.toggleBreakpointsButton.element.click();
Use InspectorTest.resumeExecution instead, we may need to merge it into  InspectorTest.completeDebuggerTest();

LayoutTests/http/tests/inspector/inspector-test2.js:127
 +      if (window.initializeDebuggerTest)
Please iterate over all functions matching a naming convention of the test initialization functions(like frontend_*) and collect them here instead of making inspector-test2.js know about all kind of tests.

LayoutTests/http/tests/inspector/inspector-test2.js:111
 +          window.InspectorTest = {};
You don't need window. prefix here and below.
Comment 6 Pavel Podivilov 2010-08-23 01:33:20 PDT
Created attachment 65088 [details]
All comments addressed.
Comment 7 Yury Semikhatsky 2010-08-23 01:59:21 PDT
Comment on attachment 65088 [details]
All comments addressed.

LayoutTests/http/tests/inspector/debugger-test2.js:24
 +          scriptsPanel.toggleBreakpointsButton.element.click();
We may need to move this code into DRT which would reset inspector settings before each test.
Comment 8 WebKit Commit Bot 2010-08-23 02:50:21 PDT
Comment on attachment 65088 [details]
All comments addressed.

Clearing flags on attachment: 65088

Committed r65799: <http://trac.webkit.org/changeset/65799>
Comment 9 WebKit Commit Bot 2010-08-23 02:50:25 PDT
All reviewed patches have been landed.  Closing bug.