Bug 46956 – WebSocket tests are flaky

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

WebSocket tests are flaky

[Adam Barth]

Created attachment 69416 [details]
Patch

[James Robinson]

Comment on attachment 69416 [details]
Patch

This patch has a ton of file moves but the patchfile doesn't include any SVN metadata.  I would hate to lose the file history and blame information for all these tests.  Can you regenerate using SVN MV instead, or coerce git to produce the metadata?

[James Robinson]

Comment on attachment 69416 [details]
Patch

Just realized I'm probably misreading the raw patch.  Restoring abarth's r?.  Please do confirm that this will preserve history before landing, though.

[Adam Barth]

I think the diff might look different because it's a directory move instead of a file move.  In any case, I did create it with "svn mv".  If I land it from my working copy, the history should be preserved.

[Fumitoshi Ukai]

Comment on attachment 69416 [details]
Patch

I understand this patch tries to serve html files for websocket test by apache on port 8000, and keeps websocket service by pywebsocket server on port 8880. right?
I'm not sure if we really need to move websocket test files under http/test directory. Can't we serve html files in websocket/test by apache on port 8000?
Moving test files would require more changes in chromium test system than just letting html serving by http server.

[Adam Barth]

> I understand this patch tries to serve html files for websocket test by apache on port 8000, and keeps websocket service by pywebsocket server on port 8880. right?

That's correct.

> I'm not sure if we really need to move websocket test files under http/test directory. Can't we serve html files in websocket/test by apache on port 8000?

The problem is that websocket/test is not mapped into the URL space of the Apache server.

> Moving test files would require more changes in chromium test system than just letting html serving by http server.

I've updated new-run-webkit-tests to run them properly from their new location.  We need to be able to move test files around.  It's not a reasonable constraint from the Chromium port that we can't do that.

[Eric Seidel (no email)]

Comment on attachment 69416 [details]
Patch

We shouldn't need to copy the js test pre stuff since we have http://trac.webkit.org/changeset/44020.

[Eric Seidel (no email)]

Comment on attachment 69416 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69416&action=review

> WebKitTools/Scripts/old-run-webkit-tests:696
> +        if ($test =~ /^http\/tests\/websocket\//) {

This big block really should have been a function.  It's not your fault that it isn't though.

[Eric Seidel (no email)]

Comment on attachment 69416 [details]
Patch

r- for moving to js-test-resources.  Otherwise looks OK.

[Adam Barth]

Created attachment 69494 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 69494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69494&action=review

OK.

> LayoutTests/http/tests/websocket/tests/reload-crash.html:6
> +<link rel="stylesheet" href="../../../js-test-resources/js-test-style.css">
> +<script src="../../../js-test-resources/js-test-pre.js"></script>
> +<script src="../../../js-test-resources/js-test-post-function.js"></script>

These could just be /js-... insetad of ../../, but that's ifne.

[Adam Barth]

Committed r68914: <http://trac.webkit.org/changeset/68914>

[WebKit Review Bot]

http://trac.webkit.org/changeset/68914 might have broken GTK Linux 32-bit Debug