[PATCH v3 0/3] kunit: support running subsets of test suites from kunit.py

[PATCH v3 0/3] kunit: support running subsets of test suites from kunit.py

[Daniel Latypov]

When using `kunit.py run` to run tests, users must populate a
`kunitconfig` file to select the options the tests are hidden behind and
all their dependencies.

The patch [1] to allow specifying a path to kunitconfig promises to make
this nicer as we can have checked in files corresponding to different
sets of tests.

But it's still annoying 
1) when trying to run a subet of tests
2) when you want to run tests that don't have such a pre-existing
kunitconfig and selecting all the necessary options is tricky.

This patch series aims to alleviate both:
1) `kunit.py run 'my-suite-*'`
I.e. use my current kunitconfig, but just run suites that match this glob
2) `kunit.py run --alltests 'my-suite-*'`
I.e. use allyesconfig so I don't have to worry about writing a
kunitconfig at all.

See the first commit message for more details and discussion about
future work.

This patch series also includes a bugfix for a latent bug that can't be
triggered right now but has worse consequences as a result of the
changes needed to plumb in this suite name glob.

[1] https://lore.kernel.org/linux-kselftest/20210201205514.3943096-1-dlatypov@google.com/

---
v1 -> v2:
  Fix free of `suites` subarray in suite_set.
  Found by Dan Carpenter and kernel test robot.
v2 -> v3:
  Add MODULE_PARM_DESC() for kunit.filter_glob.

Daniel Latypov (3):
  kunit: add kunit.filter_glob cmdline option to filter suites
  kunit: tool: add support for filtering suites by glob
  kunit: tool: fix unintentional statefulness in run_kernel()

 lib/kunit/Kconfig                   |  1 +
 lib/kunit/executor.c                | 93 ++++++++++++++++++++++++++---
 tools/testing/kunit/kunit.py        | 21 +++++--
 tools/testing/kunit/kunit_kernel.py |  6 +-
 4 files changed, 106 insertions(+), 15 deletions(-)


base-commit: 88bb507a74ea7d75fa49edd421eaa710a7d80598
-- 
2.30.0.478.g8a0d178c01-goog

[PATCH v3 1/3] kunit: add kunit.filter_glob cmdline option to filter suites

[Daniel Latypov]

E.g. specifying this would run suites with "list" in their name.
  kunit.filter_glob=list*

Note: the executor prints out a TAP header that includes the number of
suites we intend to run.
So unless we want to report empty results for filtered-out suites, we
need to do the filtering here in the executor.
It's also probably better in the executor since we most likely don't
want any filtering to apply to tests built as modules.

This code does add a CONFIG_GLOB=y dependency for CONFIG_KUNIT=y.
But the code seems light enough that it shouldn't be an issue.

For now, we only filter on suite names so we don't have to create copies
of the suites themselves, just the array (of arrays) holding them.

The name is rather generic since in the future, we could consider
extending it to a syntax like:
  kunit.filter_glob=<suite_glob>.<test_glob>
E.g. to run all the del list tests
  kunit.filter_glob=list-kunit-test.*del*

But at the moment, it's far easier to manually comment out test cases in
test files as opposed to messing with sets of Kconfig entries to select
specific suites.
So even just doing this makes using kunit far less annoying.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
 lib/kunit/Kconfig    |  1 +
 lib/kunit/executor.c | 93 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 85 insertions(+), 9 deletions(-)

diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 00909e6a2443..0b5dfb001bac 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -4,6 +4,7 @@
 
 menuconfig KUNIT
 	tristate "KUnit - Enable support for unit tests"
+	select GLOB if KUNIT=y
 	help
 	  Enables support for kernel unit tests (KUnit), a lightweight unit
 	  testing and mocking framework for the Linux kernel. These tests are
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index a95742a4ece7..15832ed44668 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <kunit/test.h>
+#include <linux/glob.h>
+#include <linux/moduleparam.h>
 
 /*
  * These symbols point to the .kunit_test_suites section and are defined in
@@ -11,14 +13,81 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
 
 #if IS_BUILTIN(CONFIG_KUNIT)
 
-static void kunit_print_tap_header(void)
+static char *filter_glob;
+module_param(filter_glob, charp, 0);
+MODULE_PARM_DESC(filter_glob,
+		"Filter which KUnit test suites run at boot-time, e.g. list*");
+
+static struct kunit_suite * const *
+kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
+{
+	int i, n = 0;
+	struct kunit_suite **filtered;
+
+	n = 0;
+	for (i = 0; subsuite[i] != NULL; ++i) {
+		if (glob_match(filter_glob, subsuite[i]->name))
+			++n;
+	}
+
+	if (n == 0)
+		return NULL;
+
+	filtered = kmalloc_array(n + 1, sizeof(*filtered), GFP_KERNEL);
+	if (!filtered)
+		return NULL;
+
+	n = 0;
+	for (i = 0; subsuite[i] != NULL; ++i) {
+		if (glob_match(filter_glob, subsuite[i]->name))
+			filtered[n++] = subsuite[i];
+	}
+	filtered[n] = NULL;
+
+	return filtered;
+}
+
+struct suite_set {
+	struct kunit_suite * const * const *start;
+	struct kunit_suite * const * const *end;
+};
+
+static struct suite_set kunit_filter_suites(void)
+{
+	int i;
+	struct kunit_suite * const **copy, * const *filtered_subsuite;
+	struct suite_set filtered;
+
+	const size_t max = __kunit_suites_end - __kunit_suites_start;
+
+	if (!filter_glob) {
+		filtered.start = __kunit_suites_start;
+		filtered.end = __kunit_suites_end;
+		return filtered;
+	}
+
+	copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
+	filtered.start = copy;
+	if (!copy) { /* won't be able to run anything, return an empty set */
+		filtered.end = copy;
+		return filtered;
+	}
+
+	for (i = 0; i < max; ++i) {
+		filtered_subsuite = kunit_filter_subsuite(__kunit_suites_start[i]);
+		if (filtered_subsuite)
+			*copy++ = filtered_subsuite;
+	}
+	filtered.end = copy;
+	return filtered;
+}
+
+static void kunit_print_tap_header(struct suite_set *suite_set)
 {
 	struct kunit_suite * const * const *suites, * const *subsuite;
 	int num_of_suites = 0;
 
-	for (suites = __kunit_suites_start;
-	     suites < __kunit_suites_end;
-	     suites++)
+	for (suites = suite_set->start; suites < suite_set->end; suites++)
 		for (subsuite = *suites; *subsuite != NULL; subsuite++)
 			num_of_suites++;
 
@@ -30,12 +99,18 @@ int kunit_run_all_tests(void)
 {
 	struct kunit_suite * const * const *suites;
 
-	kunit_print_tap_header();
+	struct suite_set suite_set = kunit_filter_suites();
+
+	kunit_print_tap_header(&suite_set);
+
+	for (suites = suite_set.start; suites < suite_set.end; suites++)
+		__kunit_test_suites_init(*suites);
 
-	for (suites = __kunit_suites_start;
-	     suites < __kunit_suites_end;
-	     suites++)
-			__kunit_test_suites_init(*suites);
+	if (filter_glob) { /* a copy was made of each array */
+		for (suites = suite_set.start; suites < suite_set.end; suites++)
+			kfree(*suites);
+		kfree(suite_set.start);
+	}
 
 	return 0;
 }
-- 
2.30.0.478.g8a0d178c01-goog

[PATCH v3 2/3] kunit: tool: add support for filtering suites by glob

[Daniel Latypov]

This allows running different subsets of tests, e.g.

$ ./tools/testing/kunit/kunit.py build
$ ./tools/testing/kunit/kunit.py exec 'list*'
$ ./tools/testing/kunit/kunit.py exec 'kunit*'

This passes the "kunit_filter.glob" commandline option to the UML
kernel, which currently only supports filtering by suite name.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
 tools/testing/kunit/kunit.py        | 21 ++++++++++++++++-----
 tools/testing/kunit/kunit_kernel.py |  4 +++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index e808a47c839b..a15ee33bb1f5 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -28,12 +28,12 @@ KunitBuildRequest = namedtuple('KunitBuildRequest',
 			       ['jobs', 'build_dir', 'alltests',
 				'make_options'])
 KunitExecRequest = namedtuple('KunitExecRequest',
-			      ['timeout', 'build_dir', 'alltests'])
+			      ['timeout', 'build_dir', 'alltests', 'filter_glob'])
 KunitParseRequest = namedtuple('KunitParseRequest',
 			       ['raw_output', 'input_data', 'build_dir', 'json'])
 KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
-					   'build_dir', 'alltests', 'json',
-					   'make_options'])
+					   'build_dir', 'alltests', 'filter_glob',
+					   'json', 'make_options'])
 
 KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]
 
@@ -93,6 +93,7 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree,
 	test_start = time.time()
 	result = linux.run_kernel(
 		timeout=None if request.alltests else request.timeout,
+                filter_glob=request.filter_glob,
 		build_dir=request.build_dir)
 
 	test_end = time.time()
@@ -149,7 +150,7 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
 		return build_result
 
 	exec_request = KunitExecRequest(request.timeout, request.build_dir,
-					request.alltests)
+					request.alltests, request.filter_glob)
 	exec_result = exec_tests(linux, exec_request)
 	if exec_result.status != KunitStatus.SUCCESS:
 		return exec_result
@@ -197,6 +198,14 @@ def add_exec_opts(parser) -> None:
 			    type=int,
 			    default=300,
 			    metavar='timeout')
+	parser.add_argument('filter_glob',
+			    help='maximum number of seconds to allow for all tests '
+			    'to run. This does not include time taken to build the '
+			    'tests.',
+			    type=str,
+			    nargs='?',
+			    default='',
+			    metavar='filter_glob')
 
 def add_parse_opts(parser) -> None:
 	parser.add_argument('--raw_output', help='don\'t format output from kernel',
@@ -263,6 +272,7 @@ def main(argv, linux=None):
 				       cli_args.jobs,
 				       cli_args.build_dir,
 				       cli_args.alltests,
+				       cli_args.filter_glob,
 				       cli_args.json,
 				       cli_args.make_options)
 		result = run_tests(linux, request)
@@ -304,7 +314,8 @@ def main(argv, linux=None):
 
 		exec_request = KunitExecRequest(cli_args.timeout,
 						cli_args.build_dir,
-						cli_args.alltests)
+						cli_args.alltests,
+						cli_args.filter_glob)
 		exec_result = exec_tests(linux, exec_request)
 		parse_request = KunitParseRequest(cli_args.raw_output,
 						  exec_result.result,
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 2076a5a2d060..71b1942f5ccd 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -199,8 +199,10 @@ class LinuxSourceTree(object):
 			return False
 		return self.validate_config(build_dir)
 
-	def run_kernel(self, args=[], build_dir='', timeout=None) -> Iterator[str]:
+	def run_kernel(self, args=[], build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
 		args.extend(['mem=1G', 'console=tty'])
+		if filter_glob:
+			args.append('kunit.filter_glob='+filter_glob)
 		self._ops.linux_bin(args, timeout, build_dir)
 		outfile = get_outfile_path(build_dir)
 		subprocess.call(['stty', 'sane'])
-- 
2.30.0.478.g8a0d178c01-goog

[PATCH v3 3/3] kunit: tool: fix unintentional statefulness in run_kernel()

[Daniel Latypov]

This is a bug that has been present since the first version of this
code.
Using [] as a default parameter is dangerous, since it's mutable.

Example using the REPL:
>>> def bad(param = []):
...     param.append(len(param))
...     print(param)
...
>>> bad()
[0]
>>> bad()
[0, 1]

This wasn't a concern in the past since it would just keep appending the
same values to it.

E.g. before, `args` would just grow in size like:
  [mem=1G', 'console=tty']
  [mem=1G', 'console=tty', mem=1G', 'console=tty']

But with now filter_glob, this is more dangerous, e.g.
  run_kernel(filter_glob='my-test*') # default modified here
  run_kernel()			     # filter_glob still applies here!
That earlier `filter_glob` will affect all subsequent calls that don't
specify `args`.

Note: currently the kunit tool only calls run_kernel() at most once, so
it's not possible to trigger any negative side-effects right now.

Fixes: 6ebf5866f2e8 ("kunit: tool: add Python wrappers for running KUnit tests")
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
 tools/testing/kunit/kunit_kernel.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 71b1942f5ccd..6dd3cf6e8efa 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -199,7 +199,9 @@ class LinuxSourceTree(object):
 			return False
 		return self.validate_config(build_dir)
 
-	def run_kernel(self, args=[], build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
+	def run_kernel(self, args=None, build_dir='', filter_glob='', timeout=None) -> Iterator[str]:
+		if not args:
+			args = []
 		args.extend(['mem=1G', 'console=tty'])
 		if filter_glob:
 			args.append('kunit.filter_glob='+filter_glob)
-- 
2.30.0.478.g8a0d178c01-goog

Re: [PATCH v3 0/3] kunit: support running subsets of test suites from kunit.py

[Brendan Higgins]

On Thu, Feb 4, 2021 at 2:54 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> When using `kunit.py run` to run tests, users must populate a
> `kunitconfig` file to select the options the tests are hidden behind and
> all their dependencies.
>
> The patch [1] to allow specifying a path to kunitconfig promises to make
> this nicer as we can have checked in files corresponding to different
> sets of tests.
>
> But it's still annoying
> 1) when trying to run a subet of tests
> 2) when you want to run tests that don't have such a pre-existing
> kunitconfig and selecting all the necessary options is tricky.
>
> This patch series aims to alleviate both:
> 1) `kunit.py run 'my-suite-*'`
> I.e. use my current kunitconfig, but just run suites that match this glob
> 2) `kunit.py run --alltests 'my-suite-*'`
> I.e. use allyesconfig so I don't have to worry about writing a
> kunitconfig at all.
>
> See the first commit message for more details and discussion about
> future work.
>
> This patch series also includes a bugfix for a latent bug that can't be
> triggered right now but has worse consequences as a result of the
> changes needed to plumb in this suite name glob.
>
> [1] https://lore.kernel.org/linux-kselftest/20210201205514.3943096-1-dlatypov@google.com/

I tried applying this series on top of several of your other patches
over the past cycle and running the kunit_tool unit tests
(kunit_tool_test.py) and I got several failures:

======================================================================
FAIL: test_exec_builddir (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 417, in test_exec_builddir
    self.linux_source_mock.run_kernel.assert_called_once_with(build_dir=build_dir,
timeout=300)
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=300)
Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit')

======================================================================
FAIL: test_exec_passes_args_pass (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 337, in
test_exec_passes_args_pass
    self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit',
timeout=300)
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=300)
Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit')

======================================================================
FAIL: test_exec_timeout (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 385, in test_exec_timeout
    self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit',
timeout=timeout)
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=3453)
Actual: run_kernel(timeout=3453, filter_glob='', build_dir='.kunit')

======================================================================
FAIL: test_run_builddir (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 400, in test_run_builddir
    self.linux_source_mock.run_kernel.assert_called_once_with(
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=300)
Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit')

======================================================================
FAIL: test_run_passes_args_pass (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 344, in
test_run_passes_args_pass
    self.linux_source_mock.run_kernel.assert_called_once_with(
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=300)
Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit')

======================================================================
FAIL: test_run_timeout (__main__.KUnitMainTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tools/testing/kunit/kunit_tool_test.py", line 392, in test_run_timeout
    self.linux_source_mock.run_kernel.assert_called_once_with(
  File "/usr/lib/python3.8/unittest/mock.py", line 925, in
assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
  File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: run_kernel(build_dir='.kunit', timeout=3453)
Actual: run_kernel(timeout=3453, filter_glob='', build_dir='.kunit')

----------------------------------------------------------------------
Ran 37 tests in 0.082s

FAILED (failures=6)