Summary: | Web Sockets Test Infrastructure Part 2/3: Patch to run-webkit-tests | ||
---|---|---|---|
Product: | WebKit | Reporter: | Yuzo Fujishima <yuzo> |
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ap, mike, ukai, yuzo |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
Description
Yuzo Fujishima
2009-07-21 01:34:49 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 Created attachment 33159 [details]
Patch to run-webkit-test to start up / shut down Web Socket Server.
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. 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 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. 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 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.
Created attachment 41492 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
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 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.
Created attachment 41540 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
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 Created attachment 41547 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Created attachment 41548 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
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. Created attachment 41845 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
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 Hi, reviewers, Now that pywebsocket has been upgraded to 0.4.1, please resume reviewing this patch. :) Yuzo Ping? 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"; > + } > + } Created attachment 42440 [details]
Start/Stop Web Socket and Web Socket Secure servers for layout tests.
Hi, David, Thank you for the review. I've changed the code as suggested. Can you take another look? Yuzo Committed r50500: <http://trac.webkit.org/changeset/50500> |