diff mbox series

[v3,3/4] Documentation: kunit: Warn that exit functions run even if init fails

Message ID 20230421040218.2156548-3-davidgow@google.com (mailing list archive)
State New
Delegated to: Brendan Higgins
Headers show
Series [v3,1/4] kunit: Always run cleanup from a test kthread | expand

Commit Message

David Gow April 21, 2023, 4:02 a.m. UTC
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 <benjamin@sipsolutions.net>
Link: https://lore.kernel.org/linux-kselftest/a39af0400abedb2e9b31d84c37551cecc3eed0e1.camel@sipsolutions.net/
Signed-off-by: David Gow <davidgow@google.com>
---

No changes since v2:
https://lore.kernel.org/linux-kselftest/20230419085426.1671703-3-davidgow@google.com/

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(-)

Comments

Sadiya Kazi April 25, 2023, 5:48 a.m. UTC | #1
On Fri, Apr 21, 2023 at 9:32 AM David Gow <davidgow@google.com> 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 <benjamin@sipsolutions.net>
> Link: https://lore.kernel.org/linux-kselftest/a39af0400abedb2e9b31d84c37551cecc3eed0e1.camel@sipsolutions.net/
> Signed-off-by: David Gow <davidgow@google.com>
> ---

Thank you, David. Except for a spelling error, this looks fine to me.
Reviewed-by: Sadiya Kazi <sadiyakazi@google.com>

Regards,
Sadiya
>
> No changes since v2:
> https://lore.kernel.org/linux-kselftest/20230419085426.1671703-3-davidgow@google.com/
>
> 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
>
diff mbox series

Patch

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.
  */