Bug 44251 - Web Inspector: add DOM breakpoints test
Summary: Web Inspector: add DOM breakpoints test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-19 05:09 PDT by Pavel Podivilov
Modified: 2010-08-23 02:50 PDT (History)
2 users (show)

See Also:


Attachments
Proposed patch. (7.00 KB, patch)
2010-08-19 05:11 PDT, Pavel Podivilov
yurys: review-
Details | Formatted Diff | Diff
Proposed patch. (17.51 KB, patch)
2010-08-20 09:47 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
All comments addressed. (17.46 KB, patch)
2010-08-23 01:33 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.