Bug 206546 – webkitpy: WebSocket server doesn't support Python 3

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Tools / Tests

[Jonathan Bedard]

There are changes we can make to fix this, although they would need to be upstreamed to work. In the mean time, we should explicitly run the WebSocket server with Python 2.

[Jonathan Bedard]

Created attachment 388331 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 388331 [details]
Patch

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

> Tools/ChangeLog:8
> +        websocket_server.py doesn't support Python 3, and needs to be run with

Is there a new version of the server that supports Python 3? Perhaps Chromium implemented the changes already?

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 388331 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388331&action=review
> 
> > Tools/ChangeLog:8
> > +        websocket_server.py doesn't support Python 3, and needs to be run with
> 
> Is there a new version of the server that supports Python 3? Perhaps
> Chromium implemented the changes already?

There is not, https://github.com/web-platform-tests/wpt/issues/8820 tracks this on their side. We need to update our import anyways, and then I can see what it takes to get this specific script Python 3.

[Alexey Proskuryakov]

Two things that make it look like something is off here:

1. We have our own server, our tests were failing. So that needs a fix or a workaround too.

2. But only one of WPT WebSocket tests was failing. Is it actually broken?

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #4)
> Two things that make it look like something is off here:
> 
> 1. We have our own server, our tests were failing. So that needs a fix or a
> workaround too.
> 
> 2. But only one of WPT WebSocket tests was failing. Is it actually broken?

We did have an 'old' server, but it wasn't our own, it was just an old version of a third party script. Carlos replaced the old one with the w3c one a few months back, https://trac.webkit.org/changeset/247640/webkit. Now we have a single WebSocket server, the w3c one.

[Alexey Proskuryakov]

Comment on attachment 388331 [details]
Patch

Seems fine as an interim measure. May be worth considering python2 when it exists though, as I’m not sure what this patch will do on systems where “python” is python3.

[Alexey Proskuryakov]

Still, would prefer to understand why wpt tests aren’t failing.

[Jonathan Bedard]

Created attachment 388453 [details]
Patch

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #6)
> Comment on attachment 388331 [details]
> Patch
> 
> Seems fine as an interim measure. May be worth considering python2 when it
> exists though, as I’m not sure what this patch will do on systems where
> “python” is python3.

This is a good point. I changed the code to reference python2 instead of python, and it will use the current interpreter if that interpreter is Python 2. If a script is running on a system in Python 3 that doesn't have python2, this will fail, but that seems acceptable, or at least, no worse than the current situation.

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #7)
> Still, would prefer to understand why wpt tests aren’t failing.

When I run those tests independently, they don't even start the WebSocket server. So it's not surprising they don't fail when the WebSocket server fails to start.

What I don't understand is what those tests are actually supposed to be testing. But I'm not sure that question is within the scope of this patch.

[WebKit Commit Bot]

Comment on attachment 388453 [details]
Patch

Clearing flags on attachment: 388453

Committed r254944: <https://trac.webkit.org/changeset/254944>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

> When I run those tests independently, they don't even start the WebSocket server. So it's not surprising they don't fail when the WebSocket server fails to start.

This doesn't necessarily tell us much. The server is started anyway when running all tests, so this problem may be masked most of the time.

[Radar WebKit Bug Importer]

<rdar://problem/58814743>

[Jonathan Bedard]

Looks like https://github.com/web-platform-tests/wpt/issues/8820 was resolved, so there now is a new version that supports Python 3.

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 451740 [details]
Patch

[Jonathan Bedard]

Created attachment 451751 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 451751 [details]
Patch

rs=me, as long as the license is unchanged.

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #19)
> Comment on attachment 451751 [details]
> Patch
> 
> rs=me, as long as the license is unchanged.

Double checked that the license in https://github.com/web-platform-tests/wpt/blob/master/LICENSE.md matches what we have in LayoutTests/imported/w3c/web-platform-tests/LICENSE.md

[Jonathan Bedard]

Looks like I need to look at the tests we are importing as well, will do so next week. Will not be landing until EWS is green.

[Sam Sneddon [:gsnedders]]

Note that bug 230319 already previously started to try and update pywebsocket; one of these should be dupe'd to the other.

Bug 227255 actually already imported a recent enough version of the WPT tools for our purposes, but this didn't get noticed because it didn't remove the (gone upstream) web-platform-tests/tools/pywebsocket/.

It may well be worthwhile just focusing on removing that, and moving to web-platform-tests/tools/third_party/pywebsocket3/, rather than doing this combined with a reimport.

[Jonathan Bedard]

Created attachment 451914 [details]
Patch

[Jonathan Bedard]

I'm going to move work over to bug 230319, since that already has more work done that I've done here.

*** This bug has been marked as a duplicate of bug 230319 ***