Bug 230319

Created attachment 438284 [details] WIP patch

From Fujii on Slack: > We have to update all *.py scripts in LayoutTests/http/tests/websocket/tests/hybi directory for python 3 and pywebsocket3.

Created attachment 438899 [details] WIP patch

Created attachment 438911 [details] WIP patch

Created attachment 438923 [details] WIP patch

<rdar://problem/83415801>

*** Bug 206546 has been marked as a duplicate of this bug. ***

Created attachment 452460 [details] Patch

Cancelled https://ews-build.webkit.org/#/builders/70/builds/648 to speed up mac-wk2 ews queue.

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?

(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 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.

(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.

Created attachment 452786 [details] Patch

Created attachment 452818 [details] Patch

Created attachment 452894 [details] Patch

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

Created attachment 452913 [details] Patch

Created attachment 452978 [details] Patch

Created attachment 452984 [details] Patch

Created attachment 453024 [details] Patch

Created attachment 453061 [details] Patch

Created attachment 453117 [details] Patch

Created attachment 453119 [details] Patch

Created attachment 453141 [details] Patch

(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 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 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

Created attachment 453363 [details] Patch for landing

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].