All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kunit: add run_checks.py script to validate kunit changes
@ 2021-11-02 23:57 Daniel Latypov
  2021-11-03  2:38 ` David Gow
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Latypov @ 2021-11-02 23:57 UTC (permalink / raw)
  To: brendanhiggins, davidgow
  Cc: linux-kernel, kunit-dev, linux-kselftest, skhan, 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 <dlatypov@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.')
+
+	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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] kunit: add run_checks.py script to validate kunit changes
  2021-11-02 23:57 [PATCH] kunit: add run_checks.py script to validate kunit changes Daniel Latypov
@ 2021-11-03  2:38 ` David Gow
  2021-11-03  4:22   ` Daniel Latypov
  0 siblings, 1 reply; 3+ messages in thread
From: David Gow @ 2021-11-03  2:38 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: Brendan Higgins, Linux Kernel Mailing List, KUnit Development,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kunit: add run_checks.py script to validate kunit changes
  2021-11-03  2:38 ` David Gow
@ 2021-11-03  4:22   ` Daniel Latypov
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Latypov @ 2021-11-03  4:22 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Linux Kernel Mailing List, KUnit Development,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-03  4:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02 23:57 [PATCH] kunit: add run_checks.py script to validate kunit changes Daniel Latypov
2021-11-03  2:38 ` David Gow
2021-11-03  4:22   ` Daniel Latypov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.