KUnit tests run in a kthread, with the current->kunit_test pointer set
to the test's context. This allows the kunit_get_current_test() and
kunit_fail_current_test() macros to work. Normally, this pointer is
still valid during test shutdown (i.e., the suite->exit function, and
any resource cleanup). However, if the test has exited early (e.g., due
to a failed assertion), the cleanup is done in the parent KUnit thread,
which does not have an active context.
Instead, in the event test terminates early, run the test exit and
cleanup from a new 'cleanup' kthread, which sets current->kunit_test,
and better isolates the rest of KUnit from issues which arise in test
cleanup.
If a test cleanup function itself aborts (e.g., due to an assertion
failing), there will be no further attempts to clean up: an error will
be logged and the test failed. For example:
# example_simple_test: test aborted during cleanup. continuing without cleaning up
This should also make it easier to get access to the KUnit context,
particularly from within resource cleanup functions, which may, for
example, need access to data in test->priv.
Signed-off-by: David Gow <[email protected]>
---
This is an updated version of / replacement of "kunit: Set the current
KUnit context when cleaning up", which instead creates a new kthread
for cleanup tasks if the original test kthread is aborted. This protects
us from failed assertions during cleanup, if the test exited early.
Changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Always run cleanup in its own kthread
- Therefore, never attempt to re-run it if it exits
- Thanks, Benjamin.
Changes since v1:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Move cleanup execution to another kthread
- (Thanks, Benjamin, for pointing out the assertion issues)
---
lib/kunit/test.c | 55 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 48 insertions(+), 7 deletions(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index e2910b261112..2025e51941e6 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -419,10 +419,50 @@ 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;
@@ -448,12 +488,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 +512,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;
--
2.40.0.634.g4ca3ef3211-goog
As assertions abort the test cleanup process, they should be avoided
from within a suite's exit function, or from within resource 'free'
functions. Unlike with initialisation or main test execution, no further
cleanup will be performed after a failed assertion, potentially causing
a leak of resources.
Signed-off-by: David Gow <[email protected]>
---
No changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
This patch was introduced in v2.
---
Documentation/dev-tools/kunit/usage.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 9faf2b4153fc..9f720f1317d3 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
--------------------------
--
2.40.0.634.g4ca3ef3211-goog
KUnit's exit functions will run even if the corresponding init function
fails. It's easy, when writing an exit function, to assume the init
function succeeded, and (for example) access uninitialised memory or
dereference NULL pointers.
Note that this case exists and should be handled in the documentation.
Suggested-by: Benjamin Berg <[email protected]>
Link: https://lore.kernel.org/linux-kselftest/[email protected]/
Signed-off-by: David Gow <[email protected]>
---
No changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
This patch was introduced in v2.
---
Documentation/dev-tools/kunit/usage.rst | 12 ++++++++++--
include/kunit/test.h | 3 +++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 9f720f1317d3..f6d6c9a9ff54 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -166,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
@@ -196,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`` encoutering 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
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 57b309c6ca27..3028a1a3fcad 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -168,6 +168,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.
*/
--
2.40.0.634.g4ca3ef3211-goog
Add an example .exit and .suite_exit function to the KUnit example
suite. Given exit functions are a bit more subtle than init functions
(due to running in a different kthread, and running even after tests or
test init functions fail), providing an easy place to experiment with
them is useful.
Signed-off-by: David Gow <[email protected]>
---
This patch was introduced in v3.
---
lib/kunit/kunit-example-test.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index cd8b7e51d02b..24315c882b31 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.
*/
@@ -211,7 +231,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,
};
--
2.40.0.634.g4ca3ef3211-goog
Hi David,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.3-rc7 next-20230420]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/Documentation-kunit-Note-that-assertions-should-not-be-used-in-cleanup/20230421-120437
patch link: https://lore.kernel.org/r/20230421040218.2156548-1-davidgow%40google.com
patch subject: [PATCH v3 1/4] kunit: Always run cleanup from a test kthread
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230421/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/e6f2b343739c4656e2090449ad7eac10db57dde9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Gow/Documentation-kunit-Note-that-assertions-should-not-be-used-in-cleanup/20230421-120437
git checkout e6f2b343739c4656e2090449ad7eac10db57dde9
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash lib/kunit/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
lib/kunit/test.c: In function 'kunit_catch_run_case':
>> lib/kunit/test.c:440:29: warning: unused variable 'suite' [-Wunused-variable]
440 | struct kunit_suite *suite = ctx->suite;
| ^~~~~
vim +/suite +440 lib/kunit/test.c
e6f2b343739c46 David Gow 2023-04-21 434
e6f2b343739c46 David Gow 2023-04-21 435
5f3e06208920ee Brendan Higgins 2019-09-23 436 static void kunit_catch_run_case(void *data)
5f3e06208920ee Brendan Higgins 2019-09-23 437 {
5f3e06208920ee Brendan Higgins 2019-09-23 438 struct kunit_try_catch_context *ctx = data;
5f3e06208920ee Brendan Higgins 2019-09-23 439 struct kunit *test = ctx->test;
5f3e06208920ee Brendan Higgins 2019-09-23 @440 struct kunit_suite *suite = ctx->suite;
5f3e06208920ee Brendan Higgins 2019-09-23 441 int try_exit_code = kunit_try_catch_get_result(&test->try_catch);
5f3e06208920ee Brendan Higgins 2019-09-23 442
5f3e06208920ee Brendan Higgins 2019-09-23 443 if (try_exit_code) {
5f3e06208920ee Brendan Higgins 2019-09-23 444 kunit_set_failure(test);
5f3e06208920ee Brendan Higgins 2019-09-23 445 /*
5f3e06208920ee Brendan Higgins 2019-09-23 446 * Test case could not finish, we have no idea what state it is
5f3e06208920ee Brendan Higgins 2019-09-23 447 * in, so don't do clean up.
5f3e06208920ee Brendan Higgins 2019-09-23 448 */
5f3e06208920ee Brendan Higgins 2019-09-23 449 if (try_exit_code == -ETIMEDOUT) {
5f3e06208920ee Brendan Higgins 2019-09-23 450 kunit_err(test, "test case timed out\n");
5f3e06208920ee Brendan Higgins 2019-09-23 451 /*
5f3e06208920ee Brendan Higgins 2019-09-23 452 * Unknown internal error occurred preventing test case from
5f3e06208920ee Brendan Higgins 2019-09-23 453 * running, so there is nothing to clean up.
5f3e06208920ee Brendan Higgins 2019-09-23 454 */
5f3e06208920ee Brendan Higgins 2019-09-23 455 } else {
5f3e06208920ee Brendan Higgins 2019-09-23 456 kunit_err(test, "internal error occurred preventing test case from running: %d\n",
5f3e06208920ee Brendan Higgins 2019-09-23 457 try_exit_code);
5f3e06208920ee Brendan Higgins 2019-09-23 458 }
5f3e06208920ee Brendan Higgins 2019-09-23 459 return;
5f3e06208920ee Brendan Higgins 2019-09-23 460 }
5f3e06208920ee Brendan Higgins 2019-09-23 461 }
5f3e06208920ee Brendan Higgins 2019-09-23 462
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi,
On Fri, 2023-04-21 at 12:02 +0800, David Gow wrote:
> KUnit tests run in a kthread, with the current->kunit_test pointer set
> to the test's context. This allows the kunit_get_current_test() and
> kunit_fail_current_test() macros to work. Normally, this pointer is
> still valid during test shutdown (i.e., the suite->exit function, and
> any resource cleanup). However, if the test has exited early (e.g., due
> to a failed assertion), the cleanup is done in the parent KUnit thread,
> which does not have an active context.
>
> Instead, in the event test terminates early, run the test exit and
> cleanup from a new 'cleanup' kthread, which sets current->kunit_test,
> and better isolates the rest of KUnit from issues which arise in test
> cleanup.
>
> If a test cleanup function itself aborts (e.g., due to an assertion
> failing), there will be no further attempts to clean up: an error will
> be logged and the test failed. For example:
> # example_simple_test: test aborted during cleanup. continuing without cleaning up
>
> This should also make it easier to get access to the KUnit context,
> particularly from within resource cleanup functions, which may, for
> example, need access to data in test->priv.
>
> Signed-off-by: David Gow <[email protected]>
Great! Looks good to me.
Reviewed-by: Benjamin Berg <[email protected]>
> ---
>
> This is an updated version of / replacement of "kunit: Set the current
> KUnit context when cleaning up", which instead creates a new kthread
> for cleanup tasks if the original test kthread is aborted. This protects
> us from failed assertions during cleanup, if the test exited early.
>
> Changes since v2:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> - Always run cleanup in its own kthread
> - Therefore, never attempt to re-run it if it exits
> - Thanks, Benjamin.
> Changes since v1:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> - Move cleanup execution to another kthread
> - (Thanks, Benjamin, for pointing out the assertion issues)
>
> ---
> lib/kunit/test.c | 55 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index e2910b261112..2025e51941e6 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -419,10 +419,50 @@ 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;
> @@ -448,12 +488,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 +512,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;
On Fri, Apr 21, 2023 at 9:32 AM David Gow <[email protected]> wrote:
>
> KUnit's exit functions will run even if the corresponding init function
> fails. It's easy, when writing an exit function, to assume the init
> function succeeded, and (for example) access uninitialised memory or
> dereference NULL pointers.
>
> Note that this case exists and should be handled in the documentation.
>
> Suggested-by: Benjamin Berg <[email protected]>
> Link: https://lore.kernel.org/linux-kselftest/[email protected]/
> Signed-off-by: David Gow <[email protected]>
> ---
Thank you, David. Except for a spelling error, this looks fine to me.
Reviewed-by: Sadiya Kazi <[email protected]>
Regards,
Sadiya
>
> No changes since v2:
> https://lore.kernel.org/linux-kselftest/[email protected]/
>
> This patch was introduced in v2.
>
> ---
> Documentation/dev-tools/kunit/usage.rst | 12 ++++++++++--
> include/kunit/test.h | 3 +++
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index 9f720f1317d3..f6d6c9a9ff54 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -166,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
>
> @@ -196,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`` encoutering errors
Nit: Spelling of "encountering"
> + 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
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 57b309c6ca27..3028a1a3fcad 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -168,6 +168,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.
> */
> --
> 2.40.0.634.g4ca3ef3211-goog
>
On Fri, Apr 21, 2023 at 9:32 AM David Gow <[email protected]> wrote:
>
> As assertions abort the test cleanup process, they should be avoided
> from within a suite's exit function, or from within resource 'free'
> functions. Unlike with initialisation or main test execution, no further
> cleanup will be performed after a failed assertion, potentially causing
> a leak of resources.
>
> Signed-off-by: David Gow <[email protected]>
> ---
Thank you, David. The note looks fine to me.
Reviewed-by: Sadiya Kazi <[email protected]>
Regards,
Sadiya
>
> No changes since v2:
> https://lore.kernel.org/linux-kselftest/[email protected]/
>
> This patch was introduced in v2.
>
> ---
> Documentation/dev-tools/kunit/usage.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index 9faf2b4153fc..9f720f1317d3 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
> --------------------------
>
> --
> 2.40.0.634.g4ca3ef3211-goog
>
On Fri, Apr 21, 2023 at 12:02:15PM +0800, David Gow wrote:
> KUnit tests run in a kthread, with the current->kunit_test pointer set
> to the test's context. This allows the kunit_get_current_test() and
> kunit_fail_current_test() macros to work. Normally, this pointer is
> still valid during test shutdown (i.e., the suite->exit function, and
> any resource cleanup). However, if the test has exited early (e.g., due
> to a failed assertion), the cleanup is done in the parent KUnit thread,
> which does not have an active context.
>
> Instead, in the event test terminates early, run the test exit and
> cleanup from a new 'cleanup' kthread, which sets current->kunit_test,
> and better isolates the rest of KUnit from issues which arise in test
> cleanup.
>
> If a test cleanup function itself aborts (e.g., due to an assertion
> failing), there will be no further attempts to clean up: an error will
> be logged and the test failed. For example:
> # example_simple_test: test aborted during cleanup. continuing without cleaning up
>
> This should also make it easier to get access to the KUnit context,
> particularly from within resource cleanup functions, which may, for
> example, need access to data in test->priv.
>
> Signed-off-by: David Gow <[email protected]>
Thanks for doing this. I've tested it with an action that needs the test
priv pointer, and it works as expected
Reviewed-by: Maxime Ripard <[email protected]>
Tested-by: Maxime Ripard <[email protected]>
Thanks!
Maxime
On Fri, Apr 21, 2023 at 12:02 AM David Gow <[email protected]> wrote:
>
> Add an example .exit and .suite_exit function to the KUnit example
> suite. Given exit functions are a bit more subtle than init functions
> (due to running in a different kthread, and running even after tests or
> test init functions fail), providing an easy place to experiment with
> them is useful.
>
> Signed-off-by: David Gow <[email protected]>
Hi David!
I have reviewed this patch and the overall changes to the cleanup
structure. It looks good to me (other than that kernel test robot
error). Nice to see an example of how to use exit functions in our
example test.
Thanks!
-Rae
Reviewed-by: Rae Moar <[email protected]>
> ---
>
> This patch was introduced in v3.
>
> ---
> lib/kunit/kunit-example-test.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index cd8b7e51d02b..24315c882b31 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.
> */
> @@ -211,7 +231,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,
> };
>
> --
> 2.40.0.634.g4ca3ef3211-goog
>