[PATCH] kunit: add unit test for filtering suites by names

[PATCH] kunit: add unit test for filtering suites by names

[Daniel Latypov]

This adds unit tests for kunit_filter_subsuite() and
kunit_filter_suites().

Note: what the executor means by "subsuite" is the array of suites
corresponding to each test file.

This patch lightly refactors executor.c to avoid the use of global
variables to make it testable.
It also includes a clever `kfree_at_end()` helper that makes this test
easier to write than it otherwise would have been.

Tested by running just the new tests using itself
$ ./tools/testing/kunit/kunit.py run '*exec*'

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 lib/kunit/executor.c      |  26 ++++----
 lib/kunit/executor_test.c | 132 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 11 deletions(-)
 create mode 100644 lib/kunit/executor_test.c

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 15832ed44668..96a4ae983786 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -19,7 +19,7 @@ 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)
+kunit_filter_subsuite(struct kunit_suite * const * const subsuite, const char *filter_glob)
 {
 	int i, n = 0;
 	struct kunit_suite **filtered;
@@ -52,19 +52,14 @@ struct suite_set {
 	struct kunit_suite * const * const *end;
 };
 
-static struct suite_set kunit_filter_suites(void)
+static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
+					    const char *filter_glob)
 {
 	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;
-	}
+	const size_t max = suite_set->end - suite_set->start;
 
 	copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
 	filtered.start = copy;
@@ -74,7 +69,7 @@ static struct suite_set kunit_filter_suites(void)
 	}
 
 	for (i = 0; i < max; ++i) {
-		filtered_subsuite = kunit_filter_subsuite(__kunit_suites_start[i]);
+		filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], filter_glob);
 		if (filtered_subsuite)
 			*copy++ = filtered_subsuite;
 	}
@@ -98,8 +93,13 @@ static void kunit_print_tap_header(struct suite_set *suite_set)
 int kunit_run_all_tests(void)
 {
 	struct kunit_suite * const * const *suites;
+	struct suite_set suite_set = {
+		.start = __kunit_suites_start,
+		.end = __kunit_suites_end,
+	};
 
-	struct suite_set suite_set = kunit_filter_suites();
+	if (filter_glob)
+		suite_set = kunit_filter_suites(&suite_set, filter_glob);
 
 	kunit_print_tap_header(&suite_set);
 
@@ -115,4 +115,8 @@ int kunit_run_all_tests(void)
 	return 0;
 }
 
+#if IS_BUILTIN(CONFIG_KUNIT_TEST)
+#include "executor_test.c"
+#endif
+
 #endif /* IS_BUILTIN(CONFIG_KUNIT) */
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
new file mode 100644
index 000000000000..8e925395beeb
--- /dev/null
+++ b/lib/kunit/executor_test.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the KUnit executor.
+ *
+ * Copyright (C) 2021, Google LLC.
+ * Author: Daniel Latypov <dlatypov@google.com>
+ */
+
+#include <kunit/test.h>
+
+static void kfree_at_end(struct kunit *test, const void *to_free);
+static struct kunit_suite *alloc_fake_suite(struct kunit *test,
+					    const char *suite_name);
+
+static void filter_subsuite_test(struct kunit *test)
+{
+	struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
+	struct kunit_suite * const *filtered;
+
+	subsuite[0] = alloc_fake_suite(test, "suite1");
+	subsuite[1] = alloc_fake_suite(test, "suite2");
+
+	/* Want: suite1, suite2, NULL -> suite2, NULL */
+	filtered = kunit_filter_subsuite(subsuite, "suite2*");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered);
+	kfree_at_end(test, filtered);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]);
+	KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2");
+
+	KUNIT_EXPECT_FALSE(test, filtered[1]);
+}
+
+static void filter_subsuite_to_empty_test(struct kunit *test)
+{
+	struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
+	struct kunit_suite * const *filtered;
+
+	subsuite[0] = alloc_fake_suite(test, "suite1");
+	subsuite[1] = alloc_fake_suite(test, "suite2");
+
+	filtered = kunit_filter_subsuite(subsuite, "not_found");
+	kfree_at_end(test, filtered); /* just in case */
+
+	KUNIT_EXPECT_FALSE_MSG(test, filtered,
+			       "should be NULL to indicate no match");
+}
+
+static void kfree_subsuites_at_end(struct kunit *test, struct suite_set *suite_set)
+{
+	struct kunit_suite * const * const *suites;
+
+	for (suites = suite_set->start; suites < suite_set->end; suites++)
+		kfree_at_end(test, *suites);
+}
+
+static void filter_suites_test(struct kunit *test)
+{
+	/* Suites per-file are stored as a NULL terminated array */
+	struct kunit_suite *subsuites[2][2] = {
+		{NULL, NULL},
+		{NULL, NULL},
+	};
+	/* Match the memory layout of suite_set */
+	struct kunit_suite * const * const suites[2] = {
+		subsuites[0], subsuites[1],
+	};
+
+	const struct suite_set suite_set = {
+		.start = suites,
+		.end = suites + 2,
+	};
+	struct suite_set filtered = {.start = NULL, .end = NULL};
+
+	/* Emulate two files, each having one suite */
+	subsuites[0][0] = alloc_fake_suite(test, "suite0");
+	subsuites[1][0] = alloc_fake_suite(test, "suite1");
+
+	/* Filter out suite1 */
+	filtered = kunit_filter_suites(&suite_set, "suite0");
+	kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */
+	KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t) 1);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0]);
+	KUNIT_EXPECT_PTR_EQ(test, filtered.start[0][0], subsuites[0][0]);
+}
+
+static struct kunit_case executor_test_cases[] = {
+	KUNIT_CASE(filter_subsuite_test),
+	KUNIT_CASE(filter_subsuite_to_empty_test),
+	KUNIT_CASE(filter_suites_test),
+	{}
+};
+
+static struct kunit_suite executor_test_suite = {
+	.name = "kunit_executor_test",
+	.test_cases = executor_test_cases,
+};
+
+kunit_test_suites(&executor_test_suite);
+
+/* Test helpers */
+
+static void kfree_res_free(struct kunit_resource *res)
+{
+	kfree(res->data);
+}
+
+/* Use the resource API to register a call to kfree(to_free).
+ * Since we never actually use the resource, it's safe to use on const data.
+ */
+static void kfree_at_end(struct kunit *test, const void *to_free)
+{
+	/* kfree() handles NULL already, but avoid allocating a no-op cleanup. */
+	if (IS_ERR_OR_NULL(to_free))
+		return;
+	kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL,
+				     (void *)to_free);
+}
+
+static struct kunit_suite *alloc_fake_suite(struct kunit *test,
+					    const char *suite_name)
+{
+	struct kunit_suite *suite;
+
+	/* We normally never expect to allocate suites, hence the non-const cast. */
+	suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL);
+	strncpy((char *)suite->name, suite_name, sizeof(suite->name));
+
+	return suite;
+}

base-commit: 1678e493d530e7977cce34e59a86bb86f3c5631e
-- 
2.31.1.295.g9ea45b61b8-goog

Re: [PATCH] kunit: add unit test for filtering suites by names

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 9421 bytes --]

On Tue, Apr 13, 2021 at 8:08 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> This adds unit tests for kunit_filter_subsuite() and
> kunit_filter_suites().
>
> Note: what the executor means by "subsuite" is the array of suites
> corresponding to each test file.
>
> This patch lightly refactors executor.c to avoid the use of global
> variables to make it testable.
> It also includes a clever `kfree_at_end()` helper that makes this test
> easier to write than it otherwise would have been.
>
> Tested by running just the new tests using itself
> $ ./tools/testing/kunit/kunit.py run '*exec*'
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

I really like this test, thanks.

A few small notes below, including what I think is a missing
kfree_at_end() call.

Assuming that one issue is fixed (or I'm mistaken):
Reviewed-by: David Gow <davidgow@google.com>

-- David

> ---
>  lib/kunit/executor.c      |  26 ++++----
>  lib/kunit/executor_test.c | 132 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 147 insertions(+), 11 deletions(-)
>  create mode 100644 lib/kunit/executor_test.c
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 15832ed44668..96a4ae983786 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -19,7 +19,7 @@ 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)
> +kunit_filter_subsuite(struct kunit_suite * const * const subsuite, const char *filter_glob)
>  {
>         int i, n = 0;
>         struct kunit_suite **filtered;
> @@ -52,19 +52,14 @@ struct suite_set {
>         struct kunit_suite * const * const *end;
>  };
>
> -static struct suite_set kunit_filter_suites(void)
> +static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
> +                                           const char *filter_glob)
>  {
>         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;
> -       }
> +       const size_t max = suite_set->end - suite_set->start;
>
>         copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
>         filtered.start = copy;
> @@ -74,7 +69,7 @@ static struct suite_set kunit_filter_suites(void)
>         }
>
>         for (i = 0; i < max; ++i) {
> -               filtered_subsuite = kunit_filter_subsuite(__kunit_suites_start[i]);
> +               filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], filter_glob);
>                 if (filtered_subsuite)
>                         *copy++ = filtered_subsuite;
>         }
> @@ -98,8 +93,13 @@ static void kunit_print_tap_header(struct suite_set *suite_set)
>  int kunit_run_all_tests(void)
>  {
>         struct kunit_suite * const * const *suites;
> +       struct suite_set suite_set = {
> +               .start = __kunit_suites_start,
> +               .end = __kunit_suites_end,
> +       };
>
> -       struct suite_set suite_set = kunit_filter_suites();
> +       if (filter_glob)
> +               suite_set = kunit_filter_suites(&suite_set, filter_glob);
>
>         kunit_print_tap_header(&suite_set);
>
> @@ -115,4 +115,8 @@ int kunit_run_all_tests(void)
>         return 0;
>  }
>
> +#if IS_BUILTIN(CONFIG_KUNIT_TEST)
> +#include "executor_test.c"
> +#endif
> +
>  #endif /* IS_BUILTIN(CONFIG_KUNIT) */
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> new file mode 100644
> index 000000000000..8e925395beeb
> --- /dev/null
> +++ b/lib/kunit/executor_test.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the KUnit executor.
> + *
> + * Copyright (C) 2021, Google LLC.
> + * Author: Daniel Latypov <dlatypov@google.com>
> + */
> +
> +#include <kunit/test.h>
> +
> +static void kfree_at_end(struct kunit *test, const void *to_free);
> +static struct kunit_suite *alloc_fake_suite(struct kunit *test,
> +                                           const char *suite_name);
> +
> +static void filter_subsuite_test(struct kunit *test)
> +{
> +       struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
> +       struct kunit_suite * const *filtered;
> +
> +       subsuite[0] = alloc_fake_suite(test, "suite1");
> +       subsuite[1] = alloc_fake_suite(test, "suite2");
> +
> +       /* Want: suite1, suite2, NULL -> suite2, NULL */
> +       filtered = kunit_filter_subsuite(subsuite, "suite2*");
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered);
> +       kfree_at_end(test, filtered);
> +
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]);
> +       KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2");

Is it worth testing that filtered[0] == subsuite[1], not just the
name? (I suspect it doesn't really matter, but that seems to be what's
happening in filter_suites_test() below.)

> +
> +       KUNIT_EXPECT_FALSE(test, filtered[1]);
> +}
> +
> +static void filter_subsuite_to_empty_test(struct kunit *test)
> +{
> +       struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
> +       struct kunit_suite * const *filtered;
> +
> +       subsuite[0] = alloc_fake_suite(test, "suite1");
> +       subsuite[1] = alloc_fake_suite(test, "suite2");
> +
> +       filtered = kunit_filter_subsuite(subsuite, "not_found");
> +       kfree_at_end(test, filtered); /* just in case */
> +
> +       KUNIT_EXPECT_FALSE_MSG(test, filtered,
> +                              "should be NULL to indicate no match");
> +}
> +
> +static void kfree_subsuites_at_end(struct kunit *test, struct suite_set *suite_set)
> +{
> +       struct kunit_suite * const * const *suites;
> +
> +       for (suites = suite_set->start; suites < suite_set->end; suites++)
> +               kfree_at_end(test, *suites);
> +}
> +
> +static void filter_suites_test(struct kunit *test)
> +{
> +       /* Suites per-file are stored as a NULL terminated array */
> +       struct kunit_suite *subsuites[2][2] = {
> +               {NULL, NULL},
> +               {NULL, NULL},
> +       };
> +       /* Match the memory layout of suite_set */
> +       struct kunit_suite * const * const suites[2] = {
> +               subsuites[0], subsuites[1],
> +       };
> +
> +       const struct suite_set suite_set = {
> +               .start = suites,
> +               .end = suites + 2,
> +       };
> +       struct suite_set filtered = {.start = NULL, .end = NULL};
> +
> +       /* Emulate two files, each having one suite */
> +       subsuites[0][0] = alloc_fake_suite(test, "suite0");
> +       subsuites[1][0] = alloc_fake_suite(test, "suite1");
> +
> +       /* Filter out suite1 */
> +       filtered = kunit_filter_suites(&suite_set, "suite0");
> +       kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */

Do we also need to kfree_at_end(test, &filtered.start) here?

> +       KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t) 1);
> +
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0]);
> +       KUNIT_EXPECT_PTR_EQ(test, filtered.start[0][0], subsuites[0][0]);
> +}
> +
> +static struct kunit_case executor_test_cases[] = {
> +       KUNIT_CASE(filter_subsuite_test),
> +       KUNIT_CASE(filter_subsuite_to_empty_test),
> +       KUNIT_CASE(filter_suites_test),
> +       {}
> +};
> +
> +static struct kunit_suite executor_test_suite = {
> +       .name = "kunit_executor_test",
> +       .test_cases = executor_test_cases,
> +};
> +
> +kunit_test_suites(&executor_test_suite);
> +
> +/* Test helpers */
> +
> +static void kfree_res_free(struct kunit_resource *res)
> +{
> +       kfree(res->data);
> +}
> +
> +/* Use the resource API to register a call to kfree(to_free).
> + * Since we never actually use the resource, it's safe to use on const data.
> + */
> +static void kfree_at_end(struct kunit *test, const void *to_free)
> +{
> +       /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */
> +       if (IS_ERR_OR_NULL(to_free))
> +               return;
> +       kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL,
> +                                    (void *)to_free);
> +}

This actually seems useful enough to move out of this test and have as
part of the KUnit framework proper. Admittedly, I normally am very
sceptical about doing this when there's only one user, but this does
seem more obviously useful than most things. As a bonus, it could
reuse the kunit_kmalloc_free() function, rather than having its own
kfree_res_free() helper.

> +
> +static struct kunit_suite *alloc_fake_suite(struct kunit *test,
> +                                           const char *suite_name)
> +{
> +       struct kunit_suite *suite;
> +
> +       /* We normally never expect to allocate suites, hence the non-const cast. */
> +       suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL);
> +       strncpy((char *)suite->name, suite_name, sizeof(suite->name));
> +
> +       return suite;
> +}
>
> base-commit: 1678e493d530e7977cce34e59a86bb86f3c5631e
> --
> 2.31.1.295.g9ea45b61b8-goog
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4000 bytes --]

Re: [PATCH] kunit: add unit test for filtering suites by names

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 1678e493d530e7977cce34e59a86bb86f3c5631e]

url:    https://github.com/0day-ci/linux/commits/Daniel-Latypov/kunit-add-unit-test-for-filtering-suites-by-names/20210413-080913
base:   1678e493d530e7977cce34e59a86bb86f3c5631e
config: microblaze-randconfig-r014-20210413 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/756df216f1586cecdf02f278fbed232fb25fa3f7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Latypov/kunit-add-unit-test-for-filtering-suites-by-names/20210413-080913
        git checkout 756df216f1586cecdf02f278fbed232fb25fa3f7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from lib/kunit/executor.c:119:
   lib/kunit/executor_test.c: In function 'alloc_fake_suite':
>> lib/kunit/executor_test.c:129:2: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
     129 |  strncpy((char *)suite->name, suite_name, sizeof(suite->name));
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/strncpy +129 lib/kunit/executor_test.c

   121	
   122	static struct kunit_suite *alloc_fake_suite(struct kunit *test,
   123						    const char *suite_name)
   124	{
   125		struct kunit_suite *suite;
   126	
   127		/* We normally never expect to allocate suites, hence the non-const cast. */
   128		suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL);
 > 129		strncpy((char *)suite->name, suite_name, sizeof(suite->name));

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28523 bytes --]

Re: [PATCH] kunit: add unit test for filtering suites by names

[Daniel Latypov]

On Mon, Apr 12, 2021 at 10:00 PM David Gow <davidgow@google.com> wrote:
>
> On Tue, Apr 13, 2021 at 8:08 AM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > This adds unit tests for kunit_filter_subsuite() and
> > kunit_filter_suites().
> >
> > Note: what the executor means by "subsuite" is the array of suites
> > corresponding to each test file.
> >
> > This patch lightly refactors executor.c to avoid the use of global
> > variables to make it testable.
> > It also includes a clever `kfree_at_end()` helper that makes this test
> > easier to write than it otherwise would have been.
> >
> > Tested by running just the new tests using itself
> > $ ./tools/testing/kunit/kunit.py run '*exec*'
> >
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
>
> I really like this test, thanks.
>
> A few small notes below, including what I think is a missing
> kfree_at_end() call.
>
> Assuming that one issue is fixed (or I'm mistaken):
> Reviewed-by: David Gow <davidgow@google.com>
>
> -- David
>
> > ---
> >  lib/kunit/executor.c      |  26 ++++----
> >  lib/kunit/executor_test.c | 132 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 147 insertions(+), 11 deletions(-)
> >  create mode 100644 lib/kunit/executor_test.c
> >
> > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> > index 15832ed44668..96a4ae983786 100644
> > --- a/lib/kunit/executor.c
> > +++ b/lib/kunit/executor.c
> > @@ -19,7 +19,7 @@ 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)
> > +kunit_filter_subsuite(struct kunit_suite * const * const subsuite, const char *filter_glob)
> >  {
> >         int i, n = 0;
> >         struct kunit_suite **filtered;
> > @@ -52,19 +52,14 @@ struct suite_set {
> >         struct kunit_suite * const * const *end;
> >  };
> >
> > -static struct suite_set kunit_filter_suites(void)
> > +static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
> > +                                           const char *filter_glob)
> >  {
> >         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;
> > -       }
> > +       const size_t max = suite_set->end - suite_set->start;
> >
> >         copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
> >         filtered.start = copy;
> > @@ -74,7 +69,7 @@ static struct suite_set kunit_filter_suites(void)
> >         }
> >
> >         for (i = 0; i < max; ++i) {
> > -               filtered_subsuite = kunit_filter_subsuite(__kunit_suites_start[i]);
> > +               filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], filter_glob);
> >                 if (filtered_subsuite)
> >                         *copy++ = filtered_subsuite;
> >         }
> > @@ -98,8 +93,13 @@ static void kunit_print_tap_header(struct suite_set *suite_set)
> >  int kunit_run_all_tests(void)
> >  {
> >         struct kunit_suite * const * const *suites;
> > +       struct suite_set suite_set = {
> > +               .start = __kunit_suites_start,
> > +               .end = __kunit_suites_end,
> > +       };
> >
> > -       struct suite_set suite_set = kunit_filter_suites();
> > +       if (filter_glob)
> > +               suite_set = kunit_filter_suites(&suite_set, filter_glob);
> >
> >         kunit_print_tap_header(&suite_set);
> >
> > @@ -115,4 +115,8 @@ int kunit_run_all_tests(void)
> >         return 0;
> >  }
> >
> > +#if IS_BUILTIN(CONFIG_KUNIT_TEST)
> > +#include "executor_test.c"
> > +#endif
> > +
> >  #endif /* IS_BUILTIN(CONFIG_KUNIT) */
> > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> > new file mode 100644
> > index 000000000000..8e925395beeb
> > --- /dev/null
> > +++ b/lib/kunit/executor_test.c
> > @@ -0,0 +1,132 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit test for the KUnit executor.
> > + *
> > + * Copyright (C) 2021, Google LLC.
> > + * Author: Daniel Latypov <dlatypov@google.com>
> > + */
> > +
> > +#include <kunit/test.h>
> > +
> > +static void kfree_at_end(struct kunit *test, const void *to_free);
> > +static struct kunit_suite *alloc_fake_suite(struct kunit *test,
> > +                                           const char *suite_name);
> > +
> > +static void filter_subsuite_test(struct kunit *test)
> > +{
> > +       struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
> > +       struct kunit_suite * const *filtered;
> > +
> > +       subsuite[0] = alloc_fake_suite(test, "suite1");
> > +       subsuite[1] = alloc_fake_suite(test, "suite2");
> > +
> > +       /* Want: suite1, suite2, NULL -> suite2, NULL */
> > +       filtered = kunit_filter_subsuite(subsuite, "suite2*");
> > +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered);
> > +       kfree_at_end(test, filtered);
> > +
> > +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]);
> > +       KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2");
>
> Is it worth testing that filtered[0] == subsuite[1], not just the
> name? (I suspect it doesn't really matter, but that seems to be what's
> happening in filter_suites_test() below.)

I'll update filter_suites_test() to check the name instead as well.
My reason reason for preferring checking the name is because if we
ever support filtering on test names as well as suites, then the suite
pointers will no longer be unchanged.

Semi-rant about why I didn't do that before in filter_suites_test():
The intuitive approach
  KUNIT_EXPECT_PTR_EQ(test, filtered.start[0][0], subsuites[0][0]);
fails to compile with a very uninformative compiler error, see [1]
  KUNIT_EXPECT_STREQ(test, (const char *)filtered.start[0][0]->name, "suite0");
_does_ compile, but is ugly.

[1] useless compile error due to how the kunit expect macros are set up:
ERROR:root:In file included from ../lib/kunit/executor.c:3:
../lib/kunit/executor_test.c: In function ‘filter_suites_test’:
../include/kunit/test.h:1132:24: error: invalid initializer
 1132 |  typeof(left) __left = (left);            \
      |                        ^
../include/kunit/test.h:1155:2: note: in expansion of macro
‘KUNIT_BINARY_STR_ASSERTION’
 1155 |  KUNIT_BINARY_STR_ASSERTION(test,           \
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
../include/kunit/test.h:1162:2: note: in expansion of macro
‘KUNIT_BINARY_STR_EQ_MSG_ASSERTION’
 1162 |  KUNIT_BINARY_STR_EQ_MSG_ASSERTION(test,           \
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/kunit/test.h:1446:2: note: in expansion of macro
‘KUNIT_BINARY_STR_EQ_ASSERTION’
 1446 |  KUNIT_BINARY_STR_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/kunit/executor_test.c:87:2: note: in expansion of macro
‘KUNIT_EXPECT_STREQ’
   87 |  KUNIT_EXPECT_STREQ(test, filtered.start[0][0]->name, "suite0");
      |  ^~~~~~~~~~~~~~~~~~

Root cause: kunit_suite.name is a const char[256] and the expect
macros don't play kindly with arrays :(
Note: nowhere in the error output does it mention any of the relevant types!
It's just complaining that you can't initialize array variables with
anything besides a string literal or {}-enclosed expression.

>
> > +
> > +       KUNIT_EXPECT_FALSE(test, filtered[1]);
> > +}
> > +
> > +static void filter_subsuite_to_empty_test(struct kunit *test)
> > +{
> > +       struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
> > +       struct kunit_suite * const *filtered;
> > +
> > +       subsuite[0] = alloc_fake_suite(test, "suite1");
> > +       subsuite[1] = alloc_fake_suite(test, "suite2");
> > +
> > +       filtered = kunit_filter_subsuite(subsuite, "not_found");
> > +       kfree_at_end(test, filtered); /* just in case */
> > +
> > +       KUNIT_EXPECT_FALSE_MSG(test, filtered,
> > +                              "should be NULL to indicate no match");
> > +}
> > +
> > +static void kfree_subsuites_at_end(struct kunit *test, struct suite_set *suite_set)
> > +{
> > +       struct kunit_suite * const * const *suites;
> > +
> > +       for (suites = suite_set->start; suites < suite_set->end; suites++)
> > +               kfree_at_end(test, *suites);
> > +}
> > +
> > +static void filter_suites_test(struct kunit *test)
> > +{
> > +       /* Suites per-file are stored as a NULL terminated array */
> > +       struct kunit_suite *subsuites[2][2] = {
> > +               {NULL, NULL},
> > +               {NULL, NULL},
> > +       };
> > +       /* Match the memory layout of suite_set */
> > +       struct kunit_suite * const * const suites[2] = {
> > +               subsuites[0], subsuites[1],
> > +       };
> > +
> > +       const struct suite_set suite_set = {
> > +               .start = suites,
> > +               .end = suites + 2,
> > +       };
> > +       struct suite_set filtered = {.start = NULL, .end = NULL};
> > +
> > +       /* Emulate two files, each having one suite */
> > +       subsuites[0][0] = alloc_fake_suite(test, "suite0");
> > +       subsuites[1][0] = alloc_fake_suite(test, "suite1");
> > +
> > +       /* Filter out suite1 */
> > +       filtered = kunit_filter_suites(&suite_set, "suite0");
> > +       kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */
>
> Do we also need to kfree_at_end(test, &filtered.start) here?

Ah, I meant to have that in kfree_subsuites_at_end().
Added the call there.

>
> > +       KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t) 1);
> > +
> > +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start);
> > +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0]);
> > +       KUNIT_EXPECT_PTR_EQ(test, filtered.start[0][0], subsuites[0][0]);
> > +}
> > +
> > +static struct kunit_case executor_test_cases[] = {
> > +       KUNIT_CASE(filter_subsuite_test),
> > +       KUNIT_CASE(filter_subsuite_to_empty_test),
> > +       KUNIT_CASE(filter_suites_test),
> > +       {}
> > +};
> > +
> > +static struct kunit_suite executor_test_suite = {
> > +       .name = "kunit_executor_test",
> > +       .test_cases = executor_test_cases,
> > +};
> > +
> > +kunit_test_suites(&executor_test_suite);
> > +
> > +/* Test helpers */
> > +
> > +static void kfree_res_free(struct kunit_resource *res)
> > +{
> > +       kfree(res->data);
> > +}
> > +
> > +/* Use the resource API to register a call to kfree(to_free).
> > + * Since we never actually use the resource, it's safe to use on const data.
> > + */
> > +static void kfree_at_end(struct kunit *test, const void *to_free)
> > +{
> > +       /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */
> > +       if (IS_ERR_OR_NULL(to_free))
> > +               return;
> > +       kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL,
> > +                                    (void *)to_free);
> > +}
>
> This actually seems useful enough to move out of this test and have as
> part of the KUnit framework proper. Admittedly, I normally am very
> sceptical about doing this when there's only one user, but this does
> seem more obviously useful than most things. As a bonus, it could
> reuse the kunit_kmalloc_free() function, rather than having its own
> kfree_res_free() helper.

I'm still a bit more on the fence about it, leaning towards not adding
it into KUnit.
I'm not too sure how much a plain `kfree()` would be needed outside of
tests, and I know there's plenty of variant xxxfree(void *) functions.

So a more generic form like [2] might be useful, but...

By my count, we have ~100 (this is >= the actual # since it double
counts decls+defs)
$ ag --nomultiline 'void .*free\((const )?void \*[^,]+\)' | wc -l
139

Sample output to make sure the regex works:
$ ag --nomultiline 'void .*free\((const )?void \*[^,]+\)' | head -5
arch/parisc/boot/compressed/misc.c:213:void free(void *ptr)
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:59:void
mpc85xx_cache_sram_free(void *ptr)
arch/powerpc/boot/simple_alloc.c:83:static void simple_free(void *ptr)
arch/powerpc/boot/ops.h:222:static inline void free(void *ptr)
arch/powerpc/include/asm/fsl_85xx_cache_sram.h:31:extern void
mpc85xx_cache_sram_free(void *ptr);

But hmm, it seems like non-const is far more common than `const void *`.
Looking at a few, they really should be `const void *`, sigh...
We could cast away the differences, but that feels a bit iffy in the
case where the function might actually need them to be non-const.

So to do that "properly", I think we need a const and non-const
version of the interface?
If so, I don't think that's justified quite yet.

[2] more generic interface
struct kunit_cleanup_res {
        void (*cleanup)(const void *);
        const void *data;
};

static void kunit_cleanup_res(struct kunit_resource *res) {
        struct kunit_cleanup_res *c = res->data;
        c->cleanup(c->data);
};

static void kunit_register_cleanup(struct kunit *test, void
(*cleanup)(const void *), const void *data)
{
        struct kunit_cleanup_res *res;

        if (IS_ERR_OR_NULL(data))
                return;

        res = kunit_kmalloc(test, sizeof(*res), GFP_KERNEL);
        res->cleanup = cleanup;
        res->data = data;

        kunit_alloc_and_get_resource(test, NULL, kunit_cleanup_res,
GFP_KERNEL, res);
}

static void kfree_at_end(struct kunit *test, const void *to_free)
{
        kunit_register_cleanup(test, kfree, to_free);

}

>
> > +
> > +static struct kunit_suite *alloc_fake_suite(struct kunit *test,
> > +                                           const char *suite_name)
> > +{
> > +       struct kunit_suite *suite;
> > +
> > +       /* We normally never expect to allocate suites, hence the non-const cast. */
> > +       suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL);
> > +       strncpy((char *)suite->name, suite_name, sizeof(suite->name));
> > +
> > +       return suite;
> > +}
> >
> > base-commit: 1678e493d530e7977cce34e59a86bb86f3c5631e
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >