Bug 215702 – [webkitcorepy] Add subprocess_utils.run

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

Python 3's subprocess.run API is great. We should have it in Python 2 as well so we can write modern Python code compatible with both versions.

[Radar WebKit Bug Importer]

<rdar://problem/67487751>

[Jonathan Bedard]

Created attachment 406938 [details]
Patch

Comment on attachment 406938 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406938&action=review

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:39
> +        input = kwargs.pop('input', None)
> +        capture_output = kwargs.pop('capture_output', False)
> +        timeout = kwargs.pop('timeout', None)
> +        check = kwargs.pop('check', False)

Other than timeout, is there any reason we want to extract other variables out of **kwargs? It looks like the same defaults in the subprocess.run

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:54
> +                return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs)
> +            return subprocess.run(
> +                *popenargs,
> +                input=input,
> +                capture_output=capture_output,
> +                timeout=min(timeout or sys.maxsize, int(math.ceil(difference))),
> +                check=check,
> +                **kwargs)

So the only difference is timeout, maybe we can simplify this by doing:
if difference:
    timeout = min(timeout or sys.maxsize, int(math.ceil(difference)))
return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs)

[Jonathan Bedard]

Comment on attachment 406938 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406938&action=review

>> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:39
>> +        check = kwargs.pop('check', False)
> 
> Other than timeout, is there any reason we want to extract other variables out of **kwargs? It looks like the same defaults in the subprocess.run

Was copying from the function subprocess.run declaration, turns out, no, we don't need to extract the other variables. I thought we needed it to get the correct defaults, for some reason, but we don't.

Will Remove.

>> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:54
>> +                **kwargs)
> 
> So the only difference is timeout, maybe we can simplify this by doing:
> if difference:
>     timeout = min(timeout or sys.maxsize, int(math.ceil(difference)))
> return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs)

Thanks for the suggestion! Much cleaner!

[Jonathan Bedard]

Created attachment 406952 [details]
Patch for landing

[EWS]

Committed r265957: <https://trac.webkit.org/changeset/265957>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406952 [details].

r=me