[PATCH] kunit: add run_checks.py script to validate kunit changes

[Daniel Latypov]

This formalizes the checks KUnit maintainers have been running (or in
other cases: forgetting to run).

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 <dlat...@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 <dlat...@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:])

base-commit: 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a
--
2.33.1.1089.g2158813163f-goog

[David Gow]

On Wed, Nov 3, 2021 at 7:57 AM 'Daniel Latypov' via KUnit Development
<kuni...@googlegroups.com> wrote:
>
> This formalizes the checks KUnit maintainers have been running (or in
> other cases: forgetting to run).

Guilty as charged. :-)

Thanks -- this is working well here. A couple of minor suggestions
below, but even without them, this is very useful.

Reviewed-by: David Gow <davi...@google.com>

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__'?

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+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20211102235734.497713-1-dlatypov%40google.com.

[Daniel Latypov]

This formalizes the checks KUnit maintainers have been running (or in
other cases: forgetting to run).

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 <dlat...@google.com>

Reviewed-by: David Gow <davi...@google.com>
---
tools/testing/kunit/run_checks.py | 81 +++++++++++++++++++++++++++++++
1 file changed, 81 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..4f32133ed77c
--- /dev/null
+++ b/tools/testing/kunit/run_checks.py
@@ -0,0 +1,81 @@

+#!/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 <dlat...@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 argv:
+ raise RuntimeError('This script takes no 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
+

+ has_failures = False

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

+ has_failures = True

+ 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()
+

+ if has_failures:
+ sys.exit(1)

+
+
+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:])

base-commit: 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a
--
2.33.1.1089.g2158813163f-goog

[Daniel Latypov]

On Tue, Nov 2, 2021 at 7:38 PM David Gow <davi...@google.com> wrote:
>
> On Wed, Nov 3, 2021 at 7:57 AM 'Daniel Latypov' via KUnit Development
> <kuni...@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 :)

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

Oh, I thought I did this before sending it out.
Will add this in for v2.

[David Gow]

Works a treat, thanks.

This is still
Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David

[Brendan Higgins]

Reviewed-by: Brendan Higgins <brendan...@google.com>