Python 3: Add test-webkitpy for Python 3

[Jonathan Bedard]

We need to allow test-webkitpy to support Python 3 so don't have to cherry-pick which tests we run.

[Radar WebKit Bug Importer]

<rdar://problem/55499881>

[Jonathan Bedard]

Created attachment 379961 [details]
Patch

[Jonathan Bedard]

Created attachment 379965 [details]
Patch

[Dean Johnson]

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]

Created attachment 380020 [details]
Patch

[Aakash Jain]

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]

Created attachment 380029 [details]
Patch

[Jonathan Bedard]

Committed r250608: <https://trac.webkit.org/changeset/250608>

[Aakash Jain]

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]

Committed r250631: <https://trac.webkit.org/changeset/250631>

[Jonathan Bedard]

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