Bug 44939

Summary:

Web Inspector: move debugger tests to the new test harness

Product:

WebKit

Reporter:

Pavel Podivilov <podivilov>

Component:

Web Inspector (Deprecated)

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bweinstein, commit-queue, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch.	yurys: review-, yurys: commit-queue-
Proposed patch.	yurys: review+
Proposed patch.	none
Proposed patch.	none
Proposed patch.	none

Pavel Podivilov

Reported 2010-08-31 03:07:10 PDT

Web Inspector: move debugger tests to the new test harness

Attachments
Proposed patch. (53.25 KB, patch) 2010-08-31 03:08 PDT, Pavel Podivilov	yurys: review- yurys: commit-queue-	Details Formatted Diff Diff
Proposed patch. (58.24 KB, patch) 2010-08-31 09:17 PDT, Pavel Podivilov	yurys: review+	Details Formatted Diff Diff
Proposed patch. (61.27 KB, patch) 2010-09-01 03:46 PDT, Pavel Podivilov	no flags	Details Formatted Diff Diff
Proposed patch. (61.57 KB, patch) 2010-09-01 03:50 PDT, Pavel Podivilov	no flags	Details Formatted Diff Diff
Proposed patch. (60.65 KB, patch) 2010-09-01 03:53 PDT, Pavel Podivilov	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Pavel Podivilov

Comment 1 2010-08-31 03:08:04 PDT

Created attachment 66026 [details] Proposed patch.

Yury Semikhatsky

Comment 2 2010-08-31 05:17:42 PDT

Comment on attachment 66026 [details] Proposed patch. btw, what's the point in switching debugger tests to inspector-tests2.js? > LayoutTests/http/tests/inspector/inspector-test2.js:164 > + dumpedResultsCount = results.length; Please keep this counter on the front-end(or even get rid of it completely) and slice results array before passing it here. > LayoutTests/inspector/debugger-cyclic-ref-expected.txt:4 > -Set timer for test function. Why is this gone? > LayoutTests/inspector/debugger-cyclic-ref.html:13 > - notifyDone(); This should be called after we left debugger code, you cannot simply invoke this code from the front-end(see the change that introduced this line). r- for that. > LayoutTests/inspector/debugger-eval-while-paused.html:12 > - notifyDone(); This should be reverted. > LayoutTests/inspector/debugger-no-nested-pause.html:11 > - if (testFunction.didExitFromNestedCall) Now the test will pass even if testFunction is executed only once.

Pavel Podivilov

Comment 3 2010-08-31 09:17:19 PDT

Created attachment 66063 [details] Proposed patch.

Pavel Podivilov

Comment 4 2010-08-31 09:27:23 PDT

(In reply to comment #2) > (From update of attachment 66026 [details]) > btw, what's the point in switching debugger tests to inspector-tests2.js? The point was to fully support tests that require page reloading. Also, other tests are easier with new test harness. > > > LayoutTests/http/tests/inspector/inspector-test2.js:164 > > + dumpedResultsCount = results.length; > Please keep this counter on the front-end(or even get rid of it completely) and slice results array before passing it here. done > > > LayoutTests/inspector/debugger-cyclic-ref-expected.txt:4 > > -Set timer for test function. > Why is this gone? restored > > > LayoutTests/inspector/debugger-cyclic-ref.html:13 > > - notifyDone(); > This should be called after we left debugger code, you cannot simply invoke this code from the front-end(see the change that introduced this line). r- for that. > > > LayoutTests/inspector/debugger-eval-while-paused.html:12 > > - notifyDone(); > This should be reverted. InspectorTest.completeDebuggerTest always waits for WebInspector.resumedScript call before closing inspector. WebInspector.resumedScript is called from InspectorDebuggerAgent::didContinue which is outside nested message loop. So it should be safe to use completeDebuggerTest function. > > > LayoutTests/inspector/debugger-no-nested-pause.html:11 > > - if (testFunction.didExitFromNestedCall) > Now the test will pass even if testFunction is executed only once. We dump invocationCount at the end of the test, so the test won't actually pass in that case.

Yury Semikhatsky

Comment 5 2010-08-31 09:56:45 PDT

Comment on attachment 66063 [details] Proposed patch. > LayoutTests/http/tests/inspector/inspector-test2.js:145 > +function onload() I'd rather keep the old name which was more descriptive. > LayoutTests/inspector/extensions-api.html:15 > +<body onload="onload()"> please revert this

Yury Semikhatsky

Comment 6 2010-08-31 09:57:34 PDT

Comment on attachment 66063 [details] Proposed patch. Please revert onload->runTest renaming before landing.

Pavel Podivilov

Comment 7 2010-09-01 03:46:38 PDT

Created attachment 66192 [details] Proposed patch.

Pavel Podivilov

Comment 8 2010-09-01 03:50:29 PDT

Created attachment 66193 [details] Proposed patch.

Pavel Podivilov

Comment 9 2010-09-01 03:53:36 PDT

Created attachment 66194 [details] Proposed patch.

WebKit Commit Bot

Comment 10 2010-09-01 05:13:40 PDT

Comment on attachment 66194 [details] Proposed patch. Clearing flags on attachment: 66194 Committed r66596: <http://trac.webkit.org/changeset/66596>

WebKit Commit Bot

Comment 11 2010-09-01 05:13:45 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.