Bug 27491 - Web Sockets Test Infrastructure Part 2/3: Patch to run-webkit-tests
Summary: Web Sockets Test Infrastructure Part 2/3: Patch to run-webkit-tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-21 01:34 PDT by Yuzo Fujishima
Modified: 2009-11-03 23:50 PST (History)
4 users (show)

See Also:


Attachments
Patch to run-webkit-test to start up / shut down Web Socket Server. (4.91 KB, patch)
2009-07-21 01:55 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Patch to run-webkit-test to use Web Socket Server for both http and file schemes. (5.40 KB, patch)
2009-08-04 03:24 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (5.89 KB, patch)
2009-10-20 02:55 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (6.17 KB, patch)
2009-10-20 18:16 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (6.16 KB, patch)
2009-10-20 22:34 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (6.11 KB, patch)
2009-10-20 22:37 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (6.16 KB, patch)
2009-10-25 23:22 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Start/Stop Web Socket and Web Socket Secure servers for layout tests. (6.17 KB, patch)
2009-11-03 18:18 PST, Yuzo Fujishima
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuzo Fujishima 2009-07-21 01:34:49 PDT
Add test infrastructure for Web Sockets (http://www.w3.org/TR/websockets/).

This bug is Part 2/3: Patch to run-webkit-tests.

Other parts are:
Part 1/3: Server
Part 3/3: Example test
Parts must be submitted in the order Part1, Part2, then Part3.
Comment 1 Yuzo Fujishima 2009-07-21 01:38:27 PDT
Links:

Part 1/3: Server: https://bugs.webkit.org/show_bug.cgi?id=27490
Part 2/3: Patch to run-webkit-tests: https://bugs.webkit.org/show_bug.cgi?id=27491
Part 3/3: Example test: https://bugs.webkit.org/show_bug.cgi?id=27492
Comment 2 Yuzo Fujishima 2009-07-21 01:55:34 PDT
Created attachment 33159 [details]
Patch to run-webkit-test to start up / shut down Web Socket Server.
Comment 3 Alexey Proskuryakov 2009-07-22 10:18:22 PDT
Running WebSocket tests from file:// would mean that all requests will be cross-origin. It seems that we should want to test both same- and cross-origin requests.
Comment 4 Yuzo Fujishima 2009-07-22 23:39:13 PDT
Hi, Alexey,

Thank you for your review.

I'm thinking of implementing
  window.websocketTestController.setOrigin(webSocket, originString)
just like window.layoutTestController offers various test functionalities.

Do you think it feasible? Your advice would be appreciated.

Yuzo
Comment 5 Alexey Proskuryakov 2009-07-23 07:47:12 PDT
I think that it may be non-trivial to add such hooks in a way that would not be distinguishable from the real thing. Also, such tests wouldn't work in browser, making debugging more complicated.

Making WebSockets tests work together with http tests would be more reliable.
Comment 6 Yuzo Fujishima 2009-08-04 03:24:07 PDT
Created attachment 34055 [details]
Patch to run-webkit-test to use Web Socket Server for both http and file schemes.

This should enable both same-origin (http://) and cross-origin(file://) testing.
Comment 7 Eric Seidel (no email) 2009-08-07 12:57:00 PDT
Comment on attachment 34055 [details]
Patch to run-webkit-test to use Web Socket Server for both http and file schemes.

Please abstract:
8         my $testPath = "$testDirectory/$test";
 639         if (isCygwin()) {
 640             $testPath = toWindowsPath($testPath);
 641         } else {
 642             $testPath = canonpath($testPath);
 643         }

This is bad practice:
 1474     mkdir "/tmp/WebKit";

It make it harder to run more than one copy in parallel.  Better to use a ~ relative path or at least a unique name.  I'm certain there are other examples of such in run-webkit-tests.

That's a huge amount of code to make sure the server is dead.  Don't we already have some of that for the httpd server?  Can't we share that?

r- for the needed abstraction.
Comment 8 Yuzo Fujishima 2009-10-20 02:55:25 PDT
Created attachment 41492 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 9 Yuzo Fujishima 2009-10-20 02:59:35 PDT
Can anyone review this patch?

This changes the run-webkit-test script such that Web Socket (Secure) servers
added via Bug 27490 are started and stopped for tests under LayoutTests/websocket.

Yuzo
Comment 10 Eric Seidel (no email) 2009-10-20 12:08:32 PDT
Comment on attachment 41492 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.

I do not understand the minus lines in run-webkit-tests.

You need a more detailed ChangeLog giving more information about what you're doing here.
Comment 11 Yuzo Fujishima 2009-10-20 18:16:50 PDT
Created attachment 41540 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 12 Yuzo Fujishima 2009-10-20 18:20:26 PDT
Hi, Eric,

Thank you for the review.

I've added explanation to WebKitTools/ChangeLog.

FYI:
The minus-ed lines has been moved to the end of the if-elsif statement.

Yuzo
Comment 13 Yuzo Fujishima 2009-10-20 22:34:31 PDT
Created attachment 41547 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 14 Yuzo Fujishima 2009-10-20 22:37:00 PDT
Created attachment 41548 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 15 Fumitoshi Ukai 2009-10-25 21:48:27 PDT
I've one question.
If I want to write a test with script-tests that uses LayoutTests/fast/js/, what should I do?
For now, websocket server sets document root to LayoutTests/websocket/tests, so the test can't access js files in LayoutTests/fast/js/.
Should I copy LayoutTests/fast/js/ into LayoutTests/websocket/tests/js ?

(same question for LayoutTests/http/tests)

Thanks in advance.
Comment 16 Yuzo Fujishima 2009-10-25 23:22:20 PDT
Created attachment 41845 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 17 Yuzo Fujishima 2009-10-25 23:25:17 PDT
Hi,

I've changed the document root to LayoutTests.
This should address https://bugs.webkit.org/show_bug.cgi?id=27491#c15

Reviewers and committers,
please commit this after https://bugs.webkit.org/show_bug.cgi?id=30763
This script change now requires pywebsocket 0.4.1

Yuzo
Comment 18 Yuzo Fujishima 2009-10-27 00:28:58 PDT
Hi, reviewers,

Now that pywebsocket has been upgraded to 0.4.1, please resume reviewing this patch. :)

Yuzo
Comment 19 Yuzo Fujishima 2009-10-29 17:58:09 PDT
Ping?
Comment 20 David Levin 2009-11-02 11:39:45 PST
Comment on attachment 41845 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.

Just one issue.

> diff --git a/WebKitTools/Scripts/run-webkit-tests b/WebKitTools/Scripts/run-webkit-tests
> @@ -604,6 +603,32 @@ for my $test (@tests) {
>              }
>              print OUT "$testPath$suffixExpectedHash\n";
>          }
> +    } elsif ($test =~ /^websocket\//) {
> +        openWebSocketServerIfNeeded();

It seems like this should be in the "else" below because some of these test may run without needing the web socket server started.

> +        if ($test =~ /^websocket\/tests\/local\//) {
> +            my $testPath = "$testDirectory/$test";
> +            if (isCygwin()) {
> +                $testPath = toWindowsPath($testPath);
> +            } else {
> +                $testPath = canonpath($testPath);
> +            }
> +            print OUT "$testPath\n";
> +        } else {
> +            my $path = canonpath($test);
> +            if ($test =~ /^websocket\/tests\/ssl\//) {
> +                print OUT "https://127.0.0.1:$webSocketSecurePort/$path\n";
> +            } else {
> +                print OUT "http://127.0.0.1:$webSocketPort/$path\n";
> +            }
> +        }
Comment 21 Yuzo Fujishima 2009-11-03 18:18:03 PST
Created attachment 42440 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Comment 22 Yuzo Fujishima 2009-11-03 18:19:32 PST
Hi, David,

Thank you for the review. I've changed the code as suggested.
Can you take another look?

Yuzo
Comment 23 Fumitoshi Ukai 2009-11-03 23:50:38 PST
Committed r50500: <http://trac.webkit.org/changeset/50500>