RESOLVED FIXED 231512
[run-benchmark] Make Python 3 compatible
https://bugs.webkit.org/show_bug.cgi?id=231512
Summary [run-benchmark] Make Python 3 compatible
Jonathan Bedard
Reported 2021-10-11 07:40:14 PDT
run-benchmark is almost Python 3 compatible, there are a few Popen calls that need to be decoded.
Attachments
Patch (3.50 KB, patch)
2021-10-11 07:42 PDT, Jonathan Bedard
no flags
Patch for landing (3.44 KB, patch)
2021-10-11 12:57 PDT, Jonathan Bedard
no flags
Radar WebKit Bug Importer
Comment 1 2021-10-11 07:41:05 PDT
Jonathan Bedard
Comment 2 2021-10-11 07:42:06 PDT
dewei_zhu
Comment 3 2021-10-11 11:25:45 PDT
Comment on attachment 440792 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440792&action=review r=me > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py:107 > + ) If we decide to go with invoking decode('utf-8') on output, we may not need it here because I don't think use output from `_browser_process` anyway. > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:73 > + **(dict(encoding='utf-8') if sys.version_info >= (3, 0) else dict()) I think encoding only available since python3.6. A most compatible way would be `process.communicate()[0].decode('utf-8')`.
Stephanie Lewis
Comment 4 2021-10-11 11:28:29 PDT
(In reply to dewei_zhu from comment #3) > Comment on attachment 440792 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440792&action=review > > r=me > > > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py:107 > > + ) > > If we decide to go with invoking decode('utf-8') on output, we may not need > it here because I don't think use output from `_browser_process` anyway. > > > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:73 > > + **(dict(encoding='utf-8') if sys.version_info >= (3, 0) else dict()) > > I think encoding only available since python3.6. > A most compatible way would be `process.communicate()[0].decode('utf-8')`. +1. We are using decode('utf-8') on pretty much all subprocess output throughout the perf. code
Jonathan Bedard
Comment 5 2021-10-11 12:57:32 PDT
Created attachment 440826 [details] Patch for landing
EWS
Comment 6 2021-10-11 14:12:15 PDT
Committed r283936 (242795@main): <https://commits.webkit.org/242795@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440826 [details].
Yusuke Suzuki
Comment 7 2021-10-12 12:37:41 PDT
Note You need to log in before you can comment on or make changes to this bug.