lkml.org 
[lkml]   [2021]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
Subject[RFC PATCH] kunit: flatten kunit_suite*** to kunit_suite** in executor
From
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/101d12fc9250b7a445ff50a9e7a25cd74d0e16eb.camel@codeconstruct.com.au/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=3b29021ddd10cfb6b2565c623595bd3b02036f33

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
--
2.33.0.882.g93a45727a2-goog

\
 
 \ /
  Last update: 2021-10-13 21:14    [W:0.061 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site