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