Bug 230319 - rwt: Switch pywebsocket from the old tools/pywebsocket to the new tools/third_party/pywebsocket3
Summary: rwt: Switch pywebsocket from the old tools/pywebsocket to the new tools/third...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
: 206546 (view as bug list)
Depends on:
Blocks: 237339
  Show dependency treegraph
 
Reported: 2021-09-15 13:47 PDT by Fujii Hironori
Modified: 2022-03-08 16:48 PST (History)
14 users (show)

See Also:


Attachments
WIP patch (1.56 KB, patch)
2021-09-15 13:48 PDT, Fujii Hironori
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP patch (30.86 KB, patch)
2021-09-21 18:15 PDT, Fujii Hironori
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP patch (32.49 KB, patch)
2021-09-21 20:49 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (33.98 KB, patch)
2021-09-21 22:59 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (69.35 KB, patch)
2022-02-17 18:05 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (83.81 KB, patch)
2022-02-21 15:27 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (84.48 KB, patch)
2022-02-21 19:48 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (84.67 KB, patch)
2022-02-22 12:42 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (84.35 KB, patch)
2022-02-22 16:28 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (84.35 KB, patch)
2022-02-23 07:19 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (85.24 KB, patch)
2022-02-23 08:51 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (88.36 KB, patch)
2022-02-23 13:50 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (90.00 KB, patch)
2022-02-23 18:00 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (92.85 KB, patch)
2022-02-24 10:43 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (93.40 KB, patch)
2022-02-24 11:07 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (93.34 KB, patch)
2022-02-24 14:48 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (93.36 KB, patch)
2022-02-27 17:44 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2021-09-15 13:47:10 PDT
rwt: Switch pywebsocket from the old tools/pywebsocket to the new tools/third_party/pywebsocket3

The upstream wpt removed tools/pywebsocket.
https://github.com/web-platform-tests/wpt/commit/da56e6cf601033599b04d36fbd0e1d092032b241

WIP patch is attachment#422172 [details].
Comment 1 Fujii Hironori 2021-09-15 13:48:18 PDT
Created attachment 438284 [details]
WIP patch
Comment 2 Sam Sneddon [:gsnedders] 2021-09-16 04:39:12 PDT
From Fujii on Slack:

> We have to update all *.py scripts in LayoutTests/http/tests/websocket/tests/hybi directory for python 3 and pywebsocket3.
Comment 3 Fujii Hironori 2021-09-21 18:15:19 PDT
Created attachment 438899 [details]
WIP patch
Comment 4 Fujii Hironori 2021-09-21 20:49:47 PDT
Created attachment 438911 [details]
WIP patch
Comment 5 Fujii Hironori 2021-09-21 22:59:25 PDT
Created attachment 438923 [details]
WIP patch
Comment 6 Radar WebKit Bug Importer 2021-09-22 13:48:19 PDT
<rdar://problem/83415801>
Comment 7 Jonathan Bedard 2022-02-15 09:41:03 PST
*** Bug 206546 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Bedard 2022-02-17 18:05:56 PST
Created attachment 452460 [details]
Patch
Comment 9 Aakash Jain 2022-02-18 07:10:52 PST
Cancelled https://ews-build.webkit.org/#/builders/70/builds/648 to speed up mac-wk2 ews queue.
Comment 10 Don Olmstead 2022-02-18 10:27:55 PST
Comment on attachment 452460 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:120
> -        if sys.version_info > (3, 0):
> -            python_interp = 'python2'
> +        if sys.version_info < (3, 0):
> +            python_interp = 'python3'

With this patch aren't we at the point where we should only be running Python 3?
Comment 11 Chris Dumez 2022-02-18 10:28:42 PST
(In reply to Don Olmstead from comment #10)
> Comment on attachment 452460 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=452460&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:120
> > -        if sys.version_info > (3, 0):
> > -            python_interp = 'python2'
> > +        if sys.version_info < (3, 0):
> > +            python_interp = 'python3'
> 
> With this patch aren't we at the point where we should only be running
> Python 3?

Right, WPT has been requiring python3 for a while now.
Comment 12 Don Olmstead 2022-02-18 10:35:32 PST
Comment on attachment 452460 [details]
Patch

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

>>> Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:120
>>> +            python_interp = 'python3'
>> 
>> With this patch aren't we at the point where we should only be running Python 3?
> 
> Right, WPT has been requiring python3 for a while now.

For WinCairo and PlayStation builds I've been waiting to remove Python 2 from our Docker containers so the question was more once this lands can we run a buildbot doing builds and tests that only has Python 3?

And if that's the case maybe we should be more vocal when we find someone still running a particular script in Python 2.
Comment 13 Jonathan Bedard 2022-02-18 10:44:25 PST
(In reply to Don Olmstead from comment #12)
> Comment on attachment 452460 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=452460&action=review
> 
> >>> Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:120
> >>> +            python_interp = 'python3'
> >> 
> >> With this patch aren't we at the point where we should only be running Python 3?
> > 
> > Right, WPT has been requiring python3 for a while now.
> 
> For WinCairo and PlayStation builds I've been waiting to remove Python 2
> from our Docker containers so the question was more once this lands can we
> run a buildbot doing builds and tests that only has Python 3?
> 
> And if that's the case maybe we should be more vocal when we find someone
> still running a particular script in Python 2.

Yes, and Apple won't be far behind. Monterey 12.3 has seeded and doesn't have Python 2. So everything will be Python 3 in short order. Plan is to land https://github.com/WebKit/WebKit/pull/160 Monday morning, which will invoke `run-webkit-tests` with Python 3 in post-commit infrastructure.
Comment 14 Jonathan Bedard 2022-02-21 15:27:05 PST
Created attachment 452786 [details]
Patch
Comment 15 Jonathan Bedard 2022-02-21 19:48:21 PST
Created attachment 452818 [details]
Patch
Comment 16 Jonathan Bedard 2022-02-22 12:42:05 PST
Created attachment 452894 [details]
Patch
Comment 17 Fujii Hironori 2022-02-22 12:57:48 PST
Comment on attachment 452894 [details]
Patch

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

> LayoutTests/ChangeLog:1
> +2022-02-21  Jonathan Bedard  <JonWBedard@gmail.com>

gmail.com
Comment 18 Jonathan Bedard 2022-02-22 16:28:22 PST
Created attachment 452913 [details]
Patch
Comment 19 Jonathan Bedard 2022-02-23 07:19:07 PST
Created attachment 452978 [details]
Patch
Comment 20 Jonathan Bedard 2022-02-23 08:51:07 PST
Created attachment 452984 [details]
Patch
Comment 21 Jonathan Bedard 2022-02-23 13:50:17 PST
Created attachment 453024 [details]
Patch
Comment 22 Jonathan Bedard 2022-02-23 18:00:18 PST
Created attachment 453061 [details]
Patch
Comment 23 Jonathan Bedard 2022-02-24 10:43:55 PST
Created attachment 453117 [details]
Patch
Comment 24 Jonathan Bedard 2022-02-24 11:07:22 PST
Created attachment 453119 [details]
Patch
Comment 25 Jonathan Bedard 2022-02-24 14:48:28 PST
Created attachment 453141 [details]
Patch
Comment 26 Jonathan Bedard 2022-02-24 14:51:13 PST
(In reply to Jonathan Bedard from comment #25)
> Created attachment 453141 [details]
> Patch

I want to make sure EWS is green before landing this change. I also want to make sure our Build Sheriffs are prepared to handle any fallout this may cause on queues we don't have EWS for.
Comment 27 Roy Reapor 2022-02-24 15:05:22 PST
Comment on attachment 453141 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:119
> +            python_interp = 'python3'

Can we use sys.executable here or the full path?

> LayoutTests/http/tests/websocket/tests/hybi/broken-utf8_wsh.py:10
> +    payload = b'This text should be ignored. \xff'  # '\xff' will never appear in UTF-8 encoded data.

extra space?
Comment 28 Jonathan Bedard 2022-02-27 17:42:49 PST
Comment on attachment 453141 [details]
Patch

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

>> Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py:119
>> +            python_interp = 'python3'
> 
> Can we use sys.executable here or the full path?

Short answer is "no". In general, we do use `sys.executable` (as you see in the above line), but we also need to always run this particular server with Python 3. We could try and determine the full path from sys.executable, but that assumes that Python 2 and 3 are installed in the same location, which isn't generally true. Ultimately, we should almost always be using sys.executable in practice since we are changing every run-webkit-tests invocation to Python 3 in https://bugs.webkit.org/show_bug.cgi?id=226658.

>> LayoutTests/http/tests/websocket/tests/hybi/broken-utf8_wsh.py:10
>> +    payload = b'This text should be ignored. \xff'  # '\xff' will never appear in UTF-8 encoded data.
> 
> extra space?

Pep8 asks for two spaces before an inline comment: https://www.python.org/dev/peps/pep-0008/#inline-comments
Comment 29 Jonathan Bedard 2022-02-27 17:44:55 PST
Created attachment 453363 [details]
Patch for landing
Comment 30 EWS 2022-02-27 20:03:17 PST
Committed r290580 (247858@main): <https://commits.webkit.org/247858@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453363 [details].