Bug 27963

Summary: Worker layout tests should be refactored to enable testing SharedWorkers also
Product: WebKit Reporter: Andrew Wilson <atwilson>
Component: WebCore JavaScriptAssignee: Andrew Wilson <atwilson>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 27927    
Bug Blocks:    
Attachments:
Description Flags
patch with updated layout tests levin: review+

Description Andrew Wilson 2009-08-03 16:51:04 PDT
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.
Comment 1 Andrew Wilson 2009-08-03 16:54:14 PDT
Created attachment 34020 [details]
patch with updated layout tests
Comment 2 David Levin 2009-08-04 08:59:28 PDT
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.
Comment 3 Eric Seidel (no email) 2009-08-04 09:03:11 PDT
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).
Comment 4 Andrew Wilson 2009-08-04 10:14:58 PDT
(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?
Comment 5 Andrew Wilson 2009-08-06 11:26:24 PDT
Committed as r46852.