js-test-post.js is not robust for asynchronous tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Steve Block]

LayoutTests that use the script-tests pattern use a standard HTML template. This includes fast/js/resources/js-test-pre.js, followed by the script for the test itself, followed by fast/js/resources/js-test-post.js.

If window.jsTestIsAsync has been set by the test, js-test-post.js calls layoutTestController.waitUntilDone(). The test should then call finishJSTest() when it completes. finishJSTest() is defined in js-test-post.js and calls layoutTestController.notifyDone().

However, it's possible for an asynchronous test to complete before js-test-post.js has been parsed. This can occur when the loading of this script is slow, causing WebCore to yield during loading and process asynchronous callbacks from the test. In this case, finishJSTest() is not yet defined, causing the test to fail.

One solution would be to update all such asynchronous tests to delay any testing until onload has fired. A less intrusive alternative is to move finishJSTest() to js-test-pre.js and add logic to js-test-post.js to handle the case of finishJSTest() having already been called.

[Steve Block]

Created attachment 65493 [details]
Patch

[Darin Adler]

Comment on attachment 65493 [details]
Patch

The change seems fine.

> +isAllScriptParsed = true;

I think that adding this new window property might affect tests that dump everything on the window object. Did you run all the scripts to test?

I would name this variable postTestScriptWasParsed or wasPostTestScriptParsed.

> +    if (window.isFinishJSTestCalled)

I would name this variable finishJSTestWasCalled or wasFinishJSTestCalled.

[Steve Block]

> I think that adding this new window property might affect tests that dump everything on the window
> object. Did you run all the scripts to test?
Yes, I ran all tests. I guess none of them dump the window object asynchronously, ie after js-test-post.js has been parsed.

> I would name this variable postTestScriptWasParsed or wasPostTestScriptParsed.
> I would name this variable finishJSTestWasCalled or wasFinishJSTestCalled.
Will do

[Steve Block]

Committed r66201: <http://trac.webkit.org/changeset/66201>

[WebKit Review Bot]

http://trac.webkit.org/changeset/66201 might have broken GTK Linux 32-bit Release

[Steve Block]

Added GTK, Qt and Win expected results in http://trac.webkit.org/changeset/66212