| Message ID | 20211019144918.3159078-9-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python/iotests: Run iotest linters during Python CI | expand |
On 19.10.21 16:49, John Snow wrote: > Instead of using a process return code as the python function return > value (or just not returning anything at all), allow run_linter() to > raise an exception instead. > > The responsibility for printing output on error shifts from the function > itself to the caller, who will know best how to present/format that > information. (Also, "suppress_output" is now a lot more accurate of a > parameter name.) > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/qemu-iotests/297 | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) Thanks! :) Reviewed-by: Hanna Reitz <hreitz@redhat.com>
On Tue, Oct 26, 2021 at 6:10 AM Hanna Reitz <hreitz@redhat.com> wrote: > On 19.10.21 16:49, John Snow wrote: > > Instead of using a process return code as the python function return > > value (or just not returning anything at all), allow run_linter() to > > raise an exception instead. > > > > The responsibility for printing output on error shifts from the function > > itself to the caller, who will know best how to present/format that > > information. (Also, "suppress_output" is now a lot more accurate of a > > parameter name.) > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > tests/qemu-iotests/297 | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > Thanks! :) > > Reviewed-by: Hanna Reitz <hreitz@redhat.com> > > No problem. Thanks for taking the time to review it! --js
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 index d21673a2929..76d6a23f531 100755 --- a/tests/qemu-iotests/297 +++ b/tests/qemu-iotests/297 @@ -70,22 +70,18 @@ def run_linter( """ Run a python-based linting tool. - If suppress_output is True, capture stdout/stderr of the child - process and only print that information back to stdout if the child - process's return code was non-zero. + :param suppress_output: If True, suppress all stdout/stderr output. + :raise CalledProcessError: If the linter process exits with failure. """ - p = subprocess.run( + subprocess.run( ('python3', '-m', tool, *args), env=env, - check=False, + check=True, stdout=subprocess.PIPE if suppress_output else None, stderr=subprocess.STDOUT if suppress_output else None, universal_newlines=True, ) - if suppress_output and p.returncode != 0: - print(p.stdout) - def main() -> None: for linter in ('pylint-3', 'mypy'): @@ -102,11 +98,19 @@ def main() -> None: print('=== pylint ===') sys.stdout.flush() - run_linter('pylint', files, env=env) + try: + run_linter('pylint', files, env=env) + except subprocess.CalledProcessError: + # pylint failure will be caught by diffing the IO. + pass print('=== mypy ===') sys.stdout.flush() - run_linter('mypy', files, env=env, suppress_output=True) + try: + run_linter('mypy', files, env=env, suppress_output=True) + except subprocess.CalledProcessError as exc: + if exc.output: + print(exc.output) iotests.script_main(main)
Instead of using a process return code as the python function return value (or just not returning anything at all), allow run_linter() to raise an exception instead. The responsibility for printing output on error shifts from the function itself to the caller, who will know best how to present/format that information. (Also, "suppress_output" is now a lot more accurate of a parameter name.) Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qemu-iotests/297 | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)