Bug 201954

Summary:

Python 3: Add test-webkitpy for Python 3

Product:

WebKit

Reporter:

Jonathan Bedard <jbedard>

Component:

Tools / Tests

Assignee:

Jonathan Bedard <jbedard>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, dean_johnson, dewei_zhu, jbedard, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=184986

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (2.72 KB, patch) 2019-10-01 16:45 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (2.70 KB, patch) 2019-10-02 07:47 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (2.86 KB, patch) 2019-10-02 09:09 PDT, Jonathan Bedard	no flags	Details Formatted Diff Diff
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.