Bug 27963 – Worker layout tests should be refactored to enable testing SharedWorkers also

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Andrew Wilson]

We have a bunch of worker layout tests - we should refactor them to enable running the same tests on SharedWorkers without duplicating lots of code.

[Andrew Wilson]

Created attachment 34020 [details]
patch with updated layout tests

[David Levin]

Comment on attachment 34020 [details]
patch with updated layout tests

> diff --git a/LayoutTests/fast/workers/resources/shared-worker-common.js b/LayoutTests/fast/workers/resources/shared-worker-common.js
>  function handleMessage(event, port) {
>      if (event.data == "ping")
>          port.postMessage("PASS: Received ping message");
>      else if (event.data == "done")
>          port.postMessage("DONE");
> -    else if (/eval.+/.test(evt.data)) {
> +    else if (/eval.+/.test(event.data)) {
>          try {
> -            port.postMessage(evt.data.substr(5) + ": " + eval(evt.data.substr(5)));
> +            port.postMessage(event.data.substr(5) + ": " + eval(event.data.substr(5)));
>          } catch (ex) {
> -            port.postMessage(evt.data.substr(5) + ": " + ex);
> +            port.postMessage(event.data.substr(5) + ": " + ex);

It seems like it would have been better to fix this in https://bugs.webkit.org/show_bug.cgi?id=27927 where you are adding the file... but at this point not worth changing both patches (which is maybe what you thought as well).


Note that this needs https://bugs.webkit.org/show_bug.cgi?id=27927 to land first since it changes files from that patch.

[Eric Seidel (no email)]

Comment on attachment 34020 [details]
patch with updated layout tests

Do you know about the JS testing harness?

fast/js/resources
the TEMPLATE.html files
make-js-test-wrappers

Gives you a bunch of stuff, including debug() and shouldBe, shouldBeEqual, etc.  In general we write most new tests using this framework (yes, it needs to be better documented and put in a better location than fast/js/resources).

[Andrew Wilson]

(In reply to comment #2)

> It seems like it would have been better to fix this in
> https://bugs.webkit.org/show_bug.cgi?id=27927 where you are adding the file...
> but at this point not worth changing both patches (which is maybe what you
> thought as well).
> 

Yeah, I figured it wasn't worth going back to fix code that wasn't used in any tests.

> 
> Note that this needs https://bugs.webkit.org/show_bug.cgi?id=27927 to land
> first since it changes files from that patch.

Yeah, I put "Depends on: 27927" in the bug - should I do something more?

>Do you know about the JS testing harness?

I saw that code when I was working on some of the window global constructor tests. I'm happy to use it in future tests - are you saying new tests should include ../js/resources/xxxxx.js?

[Andrew Wilson]

Committed as r46852.