Bug 31390 – Implement run-webkit-websocketserver

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Fumitoshi Ukai]

run-webkit-tests launches websocket server if we tries to test LayoutTests/websocket/tests/

We have a run-webkit-httpd script for debugging HTTP tests in Safari, maybe we need something similar for WebSocket.

[Yuzo Fujishima]

Created attachment 43033 [details]
Add to run-webkit-tests an option to run Web Socket server

[Yuzo Fujishima]

Hi, reviewers,

Can you review this patch?

This is a Web Socket server counterpart for run-webkit-httpd script.

Rather than writing a separate script, I chose to add an option to run-webkit-tests.
This way it is easy to run the server in the same environment as that for LayoutTests.

Yuzo

[David Levin]

This seems really odd to me. Just like there is a "run-webkit-httpd" as you noted, it seems like there should be a separate script for this.

[Yuzo Fujishima]

Hi, thank you for the comment.

It may seem odd, but I think it is better than repeating the same/similar
pieces of code in run-webkit-tests and run-webkit-websocket-server or so.

(Because the purpose is testing, although manual, using run-webkit-tests
for this purpose should not be that odd.)

I admit I have very limited knowledge about Perl. If it is easy to "import" 
run-webkit-tests, maybe I should import it to a new script.

Yuzo

[Yuzo Fujishima]

Created attachment 43137 [details]
Add run-webkit-websocketserver

[Yuzo Fujishima]

OK, I've created another patch, which adds a dedicated script,
run-webkit-websocketserver.

If you prefer this dedicated script approach to a new option approach,
that's fine with me.

Can you review the patch(es) and submit whichever you like, assuming
it looks good?

Yuzo

[Alexey Proskuryakov]

Ideally, we should share code between run-webkit-tests and run-webkit-websocketserver. We should do this for httpd, as well.

Given that the test server seems to not work on Windows or Tiger, maybe we should investigate those failures first, and refactor launching code once we know what goes wrong? I'm going to see what happens on Tiger.

[Yuzo Fujishima]

Created attachment 43266 [details]
Add run-webkit-websocketserver

[Yuzo Fujishima]

Disabled wss because currently some platform doesn't support pyOpenSSL.

Yuzo

[Eric Seidel (no email)]

FYI, Ojan just re-wrote run-webkit-httpd in Python yesterday (for chromium's own needs -- chromium currently uses lighthttp to run the layout tests and we'd like to move back to apache), an I plan to upstream chromium's python run_webkit_tests.py soon.  Not for the purposes of taking over the world with python (although that's a nice fringe benefit), more so that we can run all of Chromium's WebKit infrastructure right out of webkit.org.  Once I'm successful with that effort, I'd like to roll all of WebKit's extra run-webkit-tests goodness into Chromium's version and combine them in a single replacement for both.

Just mentioning this, as there will be code sharing chances with those scripts as well, even if we don't get to with the perl version attached here.

[Adam Barth]

Attachment 43266 [details] passed the style-queue

[Yuzo Fujishima]

Created attachment 44289 [details]
Add run-webkit-websocketserver

[Yuzo Fujishima]

Updated the patch to log to a file.

Can anyone review and commit this?

(If you don't need this kind of script any more, please close the bug. :) )

Yuzo

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44289 [details] without any errors.

[Alexey Proskuryakov]

Comment on attachment 44289 [details]
Add run-webkit-websocketserver

Thanks for updating the patch!

> +# Note: This script must be run from WebKit directory.

I don't think that there is a good reason for this limitation. Why not use webkitdirs.pm to make it work from any directory, like all our scripts do?

> +    # wss is disabled until all platforms support pyOpenSSL.
> +    # close WEBSOCKETSECURESERVER_IN;
> +    # close WEBSOCKETSECURESERVER_OUT;
> +    # close WEBSOCKETSECURESERVER_ERR;
> +    # kill 15, $webSocketSecureServerPID;

This needs to be removed prior to landing.

> +    $isWebSocketServerOpen = 0;

Tracking the state seems superfluous - there is no way for openWebSocketServerIfNeeded to be called twice in this script, and no way for it to be called after closing.

r=me, but please fix the script to work from any directory. And this patch cannot be landed by commit-queue, because commented out code needs to be removed.

[Yuzo Fujishima]

Created attachment 44383 [details]
Add run-webkit-websocketserver

[Yuzo Fujishima]

Hi,

Addressed the comments. Can you take another look?

Yuzo

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44383 [details] without any errors.

[Alexey Proskuryakov]

Comment on attachment 44383 [details]
Add run-webkit-websocketserver

r=me

[WebKit Commit Bot]

Comment on attachment 44383 [details]
Add run-webkit-websocketserver

Clearing flags on attachment: 44383

Committed r51779: <http://trac.webkit.org/changeset/51779>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.