Bug 44939 - Web Inspector: move debugger tests to the new test harness
Summary: Web Inspector: move debugger tests to the new test harness
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-31 03:07 PDT by Pavel Podivilov
Modified: 2010-09-01 05:13 PDT (History)
10 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2010-08-31 03:07:10 PDT
Web Inspector: move debugger tests to the new test harness
Comment 1 Pavel Podivilov 2010-08-31 03:08:04 PDT
Created attachment 66026 [details]
Proposed patch.
Comment 2 Yury Semikhatsky 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.
Comment 3 Pavel Podivilov 2010-08-31 09:17:19 PDT
Created attachment 66063 [details]
Proposed patch.
Comment 4 Pavel Podivilov 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.
Comment 5 Yury Semikhatsky 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
Comment 6 Yury Semikhatsky 2010-08-31 09:57:34 PDT
Comment on attachment 66063 [details]
Proposed patch.

Please revert onload->runTest renaming before landing.
Comment 7 Pavel Podivilov 2010-09-01 03:46:38 PDT
Created attachment 66192 [details]
Proposed patch.
Comment 8 Pavel Podivilov 2010-09-01 03:50:29 PDT
Created attachment 66193 [details]
Proposed patch.
Comment 9 Pavel Podivilov 2010-09-01 03:53:36 PDT
Created attachment 66194 [details]
Proposed patch.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2010-09-01 05:13:45 PDT
All reviewed patches have been landed.  Closing bug.