diff --git a/Documentation/dev-tools/kunit/architecture.rst b/Documentation/dev-tools/kunit/architecture.rst index e95ab05342bb..f335f883f8f6 100644 --- a/Documentation/dev-tools/kunit/architecture.rst +++ b/Documentation/dev-tools/kunit/architecture.rst @@ -119,9 +119,9 @@ All expectations/assertions are formatted as: terminated immediately. - Assertions call the function: - ``void __noreturn kunit_abort(struct kunit *)``. + ``void __noreturn __kunit_abort(struct kunit *)``. - - ``kunit_abort`` calls the function: + - ``__kunit_abort`` calls the function: ``void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch)``. - ``kunit_try_catch_throw`` calls the function: diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst index c736613c9b19..a98235326bab 100644 --- a/Documentation/dev-tools/kunit/start.rst +++ b/Documentation/dev-tools/kunit/start.rst @@ -250,15 +250,20 @@ Now we are ready to write the test cases. }; kunit_test_suite(misc_example_test_suite); + MODULE_LICENSE("GPL"); + 2. Add the following lines to ``drivers/misc/Kconfig``: .. code-block:: kconfig config MISC_EXAMPLE_TEST tristate "Test for my example" if !KUNIT_ALL_TESTS - depends on MISC_EXAMPLE && KUNIT=y + depends on MISC_EXAMPLE && KUNIT default KUNIT_ALL_TESTS +Note: If your test does not support being built as a loadable module (which is +discouraged), replace tristate by bool, and depend on KUNIT=y instead of KUNIT. + 3. Add the following lines to ``drivers/misc/Makefile``: .. code-block:: make diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 9faf2b4153fc..c27e1646ecd9 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -121,6 +121,12 @@ there's an allocation error. ``return`` so they only work from the test function. In KUnit, we stop the current kthread on failure, so you can call them from anywhere. +.. note:: + Warning: There is an exception to the above rule. You shouldn't use assertions + in the suite's exit() function, or in the free function for a resource. These + run when a test is shutting down, and an assertion here prevents further + cleanup code from running, potentially leading to a memory leak. + Customizing error messages -------------------------- @@ -160,7 +166,12 @@ many similar tests. In order to reduce duplication in these closely related tests, most unit testing frameworks (including KUnit) provide the concept of a *test suite*. A test suite is a collection of test cases for a unit of code with optional setup and teardown functions that run before/after the whole -suite and/or every test case. For example: +suite and/or every test case. + +.. note:: + A test case will only run if it is associated with a test suite. + +For example: .. code-block:: c @@ -190,7 +201,10 @@ after everything else. ``kunit_test_suite(example_test_suite)`` registers the test suite with the KUnit test framework. .. note:: - A test case will only run if it is associated with a test suite. + The ``exit`` and ``suite_exit`` functions will run even if ``init`` or + ``suite_init`` fail. Make sure that they can handle any inconsistent + state which may result from ``init`` or ``suite_init`` encountering errors + or exiting early. ``kunit_test_suite(...)`` is a macro which tells the linker to put the specified test suite in a special linker section so that it can be run by KUnit @@ -601,6 +615,57 @@ For example: KUNIT_ASSERT_STREQ(test, buffer, ""); } +Registering Cleanup Actions +--------------------------- + +If you need to perform some cleanup beyond simple use of ``kunit_kzalloc``, +you can register a custom "deferred action", which is a cleanup function +run when the test exits (whether cleanly, or via a failed assertion). + +Actions are simple functions with no return value, and a single ``void*`` +context argument, and fulfill the same role as "cleanup" functions in Python +and Go tests, "defer" statements in languages which support them, and +(in some cases) destructors in RAII languages. + +These are very useful for unregistering things from global lists, closing +files or other resources, or freeing resources. + +For example: + +.. code-block:: C + + static void cleanup_device(void *ctx) + { + struct device *dev = (struct device *)ctx; + + device_unregister(dev); + } + + void example_device_test(struct kunit *test) + { + struct my_device dev; + + device_register(&dev); + + kunit_add_action(test, &cleanup_device, &dev); + } + +Note that, for functions like device_unregister which only accept a single +pointer-sized argument, it's possible to directly cast that function to +a ``kunit_action_t`` rather than writing a wrapper function, for example: + +.. code-block:: C + + kunit_add_action(test, (kunit_action_t *)&device_unregister, &dev); + +``kunit_add_action`` can fail if, for example, the system is out of memory. +You can use ``kunit_add_action_or_reset`` instead which runs the action +immediately if it cannot be deferred. + +If you need more control over when the cleanup function is called, you +can trigger it early using ``kunit_release_action``, or cancel it entirely +with ``kunit_remove_action``. + Testing Static Functions ------------------------ diff --git a/MAINTAINERS b/MAINTAINERS index 7e0b87d5aa2e..218489d0039e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11311,6 +11311,8 @@ L: linux-kselftest@vger.kernel.org L: kunit-dev@googlegroups.com S: Maintained W: https://google.github.io/kunit-docs/third_party/kernel/docs/ +T: git git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git kunit +T: git git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git kunit-fixes F: Documentation/dev-tools/kunit/ F: include/kunit/ F: lib/kunit/ diff --git a/include/kunit/resource.h b/include/kunit/resource.h index c0d88b318e90..c7383e90f5c9 100644 --- a/include/kunit/resource.h +++ b/include/kunit/resource.h @@ -387,4 +387,96 @@ static inline int kunit_destroy_named_resource(struct kunit *test, */ void kunit_remove_resource(struct kunit *test, struct kunit_resource *res); +/* A 'deferred action' function to be used with kunit_add_action. */ +typedef void (kunit_action_t)(void *); + +/** + * kunit_add_action() - Call a function when the test ends. + * @test: Test case to associate the action with. + * @action: The function to run on test exit + * @ctx: Data passed into @func + * + * Defer the execution of a function until the test exits, either normally or + * due to a failure. @ctx is passed as additional context. All functions + * registered with kunit_add_action() will execute in the opposite order to that + * they were registered in. + * + * This is useful for cleaning up allocated memory and resources, as these + * functions are called even if the test aborts early due to, e.g., a failed + * assertion. + * + * See also: devm_add_action() for the devres equivalent. + * + * Returns: + * 0 on success, an error if the action could not be deferred. + */ +int kunit_add_action(struct kunit *test, kunit_action_t *action, void *ctx); + +/** + * kunit_add_action_or_reset() - Call a function when the test ends. + * @test: Test case to associate the action with. + * @action: The function to run on test exit + * @ctx: Data passed into @func + * + * Defer the execution of a function until the test exits, either normally or + * due to a failure. @ctx is passed as additional context. All functions + * registered with kunit_add_action() will execute in the opposite order to that + * they were registered in. + * + * This is useful for cleaning up allocated memory and resources, as these + * functions are called even if the test aborts early due to, e.g., a failed + * assertion. + * + * If the action cannot be created (e.g., due to the system being out of memory), + * then action(ctx) will be called immediately, and an error will be returned. + * + * See also: devm_add_action_or_reset() for the devres equivalent. + * + * Returns: + * 0 on success, an error if the action could not be deferred. + */ +int kunit_add_action_or_reset(struct kunit *test, kunit_action_t *action, + void *ctx); + +/** + * kunit_remove_action() - Cancel a matching deferred action. + * @test: Test case the action is associated with. + * @action: The deferred function to cancel. + * @ctx: The context passed to the deferred function to trigger. + * + * Prevent an action deferred via kunit_add_action() from executing when the + * test terminates. + * + * If the function/context pair was deferred multiple times, only the most + * recent one will be cancelled. + * + * See also: devm_remove_action() for the devres equivalent. + */ +void kunit_remove_action(struct kunit *test, + kunit_action_t *action, + void *ctx); + +/** + * kunit_release_action() - Run a matching action call immediately. + * @test: Test case the action is associated with. + * @action: The deferred function to trigger. + * @ctx: The context passed to the deferred function to trigger. + * + * Execute a function deferred via kunit_add_action()) immediately, rather than + * when the test ends. + * + * If the function/context pair was deferred multiple times, it will only be + * executed once here. The most recent deferral will no longer execute when + * the test ends. + * + * kunit_release_action(test, func, ctx); + * is equivalent to + * func(ctx); + * kunit_remove_action(test, func, ctx); + * + * See also: devm_release_action() for the devres equivalent. + */ +void kunit_release_action(struct kunit *test, + kunit_action_t *action, + void *ctx); #endif /* _KUNIT_RESOURCE_H */ diff --git a/include/kunit/test.h b/include/kunit/test.h index 57b309c6ca27..23120d50499e 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -47,6 +47,7 @@ struct kunit; * sub-subtest. See the "Subtests" section in * https://node-tap.org/tap-protocol/ */ +#define KUNIT_INDENT_LEN 4 #define KUNIT_SUBTEST_INDENT " " #define KUNIT_SUBSUBTEST_INDENT " " @@ -168,6 +169,9 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) * test case, similar to the notion of a *test fixture* or a *test class* * in other unit testing frameworks like JUnit or Googletest. * + * Note that @exit and @suite_exit will run even if @init or @suite_init + * fail: make sure they can handle any inconsistent state which may result. + * * Every &struct kunit_case must be associated with a kunit_suite for KUnit * to run it. */ @@ -321,8 +325,11 @@ enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite); * @gfp: flags passed to underlying kmalloc(). * * Just like `kmalloc_array(...)`, except the allocation is managed by the test case - * and is automatically cleaned up after the test case concludes. See &struct - * kunit_resource for more information. + * and is automatically cleaned up after the test case concludes. See kunit_add_action() + * for more information. + * + * Note that some internal context data is also allocated with GFP_KERNEL, + * regardless of the gfp passed in. */ void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp); @@ -333,6 +340,9 @@ void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp); * @gfp: flags passed to underlying kmalloc(). * * See kmalloc() and kunit_kmalloc_array() for more information. + * + * Note that some internal context data is also allocated with GFP_KERNEL, + * regardless of the gfp passed in. */ static inline void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) { @@ -472,7 +482,9 @@ void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...); */ #define KUNIT_SUCCEED(test) do {} while (0) -void kunit_do_failed_assertion(struct kunit *test, +void __noreturn __kunit_abort(struct kunit *test); + +void __kunit_do_failed_assertion(struct kunit *test, const struct kunit_loc *loc, enum kunit_assert_type type, const struct kunit_assert *assert, @@ -482,13 +494,15 @@ void kunit_do_failed_assertion(struct kunit *test, #define _KUNIT_FAILED(test, assert_type, assert_class, assert_format, INITIALIZER, fmt, ...) do { \ static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \ const struct assert_class __assertion = INITIALIZER; \ - kunit_do_failed_assertion(test, \ - &__loc, \ - assert_type, \ - &__assertion.assert, \ - assert_format, \ - fmt, \ - ##__VA_ARGS__); \ + __kunit_do_failed_assertion(test, \ + &__loc, \ + assert_type, \ + &__assertion.assert, \ + assert_format, \ + fmt, \ + ##__VA_ARGS__); \ + if (assert_type == KUNIT_ASSERTION) \ + __kunit_abort(test); \ } while (0) diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 0cea31c27b23..ce6749af374d 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -125,11 +125,6 @@ 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. */ @@ -138,8 +133,10 @@ 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_resource(test, NULL, kfree_res_free, GFP_KERNEL, - (void *)to_free); + + kunit_add_action(test, + (kunit_action_t *)kfree, + (void *)to_free); } static struct kunit_suite *alloc_fake_suite(struct kunit *test, diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index cd8b7e51d02b..b69b689ea850 100644 --- a/lib/kunit/kunit-example-test.c +++ b/lib/kunit/kunit-example-test.c @@ -41,6 +41,16 @@ static int example_test_init(struct kunit *test) return 0; } +/* + * This is run once after each test case, see the comment on + * example_test_suite for more information. + */ +static void example_test_exit(struct kunit *test) +{ + kunit_info(test, "cleaning up\n"); +} + + /* * This is run once before all test cases in the suite. * See the comment on example_test_suite for more information. @@ -52,6 +62,16 @@ static int example_test_init_suite(struct kunit_suite *suite) return 0; } +/* + * This is run once after all test cases in the suite. + * See the comment on example_test_suite for more information. + */ +static void example_test_exit_suite(struct kunit_suite *suite) +{ + kunit_info(suite, "exiting suite\n"); +} + + /* * This test should always be skipped. */ @@ -167,6 +187,39 @@ static void example_static_stub_test(struct kunit *test) KUNIT_EXPECT_EQ(test, add_one(1), 2); } +static const struct example_param { + int value; +} example_params_array[] = { + { .value = 2, }, + { .value = 1, }, + { .value = 0, }, +}; + +static void example_param_get_desc(const struct example_param *p, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "example value %d", p->value); +} + +KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc); + +/* + * This test shows the use of params. + */ +static void example_params_test(struct kunit *test) +{ + const struct example_param *param = test->param_value; + + /* By design, param pointer will not be NULL */ + KUNIT_ASSERT_NOT_NULL(test, param); + + /* Test can be skipped on unsupported param values */ + if (!param->value) + kunit_skip(test, "unsupported param value"); + + /* You can use param values for parameterized testing */ + KUNIT_EXPECT_EQ(test, param->value % param->value, 0); +} + /* * Here we make a list of all the test cases we want to add to the test suite * below. @@ -183,6 +236,7 @@ static struct kunit_case example_test_cases[] = { KUNIT_CASE(example_mark_skipped_test), KUNIT_CASE(example_all_expect_macros_test), KUNIT_CASE(example_static_stub_test), + KUNIT_CASE_PARAM(example_params_test, example_gen_params), {} }; @@ -211,7 +265,9 @@ static struct kunit_case example_test_cases[] = { static struct kunit_suite example_test_suite = { .name = "example", .init = example_test_init, + .exit = example_test_exit, .suite_init = example_test_init_suite, + .suite_exit = example_test_exit_suite, .test_cases = example_test_cases, }; diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 42e44caa1bdd..83d8e90ca7a2 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -112,7 +112,7 @@ struct kunit_test_resource_context { struct kunit test; bool is_resource_initialized; int allocate_order[2]; - int free_order[2]; + int free_order[4]; }; static int fake_resource_init(struct kunit_resource *res, void *context) @@ -403,6 +403,88 @@ static void kunit_resource_test_named(struct kunit *test) KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); } +static void increment_int(void *ctx) +{ + int *i = (int *)ctx; + (*i)++; +} + +static void kunit_resource_test_action(struct kunit *test) +{ + int num_actions = 0; + + kunit_add_action(test, increment_int, &num_actions); + KUNIT_EXPECT_EQ(test, num_actions, 0); + kunit_cleanup(test); + KUNIT_EXPECT_EQ(test, num_actions, 1); + + /* Once we've cleaned up, the action queue is empty. */ + kunit_cleanup(test); + KUNIT_EXPECT_EQ(test, num_actions, 1); + + /* Check the same function can be deferred multiple times. */ + kunit_add_action(test, increment_int, &num_actions); + kunit_add_action(test, increment_int, &num_actions); + kunit_cleanup(test); + KUNIT_EXPECT_EQ(test, num_actions, 3); +} +static void kunit_resource_test_remove_action(struct kunit *test) +{ + int num_actions = 0; + + kunit_add_action(test, increment_int, &num_actions); + KUNIT_EXPECT_EQ(test, num_actions, 0); + + kunit_remove_action(test, increment_int, &num_actions); + kunit_cleanup(test); + KUNIT_EXPECT_EQ(test, num_actions, 0); +} +static void kunit_resource_test_release_action(struct kunit *test) +{ + int num_actions = 0; + + kunit_add_action(test, increment_int, &num_actions); + KUNIT_EXPECT_EQ(test, num_actions, 0); + /* Runs immediately on trigger. */ + kunit_release_action(test, increment_int, &num_actions); + KUNIT_EXPECT_EQ(test, num_actions, 1); + + /* Doesn't run again on test exit. */ + kunit_cleanup(test); + KUNIT_EXPECT_EQ(test, num_actions, 1); +} +static void action_order_1(void *ctx) +{ + struct kunit_test_resource_context *res_ctx = (struct kunit_test_resource_context *)ctx; + + KUNIT_RESOURCE_TEST_MARK_ORDER(res_ctx, free_order, 1); + kunit_log(KERN_INFO, current->kunit_test, "action_order_1"); +} +static void action_order_2(void *ctx) +{ + struct kunit_test_resource_context *res_ctx = (struct kunit_test_resource_context *)ctx; + + KUNIT_RESOURCE_TEST_MARK_ORDER(res_ctx, free_order, 2); + kunit_log(KERN_INFO, current->kunit_test, "action_order_2"); +} +static void kunit_resource_test_action_ordering(struct kunit *test) +{ + struct kunit_test_resource_context *ctx = test->priv; + + kunit_add_action(test, action_order_1, ctx); + kunit_add_action(test, action_order_2, ctx); + kunit_add_action(test, action_order_1, ctx); + kunit_add_action(test, action_order_2, ctx); + kunit_remove_action(test, action_order_1, ctx); + kunit_release_action(test, action_order_2, ctx); + kunit_cleanup(test); + + /* [2 is triggered] [2], [(1 is cancelled)] [1] */ + KUNIT_EXPECT_EQ(test, ctx->free_order[0], 2); + KUNIT_EXPECT_EQ(test, ctx->free_order[1], 2); + KUNIT_EXPECT_EQ(test, ctx->free_order[2], 1); +} + static int kunit_resource_test_init(struct kunit *test) { struct kunit_test_resource_context *ctx = @@ -434,6 +516,10 @@ static struct kunit_case kunit_resource_test_cases[] = { KUNIT_CASE(kunit_resource_test_proper_free_ordering), KUNIT_CASE(kunit_resource_test_static), KUNIT_CASE(kunit_resource_test_named), + KUNIT_CASE(kunit_resource_test_action), + KUNIT_CASE(kunit_resource_test_remove_action), + KUNIT_CASE(kunit_resource_test_release_action), + KUNIT_CASE(kunit_resource_test_action_ordering), {} }; diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c index c414df922f34..f0209252b179 100644 --- a/lib/kunit/resource.c +++ b/lib/kunit/resource.c @@ -77,3 +77,102 @@ int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, return 0; } EXPORT_SYMBOL_GPL(kunit_destroy_resource); + +struct kunit_action_ctx { + struct kunit_resource res; + kunit_action_t *func; + void *ctx; +}; + +static void __kunit_action_free(struct kunit_resource *res) +{ + struct kunit_action_ctx *action_ctx = container_of(res, struct kunit_action_ctx, res); + + action_ctx->func(action_ctx->ctx); +} + + +int kunit_add_action(struct kunit *test, void (*action)(void *), void *ctx) +{ + struct kunit_action_ctx *action_ctx; + + KUNIT_ASSERT_NOT_NULL_MSG(test, action, "Tried to action a NULL function!"); + + action_ctx = kzalloc(sizeof(*action_ctx), GFP_KERNEL); + if (!action_ctx) + return -ENOMEM; + + action_ctx->func = action; + action_ctx->ctx = ctx; + + action_ctx->res.should_kfree = true; + /* As init is NULL, this cannot fail. */ + __kunit_add_resource(test, NULL, __kunit_action_free, &action_ctx->res, action_ctx); + + return 0; +} +EXPORT_SYMBOL_GPL(kunit_add_action); + +int kunit_add_action_or_reset(struct kunit *test, void (*action)(void *), + void *ctx) +{ + int res = kunit_add_action(test, action, ctx); + + if (res) + action(ctx); + return res; +} +EXPORT_SYMBOL_GPL(kunit_add_action_or_reset); + +static bool __kunit_action_match(struct kunit *test, + struct kunit_resource *res, void *match_data) +{ + struct kunit_action_ctx *match_ctx = (struct kunit_action_ctx *)match_data; + struct kunit_action_ctx *res_ctx = container_of(res, struct kunit_action_ctx, res); + + /* Make sure this is a free function. */ + if (res->free != __kunit_action_free) + return false; + + /* Both the function and context data should match. */ + return (match_ctx->func == res_ctx->func) && (match_ctx->ctx == res_ctx->ctx); +} + +void kunit_remove_action(struct kunit *test, + kunit_action_t *action, + void *ctx) +{ + struct kunit_action_ctx match_ctx; + struct kunit_resource *res; + + match_ctx.func = action; + match_ctx.ctx = ctx; + + res = kunit_find_resource(test, __kunit_action_match, &match_ctx); + if (res) { + /* Remove the free function so we don't run the action. */ + res->free = NULL; + kunit_remove_resource(test, res); + kunit_put_resource(res); + } +} +EXPORT_SYMBOL_GPL(kunit_remove_action); + +void kunit_release_action(struct kunit *test, + kunit_action_t *action, + void *ctx) +{ + struct kunit_action_ctx match_ctx; + struct kunit_resource *res; + + match_ctx.func = action; + match_ctx.ctx = ctx; + + res = kunit_find_resource(test, __kunit_action_match, &match_ctx); + if (res) { + kunit_remove_resource(test, res); + /* We have to put() this here, else free won't be called. */ + kunit_put_resource(res); + } +} +EXPORT_SYMBOL_GPL(kunit_release_action); diff --git a/lib/kunit/test.c b/lib/kunit/test.c index e2910b261112..84e4666555c9 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -185,16 +185,28 @@ static void kunit_print_suite_start(struct kunit_suite *suite) kunit_suite_num_test_cases(suite)); } -static void kunit_print_ok_not_ok(void *test_or_suite, - bool is_test, +/* Currently supported test levels */ +enum { + KUNIT_LEVEL_SUITE = 0, + KUNIT_LEVEL_CASE, + KUNIT_LEVEL_CASE_PARAM, +}; + +static void kunit_print_ok_not_ok(struct kunit *test, + unsigned int test_level, enum kunit_status status, size_t test_number, const char *description, const char *directive) { - struct kunit_suite *suite = is_test ? NULL : test_or_suite; - struct kunit *test = is_test ? test_or_suite : NULL; const char *directive_header = (status == KUNIT_SKIPPED) ? " # SKIP " : ""; + const char *directive_body = (status == KUNIT_SKIPPED) ? directive : ""; + + /* + * When test is NULL assume that results are from the suite + * and today suite results are expected at level 0 only. + */ + WARN(!test && test_level, "suite test level can't be %u!\n", test_level); /* * We do not log the test suite results as doing so would @@ -203,17 +215,18 @@ static void kunit_print_ok_not_ok(void *test_or_suite, * separately seq_printf() the suite results for the debugfs * representation. */ - if (suite) + if (!test) pr_info("%s %zd %s%s%s\n", kunit_status_to_ok_not_ok(status), test_number, description, directive_header, - (status == KUNIT_SKIPPED) ? directive : ""); + directive_body); else kunit_log(KERN_INFO, test, - KUNIT_SUBTEST_INDENT "%s %zd %s%s%s", + "%*s%s %zd %s%s%s", + KUNIT_INDENT_LEN * test_level, "", kunit_status_to_ok_not_ok(status), test_number, description, directive_header, - (status == KUNIT_SKIPPED) ? directive : ""); + directive_body); } enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite) @@ -239,7 +252,7 @@ static size_t kunit_suite_counter = 1; static void kunit_print_suite_end(struct kunit_suite *suite) { - kunit_print_ok_not_ok((void *)suite, false, + kunit_print_ok_not_ok(NULL, KUNIT_LEVEL_SUITE, kunit_suite_has_succeeded(suite), kunit_suite_counter++, suite->name, @@ -310,7 +323,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, string_stream_destroy(stream); } -static void __noreturn kunit_abort(struct kunit *test) +void __noreturn __kunit_abort(struct kunit *test) { kunit_try_catch_throw(&test->try_catch); /* Does not return. */ @@ -322,8 +335,9 @@ static void __noreturn kunit_abort(struct kunit *test) */ WARN_ONCE(true, "Throw could not abort from test!\n"); } +EXPORT_SYMBOL_GPL(__kunit_abort); -void kunit_do_failed_assertion(struct kunit *test, +void __kunit_do_failed_assertion(struct kunit *test, const struct kunit_loc *loc, enum kunit_assert_type type, const struct kunit_assert *assert, @@ -340,11 +354,8 @@ void kunit_do_failed_assertion(struct kunit *test, kunit_fail(test, loc, type, assert, assert_format, &message); va_end(args); - - if (type == KUNIT_ASSERTION) - kunit_abort(test); } -EXPORT_SYMBOL_GPL(kunit_do_failed_assertion); +EXPORT_SYMBOL_GPL(__kunit_do_failed_assertion); void kunit_init_test(struct kunit *test, const char *name, char *log) { @@ -419,15 +430,54 @@ static void kunit_try_run_case(void *data) * thread will resume control and handle any necessary clean up. */ kunit_run_case_internal(test, suite, test_case); - /* This line may never be reached. */ +} + +static void kunit_try_run_case_cleanup(void *data) +{ + struct kunit_try_catch_context *ctx = data; + struct kunit *test = ctx->test; + struct kunit_suite *suite = ctx->suite; + + current->kunit_test = test; + kunit_run_case_cleanup(test, suite); } +static void kunit_catch_run_case_cleanup(void *data) +{ + struct kunit_try_catch_context *ctx = data; + struct kunit *test = ctx->test; + int try_exit_code = kunit_try_catch_get_result(&test->try_catch); + + /* It is always a failure if cleanup aborts. */ + kunit_set_failure(test); + + if (try_exit_code) { + /* + * Test case could not finish, we have no idea what state it is + * in, so don't do clean up. + */ + if (try_exit_code == -ETIMEDOUT) { + kunit_err(test, "test case cleanup timed out\n"); + /* + * Unknown internal error occurred preventing test case from + * running, so there is nothing to clean up. + */ + } else { + kunit_err(test, "internal error occurred during test case cleanup: %d\n", + try_exit_code); + } + return; + } + + kunit_err(test, "test aborted during cleanup. continuing without cleaning up\n"); +} + + static void kunit_catch_run_case(void *data) { struct kunit_try_catch_context *ctx = data; struct kunit *test = ctx->test; - struct kunit_suite *suite = ctx->suite; int try_exit_code = kunit_try_catch_get_result(&test->try_catch); if (try_exit_code) { @@ -448,12 +498,6 @@ static void kunit_catch_run_case(void *data) } return; } - - /* - * Test case was run, but aborted. It is the test case's business as to - * whether it failed or not, we just need to clean up. - */ - kunit_run_case_cleanup(test, suite); } /* @@ -478,6 +522,13 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite, context.test_case = test_case; kunit_try_catch_run(try_catch, &context); + /* Now run the cleanup */ + kunit_try_catch_init(try_catch, + test, + kunit_try_run_case_cleanup, + kunit_catch_run_case_cleanup); + kunit_try_catch_run(try_catch, &context); + /* Propagate the parameter result to the test case. */ if (test->status == KUNIT_FAILURE) test_case->status = KUNIT_FAILURE; @@ -585,11 +636,11 @@ int kunit_run_tests(struct kunit_suite *suite) "param-%d", test.param_index); } - kunit_log(KERN_INFO, &test, - KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT - "%s %d %s", - kunit_status_to_ok_not_ok(test.status), - test.param_index + 1, param_desc); + kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE_PARAM, + test.status, + test.param_index + 1, + param_desc, + test.status_comment); /* Get next param. */ param_desc[0] = '\0'; @@ -603,7 +654,7 @@ int kunit_run_tests(struct kunit_suite *suite) kunit_print_test_stats(&test, param_stats); - kunit_print_ok_not_ok(&test, true, test_case->status, + kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status, kunit_test_case_num(suite, test_case), test_case->name, test.status_comment); @@ -712,58 +763,28 @@ static struct notifier_block kunit_mod_nb = { }; #endif -struct kunit_kmalloc_array_params { - size_t n; - size_t size; - gfp_t gfp; -}; - -static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context) +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) { - struct kunit_kmalloc_array_params *params = context; + void *data; - res->data = kmalloc_array(params->n, params->size, params->gfp); - if (!res->data) - return -ENOMEM; + data = kmalloc_array(n, size, gfp); - return 0; -} + if (!data) + return NULL; -static void kunit_kmalloc_array_free(struct kunit_resource *res) -{ - kfree(res->data); -} - -void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) -{ - struct kunit_kmalloc_array_params params = { - .size = size, - .n = n, - .gfp = gfp - }; + if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree, data) != 0) + return NULL; - return kunit_alloc_resource(test, - kunit_kmalloc_array_init, - kunit_kmalloc_array_free, - gfp, - ¶ms); + return data; } EXPORT_SYMBOL_GPL(kunit_kmalloc_array); -static inline bool kunit_kfree_match(struct kunit *test, - struct kunit_resource *res, void *match_data) -{ - /* Only match resources allocated with kunit_kmalloc() and friends. */ - return res->free == kunit_kmalloc_array_free && res->data == match_data; -} - void kunit_kfree(struct kunit *test, const void *ptr) { if (!ptr) return; - if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) - KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); + kunit_release_action(test, (kunit_action_t *)kfree, (void *)ptr); } EXPORT_SYMBOL_GPL(kunit_kfree); diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index f01f94106129..7f648802caf6 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -92,7 +92,7 @@ class LinuxSourceTreeOperations: if stderr: # likely only due to build warnings print(stderr.decode()) - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: raise RuntimeError('not implemented!') @@ -113,7 +113,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: kernel_path = os.path.join(build_dir, self._kernel_path) qemu_command = ['qemu-system-' + self._qemu_arch, '-nodefaults', @@ -142,7 +142,7 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: """Runs the Linux UML binary. Must be named 'linux'.""" linux_bin = os.path.join(build_dir, 'linux') params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt']) diff --git a/tools/testing/kunit/mypy.ini b/tools/testing/kunit/mypy.ini new file mode 100644 index 000000000000..ddd288309efa --- /dev/null +++ b/tools/testing/kunit/mypy.ini @@ -0,0 +1,6 @@ +[mypy] +strict = True + +# E.g. we can't write subprocess.Popen[str] until Python 3.9+. +# But kunit.py tries to support Python 3.7+, so let's disable it. +disable_error_code = type-arg diff --git a/tools/testing/kunit/run_checks.py b/tools/testing/kunit/run_checks.py index 8208c3b3135e..c6d494ea3373 100755 --- a/tools/testing/kunit/run_checks.py +++ b/tools/testing/kunit/run_checks.py @@ -23,7 +23,7 @@ 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': ['mypy', '--strict', '--exclude', '_test.py$', '--exclude', 'qemu_configs/', '.'], + 'mypy': ['mypy', '--config-file', 'mypy.ini', '--exclude', '_test.py$', '--exclude', 'qemu_configs/', '.'], } # The user might not have mypy or pytype installed, skip them if so.