| Summary: | [webkitcorepy] Add subprocess_utils.run | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jonathan Bedard <jbedard> | ||||||
| Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aakash_jain, dewei_zhu, jlewis3, slewis, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=214378 | ||||||||
| Attachments: |
|
||||||||
|
Description
Jonathan Bedard
2020-08-20 09:52:28 PDT
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) 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! Created attachment 406952 [details]
Patch for landing
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 |