RESOLVED FIXED 201954
Python 3: Add test-webkitpy for Python 3 
https://bugs.webkit.org/show_bug.cgi?id=201954
Summary Python 3: Add test-webkitpy for Python 3
Jonathan Bedard
Reported 2019-09-18 16:58:42 PDT
We need to allow test-webkitpy to support Python 3 so don't have to cherry-pick which tests we run.
Attachments
Patch (2.69 KB, patch)
2019-10-01 15:51 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (2.72 KB, patch)
2019-10-01 16:45 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (2.70 KB, patch)
2019-10-02 07:47 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (2.86 KB, patch)
2019-10-02 09:09 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-09-18 17:04:21 PDT
<rdar://problem/55499881>
Jonathan Bedard
Comment 2 2019-10-01 15:51:52 PDT
Created attachment 379961 [details] Patch
Jonathan Bedard
Comment 3 2019-10-01 16:45:58 PDT
Created attachment 379965 [details] Patch
Dean Johnson
Comment 4 2019-10-01 19:23:33 PDT
Comment on attachment 379965 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379965&action=review Minor nits, but otherwise looks pretty good. Unofficial r+. > Tools/Scripts/test-webkitpy-python3:32 > + PYTHON3_DIRECTORIES[0] if len(PYTHON3_DIRECTORIES) == 1 else '{} and {}'.format( Nit: This is a bit hard to read; can you either put it into a function or run this logic before generating the help message? > Tools/Scripts/test-webkitpy-python3:52 > + result = unittest.TextTestRunner(verbosity=2 if options.verbose else 1, failfast=options.stop_on_fail, buffer=not options.verbose).run(suite) Nit: This is a bit hard to read. Can you separate out options.verbosity to just store a value of 1 (default) or 2 when -v|--verbose is provided?
Jonathan Bedard
Comment 5 2019-10-02 07:47:25 PDT
Created attachment 380020 [details] Patch
Aakash Jain
Comment 6 2019-10-02 08:41:48 PDT
Comment on attachment 380020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380020&action=review r+ with comments. > Tools/Scripts/test-webkitpy-python3:11 > +PYTHON3_DIRECTORIES = [ might consider renaming it to something like this to be more readable/clear: PYTHON3_COMPATIBLE_DIRECTORIES > Tools/Scripts/test-webkitpy-python3:17 > +def list_to_string(lst): lst is confusing (it might be read as first). alternatives: input, input_list. Also this method isn't really required, it's used only once, you can simply use following at that place (although this one generates string which is slightly more readable, since it adds 'and'). ', '.join(list) > Tools/Scripts/test-webkitpy-python3:50 > + for tst in module_suite if module_suite else []: Nit: tst => test Following might be more cleaner: for test in (module_suite or []): > Tools/Scripts/test-webkitpy-python3:54 > + raise RuntimeError('No tests matching...') Nit: 'No tests matching...' => 'No matching tests found.'
Jonathan Bedard
Comment 7 2019-10-02 09:09:40 PDT
Created attachment 380029 [details] Patch
Jonathan Bedard
Comment 8 2019-10-02 09:21:05 PDT
Committed r250608: <https://trac.webkit.org/changeset/250608>
Aakash Jain
Comment 9 2019-10-02 13:21:25 PDT
Comment on attachment 380029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380029&action=review > Tools/Scripts/test-webkitpy-python3:1 > +#!/usr/bin/env python3 Copyright message is missing. Can you add it in a follow-up commit?
Jonathan Bedard
Comment 10 2019-10-02 14:45:31 PDT
Committed r250631: <https://trac.webkit.org/changeset/250631>
Jonathan Bedard
Comment 11 2019-10-02 15:42:03 PDT
(In reply to Aakash Jain from comment #9) > Comment on attachment 380029 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=380029&action=review > > > Tools/Scripts/test-webkitpy-python3:1 > > +#!/usr/bin/env python3 > > Copyright message is missing. Can you add it in a follow-up commit? Landed in <https://trac.webkit.org/changeset/250631>
Note You need to log in before you can comment on or make changes to this bug.