Bug 44251 – Web Inspector: add DOM breakpoints test

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Pavel Podivilov]

Web Inspector: add DOM breakpoints test

[Pavel Podivilov]

Created attachment 64829 [details]
Proposed patch.

[Yury Semikhatsky]

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.

[Pavel Podivilov]

Created attachment 64962 [details]
Proposed patch.

[Pavel Podivilov]

(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

[Yury Semikhatsky]

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.

[Pavel Podivilov]

Created attachment 65088 [details]
All comments addressed.

[Yury Semikhatsky]

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.

[WebKit Commit Bot]

Comment on attachment 65088 [details]
All comments addressed.

Clearing flags on attachment: 65088

Committed r65799: <http://trac.webkit.org/changeset/65799>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.