Per [1], we might not need the array-of-array of kunit_suite's.
This RFC patch previews the changes we'd make to the executor to accommodate that by making the executor automatically flatten the kunit_suite*** into a kunit_suite**.
The test filtering support [2] added the largest dependency on the current kunit_suite*** layout, so this patch is based on that.
It actually drastically simplifies the code, so it might be useful to keep the auto-flattening step until we actually make the change.
[1] https://lore.kernel.org/linux-kselftest/101d12fc9250b7a445ff50a9e7a25cd74d0e... [2] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/co...
Cc: Jeremy Kerr jk@codeconstruct.com.au Signed-off-by: Daniel Latypov dlatypov@google.com --- lib/kunit/executor.c | 132 +++++++++++++++----------------------- lib/kunit/executor_test.c | 131 ++++++++++--------------------------- 2 files changed, 85 insertions(+), 178 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 22640c9ee819..3a7246336625 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -88,60 +88,18 @@ kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob) static char *kunit_shutdown; core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
-static struct kunit_suite * const * -kunit_filter_subsuite(struct kunit_suite * const * const subsuite, - struct kunit_test_filter *filter) -{ - int i, n = 0; - struct kunit_suite **filtered, *filtered_suite; - - n = 0; - for (i = 0; subsuite[i]; ++i) { - if (glob_match(filter->suite_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->suite_glob, subsuite[i]->name)) - continue; - filtered_suite = kunit_filter_tests(subsuite[i], filter->test_glob); - if (filtered_suite) - filtered[n++] = filtered_suite; - } - filtered[n] = NULL; - - return filtered; -} - +/* Stores a NULL-terminated array of suites. */ struct suite_set { - struct kunit_suite * const * const *start; - struct kunit_suite * const * const *end; + struct kunit_suite * const *start; + struct kunit_suite * const *end; };
-static void kunit_free_subsuite(struct kunit_suite * const *subsuite) -{ - unsigned int i; - - for (i = 0; subsuite[i]; i++) - kfree(subsuite[i]); - - kfree(subsuite); -} - static void kunit_free_suite_set(struct suite_set suite_set) { - struct kunit_suite * const * const *suites; + struct kunit_suite * const *suites;
for (suites = suite_set.start; suites < suite_set.end; suites++) - kunit_free_subsuite(*suites); + kfree(*suites); kfree(suite_set.start); }
@@ -149,10 +107,11 @@ 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 kunit_suite **copy, *filtered_suite; struct suite_set filtered; struct kunit_test_filter filter;
+ /* Note: this includes space for the terminating NULL. */ const size_t max = suite_set->end - suite_set->start;
copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); @@ -164,11 +123,17 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
kunit_parse_filter_glob(&filter, filter_glob);
- for (i = 0; i < max; ++i) { - filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], &filter); - if (filtered_subsuite) - *copy++ = filtered_subsuite; + for (i = 0; suite_set->start[i] != NULL; i++) { + if (!glob_match(filter.suite_glob, suite_set->start[i]->name)) + continue; + + filtered_suite = kunit_filter_tests(suite_set->start[i], filter.test_glob); + if (!filtered_suite) + continue; + + *copy++ = filtered_suite; } + *copy = NULL; filtered.end = copy;
kfree(filter.suite_glob); @@ -190,52 +155,56 @@ static void kunit_handle_shutdown(void)
}
-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 = suite_set->start; suites < suite_set->end; suites++) - for (subsuite = *suites; *subsuite != NULL; subsuite++) - num_of_suites++; - - pr_info("TAP version 14\n"); - pr_info("1..%d\n", num_of_suites); -} - static void kunit_exec_run_tests(struct suite_set *suite_set) { - struct kunit_suite * const * const *suites; - - kunit_print_tap_header(suite_set); + pr_info("TAP version 14\n"); + pr_info("1..%zu\n", suite_set->end - suite_set->start);
- for (suites = suite_set->start; suites < suite_set->end; suites++) - __kunit_test_suites_init(*suites); + __kunit_test_suites_init(suite_set->start); }
static void kunit_exec_list_tests(struct suite_set *suite_set) { - unsigned int i; - struct kunit_suite * const * const *suites; + struct kunit_suite * const *suites; struct kunit_case *test_case;
/* Hack: print a tap header so kunit.py can find the start of KUnit output. */ pr_info("TAP version 14\n");
for (suites = suite_set->start; suites < suite_set->end; suites++) - for (i = 0; (*suites)[i] != NULL; i++) { - kunit_suite_for_each_test_case((*suites)[i], test_case) { - pr_info("%s.%s\n", (*suites)[i]->name, test_case->name); - } + kunit_suite_for_each_test_case((*suites), test_case) { + pr_info("%s.%s\n", (*suites)->name, test_case->name); } }
+// TODO(dlatypov@google.com): delete this when we store suites in a single array. +static struct suite_set make_suite_set(void) +{ + struct suite_set flattened; + size_t num_of_suites = 0; + + struct kunit_suite * const * const *suites, * const *subsuite; + struct kunit_suite **end; + + for (suites = __kunit_suites_start; suites < __kunit_suites_end; suites++) + for (subsuite = *suites; *subsuite != NULL; subsuite++) + num_of_suites++; + + end = kcalloc(num_of_suites + 1, sizeof(*flattened.start), GFP_KERNEL); + flattened.start = end; + + for (suites = __kunit_suites_start; suites < __kunit_suites_end; suites++) + for (subsuite = *suites; *subsuite != NULL; subsuite++) + *end++ = *subsuite; + *end = NULL; + flattened.end = end; + return flattened; +} + int kunit_run_all_tests(void) { - struct suite_set suite_set = { - .start = __kunit_suites_start, - .end = __kunit_suites_end, - }; + struct suite_set suite_set = make_suite_set(); + struct kunit_suite * const *unfiltered = suite_set.start; /* need to free at end */
if (filter_glob_param) suite_set = kunit_filter_suites(&suite_set, filter_glob_param); @@ -247,9 +216,10 @@ int kunit_run_all_tests(void) else pr_err("kunit executor: unknown action '%s'\n", action_param);
- if (filter_glob_param) { /* a copy was made of each array */ + if (filter_glob_param) { /* a copy was made of each suite */ kunit_free_suite_set(suite_set); } + kfree(unfiltered);
kunit_handle_shutdown();
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 7d2b8dc668b1..d9fce637eb56 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -9,8 +9,6 @@ #include <kunit/test.h>
static void kfree_at_end(struct kunit *test, const void *to_free); -static void free_subsuite_at_end(struct kunit *test, - struct kunit_suite *const *to_free); static struct kunit_suite *alloc_fake_suite(struct kunit *test, const char *suite_name, struct kunit_case *test_cases); @@ -41,124 +39,77 @@ static void parse_filter_test(struct kunit *test) kfree(filter.test_glob); }
-static void filter_subsuite_test(struct kunit *test) +static void filter_suites_test(struct kunit *test) { struct kunit_suite *subsuite[3] = {NULL, NULL, NULL}; - struct kunit_suite * const *filtered; - struct kunit_test_filter filter = { - .suite_glob = "suite2", - .test_glob = NULL, - }; + struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]}; + struct suite_set got;
subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases); subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);
/* Want: suite1, suite2, NULL -> suite2, NULL */ - filtered = kunit_filter_subsuite(subsuite, &filter); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered); - free_subsuite_at_end(test, filtered); + got = kunit_filter_suites(&suite_set, "suite2"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); + kfree_at_end(test, got.start);
/* Validate we just have suite2 */ - 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]); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]); + KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->name, "suite2"); + // DO NOT SUBMIT: null-terminated for now. + KUNIT_ASSERT_EQ(test, got.end - got.start, 1); + KUNIT_EXPECT_FALSE(test, *got.end); }
-static void filter_subsuite_test_glob_test(struct kunit *test) +static void filter_suites_test_glob_test(struct kunit *test) { struct kunit_suite *subsuite[3] = {NULL, NULL, NULL}; - struct kunit_suite * const *filtered; - struct kunit_test_filter filter = { - .suite_glob = "suite2", - .test_glob = "test2", - }; + struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]}; + struct suite_set got;
subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases); subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);
/* Want: suite1, suite2, NULL -> suite2 (just test1), NULL */ - filtered = kunit_filter_subsuite(subsuite, &filter); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered); - free_subsuite_at_end(test, filtered); + got = kunit_filter_suites(&suite_set, "suite2.test2"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); + kfree_at_end(test, got.start);
/* Validate we just have suite2 */ - 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]); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]); + KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->name, "suite2"); + // DO NOT SUBMIT: null-terminated for now. + KUNIT_ASSERT_EQ(test, got.end - got.start, 1); + KUNIT_EXPECT_FALSE(test, *got.end);
/* Now validate we just have test2 */ - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]->test_cases); - KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->test_cases[0].name, "test2"); - KUNIT_EXPECT_FALSE(test, filtered[0]->test_cases[1].name); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]->test_cases); + KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->test_cases[0].name, "test2"); + KUNIT_EXPECT_FALSE(test, got.start[0]->test_cases[1].name); }
-static void filter_subsuite_to_empty_test(struct kunit *test) +static void filter_suites_to_empty_test(struct kunit *test) { struct kunit_suite *subsuite[3] = {NULL, NULL, NULL}; - struct kunit_suite * const *filtered; - struct kunit_test_filter filter = { - .suite_glob = "not_found", - .test_glob = NULL, - }; + struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]}; + struct suite_set got;
subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases); subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);
- filtered = kunit_filter_subsuite(subsuite, &filter); - free_subsuite_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; + got = kunit_filter_suites(&suite_set, "not_found"); + kfree_at_end(test, got.start); /* just in case */
- kfree_at_end(test, suite_set->start); - for (suites = suite_set->start; suites < suite_set->end; suites++) - free_subsuite_at_end(test, *suites); + KUNIT_EXPECT_PTR_EQ_MSG(test, got.start, got.end, + "should be empty to indicate no match"); }
-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", dummy_test_cases); - subsuites[1][0] = alloc_fake_suite(test, "suite1", dummy_test_cases); - - /* 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_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0][0]); - KUNIT_EXPECT_STREQ(test, (const char *)filtered.start[0][0]->name, "suite0"); -}
static struct kunit_case executor_test_cases[] = { KUNIT_CASE(parse_filter_test), - KUNIT_CASE(filter_subsuite_test), - KUNIT_CASE(filter_subsuite_test_glob_test), - KUNIT_CASE(filter_subsuite_to_empty_test), KUNIT_CASE(filter_suites_test), + KUNIT_CASE(filter_suites_test_glob_test), + KUNIT_CASE(filter_suites_to_empty_test), {} };
@@ -188,20 +139,6 @@ static void kfree_at_end(struct kunit *test, const void *to_free) (void *)to_free); }
-static void free_subsuite_res_free(struct kunit_resource *res) -{ - kunit_free_subsuite(res->data); -} - -static void free_subsuite_at_end(struct kunit *test, - struct kunit_suite *const *to_free) -{ - if (IS_ERR_OR_NULL(to_free)) - return; - kunit_alloc_resource(test, NULL, free_subsuite_res_free, - GFP_KERNEL, (void *)to_free); -} - static struct kunit_suite *alloc_fake_suite(struct kunit *test, const char *suite_name, struct kunit_case *test_cases)
base-commit: e7198adb84dcad671ad4f0e90aaa7e9fabf258dc