| Message ID | 20211102235734.497713-1-dlatypov@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: add run_checks.py script to validate kunit changes | expand |
On Wed, Nov 3, 2021 at 7:57 AM 'Daniel Latypov' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > This formalizes the checks KUnit maintainers have been running (or in > other cases: forgetting to run). Guilty as charged. :-) > > This script also runs them all in parallel to minimize friction (pytype > can be fairly slow, but not slower than running kunit.py). > > Example output: > $ ./tools/testing/kunit/run_checks.py > Waiting on 4 checks (kunit_tool_test.py, kunit smoke test, pytype, mypy)... > kunit_tool_test.py: PASSED > mypy: PASSED > pytype: PASSED > kunit smoke test: PASSED > > On failure or timeout (5 minutes), it'll dump out the stdout/stderr. > E.g. adding in a type-checking error: > mypy: FAILED > > kunit.py:54: error: Name 'nonexistent_function' is not defined > > Found 1 error in 1 file (checked 8 source files) > > mypy and pytype are two Python type-checkers and must be installed. > This file treats them as optional and will mark them as SKIPPED if not > installed. > > This tool also runs `kunit.py run --kunitconfig=lib/kunit` to run > KUnit's own KUnit tests and to verify KUnit kernel code and kunit.py > play nicely together. > > It uses --build_dir=kunit_run_checks so as not to clobber the default > build_dir, which helps make it faster by reducing the need to rebuild, > esp. if you're been passing in --arch instead of using UML. > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- Thanks -- this is working well here. A couple of minor suggestions below, but even without them, this is very useful. Reviewed-by: David Gow <davidgow@google.com> > tools/testing/kunit/run_checks.py | 76 +++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > create mode 100755 tools/testing/kunit/run_checks.py > > diff --git a/tools/testing/kunit/run_checks.py b/tools/testing/kunit/run_checks.py > new file mode 100755 > index 000000000000..d03ca3f84b91 > --- /dev/null > +++ b/tools/testing/kunit/run_checks.py > @@ -0,0 +1,76 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0 > +# > +# This file runs some basic checks to verify kunit works. > +# It is only of interest if you're making changes to KUnit itself. > +# > +# Copyright (C) 2021, Google LLC. > +# Author: Daniel Latypov <dlatypov@google.com.com> > + > +from concurrent import futures > +import datetime > +import os > +import shutil > +import subprocess > +import sys > +import textwrap > +from typing import Dict, List, Sequence, Tuple > + > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__)) > +_TIMEOUT = datetime.timedelta(minutes=5).total_seconds() > + > +commands: Dict[str, Sequence[str]] = { > + 'kunit_tool_test.py': ['./kunit_tool_test.py'], > + 'kunit smoke test': ['./kunit.py', 'run', '--kunitconfig=lib/kunit', '--build_dir=kunit_run_checks'], > + 'pytype': ['/bin/sh', '-c', 'pytype *.py'], > + 'mypy': ['/bin/sh', '-c', 'mypy *.py'], > +} > + > +# The user might not have mypy or pytype installed, skip them if so. > +# Note: you can install both via `$ pip install mypy pytype` > +necessary_deps : Dict[str, str] = { > + 'pytype': 'pytype', > + 'mypy': 'mypy', > +} > + > +def main(argv: Sequence[str]) -> None: > + if len(argv) > 1: > + raise RuntimeError('Too many command-line arguments.') What does the command-line argument here actually do? It looks like nothing, because the argv variable is shadowed below? Or was this supposed to check that there are no arguments, which doesn't work because argv[] is stripped of its first element in the 'if __name__=='__main__'? > + > + future_to_name: Dict[futures.Future, str] = {} > + executor = futures.ThreadPoolExecutor(max_workers=len(commands)) > + for name, argv in commands.items(): > + if name in necessary_deps and shutil.which(necessary_deps[name]) is None: > + print(f'{name}: SKIPPED, {necessary_deps[name]} not in $PATH') > + continue > + f = executor.submit(run_cmd, argv) > + future_to_name[f] = name > + > + print(f'Waiting on {len(future_to_name)} checks ({", ".join(future_to_name.values())})...') > + for f in futures.as_completed(future_to_name.keys()): > + name = future_to_name[f] > + ex = f.exception() > + if not ex: > + print(f'{name}: PASSED') > + continue > + > + if isinstance(ex, subprocess.TimeoutExpired): > + print(f'{name}: TIMED OUT') > + elif isinstance(ex, subprocess.CalledProcessError): > + print(f'{name}: FAILED') > + else: > + print('{name}: unexpected exception: {ex}') > + continue > + > + output = ex.output > + if output: > + print(textwrap.indent(output.decode(), '> ')) > + executor.shutdown() > + > + > +def run_cmd(argv: Sequence[str]): > + subprocess.check_output(argv, stderr=subprocess.STDOUT, cwd=ABS_TOOL_PATH, timeout=_TIMEOUT) > + > + > +if __name__ == '__main__': > + main(sys.argv[1:]) Any chance we could get this to return a non-zero exit code if one of these checks fails? > > base-commit: 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a > -- > 2.33.1.1089.g2158813163f-goog > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20211102235734.497713-1-dlatypov%40google.com.
On Tue, Nov 2, 2021 at 7:38 PM David Gow <davidgow@google.com> wrote: > > On Wed, Nov 3, 2021 at 7:57 AM 'Daniel Latypov' via KUnit Development > <kunit-dev@googlegroups.com> wrote: > > > > This formalizes the checks KUnit maintainers have been running (or in > > other cases: forgetting to run). > > Guilty as charged. :-) I was largely referring to myself :) > > > > This script also runs them all in parallel to minimize friction (pytype > > can be fairly slow, but not slower than running kunit.py). > > > > Example output: > > $ ./tools/testing/kunit/run_checks.py > > Waiting on 4 checks (kunit_tool_test.py, kunit smoke test, pytype, mypy)... > > kunit_tool_test.py: PASSED > > mypy: PASSED > > pytype: PASSED > > kunit smoke test: PASSED > > > > On failure or timeout (5 minutes), it'll dump out the stdout/stderr. > > E.g. adding in a type-checking error: > > mypy: FAILED > > > kunit.py:54: error: Name 'nonexistent_function' is not defined > > > Found 1 error in 1 file (checked 8 source files) > > > > mypy and pytype are two Python type-checkers and must be installed. > > This file treats them as optional and will mark them as SKIPPED if not > > installed. > > > > This tool also runs `kunit.py run --kunitconfig=lib/kunit` to run > > KUnit's own KUnit tests and to verify KUnit kernel code and kunit.py > > play nicely together. > > > > It uses --build_dir=kunit_run_checks so as not to clobber the default > > build_dir, which helps make it faster by reducing the need to rebuild, > > esp. if you're been passing in --arch instead of using UML. > > > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > > --- > > Thanks -- this is working well here. A couple of minor suggestions > below, but even without them, this is very useful. > > Reviewed-by: David Gow <davidgow@google.com> > > > tools/testing/kunit/run_checks.py | 76 +++++++++++++++++++++++++++++++ > > 1 file changed, 76 insertions(+) > > create mode 100755 tools/testing/kunit/run_checks.py > > > > diff --git a/tools/testing/kunit/run_checks.py b/tools/testing/kunit/run_checks.py > > new file mode 100755 > > index 000000000000..d03ca3f84b91 > > --- /dev/null > > +++ b/tools/testing/kunit/run_checks.py > > @@ -0,0 +1,76 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# This file runs some basic checks to verify kunit works. > > +# It is only of interest if you're making changes to KUnit itself. > > +# > > +# Copyright (C) 2021, Google LLC. > > +# Author: Daniel Latypov <dlatypov@google.com.com> > > + > > +from concurrent import futures > > +import datetime > > +import os > > +import shutil > > +import subprocess > > +import sys > > +import textwrap > > +from typing import Dict, List, Sequence, Tuple > > + > > +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__)) > > +_TIMEOUT = datetime.timedelta(minutes=5).total_seconds() > > + > > +commands: Dict[str, Sequence[str]] = { > > + 'kunit_tool_test.py': ['./kunit_tool_test.py'], > > + 'kunit smoke test': ['./kunit.py', 'run', '--kunitconfig=lib/kunit', '--build_dir=kunit_run_checks'], > > + 'pytype': ['/bin/sh', '-c', 'pytype *.py'], > > + 'mypy': ['/bin/sh', '-c', 'mypy *.py'], > > +} > > + > > +# The user might not have mypy or pytype installed, skip them if so. > > +# Note: you can install both via `$ pip install mypy pytype` > > +necessary_deps : Dict[str, str] = { > > + 'pytype': 'pytype', > > + 'mypy': 'mypy', > > +} > > + > > +def main(argv: Sequence[str]) -> None: > > + if len(argv) > 1: > > + raise RuntimeError('Too many command-line arguments.') > > What does the command-line argument here actually do? It looks like > nothing, because the argv variable is shadowed below? > > Or was this supposed to check that there are no arguments, which > doesn't work because argv[] is stripped of its first element in the > 'if __name__=='__main__'? It was supposed to check for no args. I forgot I already stripped it off (this bit of code here was autogenerated for me by a snippet). > > > + > > + future_to_name: Dict[futures.Future, str] = {} > > + executor = futures.ThreadPoolExecutor(max_workers=len(commands)) > > + for name, argv in commands.items(): > > + if name in necessary_deps and shutil.which(necessary_deps[name]) is None: > > + print(f'{name}: SKIPPED, {necessary_deps[name]} not in $PATH') > > + continue > > + f = executor.submit(run_cmd, argv) > > + future_to_name[f] = name > > + > > + print(f'Waiting on {len(future_to_name)} checks ({", ".join(future_to_name.values())})...') > > + for f in futures.as_completed(future_to_name.keys()): > > + name = future_to_name[f] > > + ex = f.exception() > > + if not ex: > > + print(f'{name}: PASSED') > > + continue > > + > > + if isinstance(ex, subprocess.TimeoutExpired): > > + print(f'{name}: TIMED OUT') > > + elif isinstance(ex, subprocess.CalledProcessError): > > + print(f'{name}: FAILED') > > + else: > > + print('{name}: unexpected exception: {ex}') > > + continue > > + > > + output = ex.output > > + if output: > > + print(textwrap.indent(output.decode(), '> ')) > > + executor.shutdown() > > + > > + > > +def run_cmd(argv: Sequence[str]): > > + subprocess.check_output(argv, stderr=subprocess.STDOUT, cwd=ABS_TOOL_PATH, timeout=_TIMEOUT) > > + > > + > > +if __name__ == '__main__': > > + main(sys.argv[1:]) > > Any chance we could get this to return a non-zero exit code if one of > these checks fails? Oh, I thought I did this before sending it out. Will add this in for v2. > > > > > base-commit: 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a > > -- > > 2.33.1.1089.g2158813163f-goog > > > > -- > > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. > > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20211102235734.497713-1-dlatypov%40google.com.
diff --git a/tools/testing/kunit/run_checks.py b/tools/testing/kunit/run_checks.py new file mode 100755 index 000000000000..d03ca3f84b91 --- /dev/null +++ b/tools/testing/kunit/run_checks.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 +# +# This file runs some basic checks to verify kunit works. +# It is only of interest if you're making changes to KUnit itself. +# +# Copyright (C) 2021, Google LLC. +# Author: Daniel Latypov <dlatypov@google.com.com> + +from concurrent import futures +import datetime +import os +import shutil +import subprocess +import sys +import textwrap +from typing import Dict, List, Sequence, Tuple + +ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__)) +_TIMEOUT = datetime.timedelta(minutes=5).total_seconds() + +commands: Dict[str, Sequence[str]] = { + 'kunit_tool_test.py': ['./kunit_tool_test.py'], + 'kunit smoke test': ['./kunit.py', 'run', '--kunitconfig=lib/kunit', '--build_dir=kunit_run_checks'], + 'pytype': ['/bin/sh', '-c', 'pytype *.py'], + 'mypy': ['/bin/sh', '-c', 'mypy *.py'], +} + +# The user might not have mypy or pytype installed, skip them if so. +# Note: you can install both via `$ pip install mypy pytype` +necessary_deps : Dict[str, str] = { + 'pytype': 'pytype', + 'mypy': 'mypy', +} + +def main(argv: Sequence[str]) -> None: + if len(argv) > 1: + raise RuntimeError('Too many command-line arguments.') + + future_to_name: Dict[futures.Future, str] = {} + executor = futures.ThreadPoolExecutor(max_workers=len(commands)) + for name, argv in commands.items(): + if name in necessary_deps and shutil.which(necessary_deps[name]) is None: + print(f'{name}: SKIPPED, {necessary_deps[name]} not in $PATH') + continue + f = executor.submit(run_cmd, argv) + future_to_name[f] = name + + print(f'Waiting on {len(future_to_name)} checks ({", ".join(future_to_name.values())})...') + for f in futures.as_completed(future_to_name.keys()): + name = future_to_name[f] + ex = f.exception() + if not ex: + print(f'{name}: PASSED') + continue + + if isinstance(ex, subprocess.TimeoutExpired): + print(f'{name}: TIMED OUT') + elif isinstance(ex, subprocess.CalledProcessError): + print(f'{name}: FAILED') + else: + print('{name}: unexpected exception: {ex}') + continue + + output = ex.output + if output: + print(textwrap.indent(output.decode(), '> ')) + executor.shutdown() + + +def run_cmd(argv: Sequence[str]): + subprocess.check_output(argv, stderr=subprocess.STDOUT, cwd=ABS_TOOL_PATH, timeout=_TIMEOUT) + + +if __name__ == '__main__': + main(sys.argv[1:])