[PATCH v2 1/3] Documentation: KUnit: make usage.rst a superset of tips.rst, remove duplication

[Daniel Latypov]

usage.rst had most of the content of the tips.rst page copied over.
But it's missing https://www.kernel.org/doc/html/v6.0/dev-tools/kunit/tips.html#customizing-error-messages
Copy it over so we can retire tips.rst w/o losing content.

And in that process, it also gained a duplicate section about how
KUNIT_ASSERT_*() exit the test case early. Remove that.

Signed-off-by: Daniel Latypov <dlat...@google.com>
---
Documentation/dev-tools/kunit/usage.rst | 49 ++++++++++++++++---------
1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 2737863ef365..b0a6c3bc0eeb 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -118,6 +118,37 @@ expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us
to bail out of the test case if the appropriate conditions are not satisfied to
complete the test.

+Customizing error messages
+--------------------------
+
+Each of the ``KUNIT_EXPECT`` and ``KUNIT_ASSERT`` macros have a ``_MSG``
+variant. These take a format string and arguments to provide additional
+context to the automatically generated error messages.
+
+.. code-block:: c
+
+ char some_str[41];
+ generate_sha1_hex_string(some_str);
+
+ /* Before. Not easy to tell why the test failed. */
+ KUNIT_EXPECT_EQ(test, strlen(some_str), 40);
+
+ /* After. Now we see the offending string. */
+ KUNIT_EXPECT_EQ_MSG(test, strlen(some_str), 40, "some_str='%s'", some_str);
+
+Alternatively, one can take full control over the error message by using
+``KUNIT_FAIL()``, e.g.
+
+.. code-block:: c
+
+ /* Before */
+ KUNIT_EXPECT_EQ(test, some_setup_function(), 0);
+
+ /* After: full control over the failure message. */
+ if (some_setup_function())
+ KUNIT_FAIL(test, "Failed to setup thing for testing");
+
+
Test Suites
~~~~~~~~~~~

@@ -546,24 +577,6 @@ By reusing the same ``cases`` array from above, we can write the test as a
{}
};

-Exiting Early on Failed Expectations
-------------------------------------
-
-We can use ``KUNIT_EXPECT_EQ`` to mark the test as failed and continue
-execution. In some cases, it is unsafe to continue. We can use the
-``KUNIT_ASSERT`` variant to exit on failure.
-
-.. code-block:: c
-
- void example_test_user_alloc_function(struct kunit *test)
- {
- void *object = alloc_some_object_for_me();
-
- /* Make sure we got a valid pointer back. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, object);
- do_something_with_object(object);
- }
-
Allocating Memory
-----------------


base-commit: 6fe1ad4a156095859721fef85073df3ed43081d4
--
2.38.1.431.g37b22c650d-goog

[Daniel Latypov]

The existing wording implies that kunit_kmalloc_array() is "the method
under test". We're actually testing the sort() function in that example.
This is because the example was changed in commit 953574390634
("Documentation: KUnit: Rework writing page to focus on writing tests"),
but the wording was not.

Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
macros from any function. Some users might be coming from a framework
like gUnit where that'll compile but silently do the wrong thing.

Signed-off-by: Daniel Latypov <dlat...@google.com>
---

Documentation/dev-tools/kunit/usage.rst | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index b0a6c3bc0eeb..8060114e3aa6 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -112,11 +112,14 @@ terminates the test case if the condition is not satisfied. For example:
KUNIT_EXPECT_LE(test, a[i], a[i + 1]);
}

-In this example, the method under test should return pointer to a value. If the
-pointer returns null or an errno, we want to stop the test since the following
-expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us
-to bail out of the test case if the appropriate conditions are not satisfied to
-complete the test.
+In this example, we need to be able to allocate an array to test the ``sort()``
+function. So we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()`` to abort the test if
+we there's an allocation error.
+
+.. note::
+ In other test frameworks, ``ASSERT`` macros are often implemented by calling
+ ``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.

Customizing error messages
--------------------------
--
2.38.1.431.g37b22c650d-goog

[Daniel Latypov]

From: David Gow <davi...@google.com>

The contents of 'tips.rst' was mostly included in 'usage.rst' way back in

commit 953574390634 ("Documentation: KUnit: Rework writing page to focus on writing tests"),

but the tips page remained behind as well.

The parent patches in this series fill in the gaps, so now 'tips.rst' is
redundant.
Therefore, delete 'tips.rst'.

While I regret breaking any links to 'tips' which might exist
externally, it's confusing to have two subtly different versions of the
same content around.

Signed-off-by: David Gow <davi...@google.com>

Signed-off-by: Daniel Latypov <dlat...@google.com>
---

v1 -> v2: rebased onto some parent patches to fix the missing sections
in usage.rst and tweaked the commit message to reflect that.
---
Documentation/dev-tools/kunit/index.rst | 1 -
Documentation/dev-tools/kunit/tips.rst | 190 ------------------------
2 files changed, 191 deletions(-)
delete mode 100644 Documentation/dev-tools/kunit/tips.rst

diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst
index f5d13f1d37be..d5629817cd72 100644
--- a/Documentation/dev-tools/kunit/index.rst
+++ b/Documentation/dev-tools/kunit/index.rst
@@ -16,7 +16,6 @@ KUnit - Linux Kernel Unit Testing
api/index
style
faq
- tips
running_tips

This section details the kernel unit testing framework.
diff --git a/Documentation/dev-tools/kunit/tips.rst b/Documentation/dev-tools/kunit/tips.rst
deleted file mode 100644
index 492d2ded2f5a..000000000000
--- a/Documentation/dev-tools/kunit/tips.rst
+++ /dev/null
@@ -1,190 +0,0 @@
-.. SPDX-License-Identifier: GPL-2.0
-
-============================
-Tips For Writing KUnit Tests
-============================
-
-Exiting early on failed expectations
-------------------------------------
-
-``KUNIT_EXPECT_EQ`` and friends will mark the test as failed and continue
-execution. In some cases, it's unsafe to continue and you can use the

-``KUNIT_ASSERT`` variant to exit on failure.
-
-.. code-block:: c
-
- void example_test_user_alloc_function(struct kunit *test)
- {
- void *object = alloc_some_object_for_me();
-
- /* Make sure we got a valid pointer back. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, object);
- do_something_with_object(object);
- }
-

-Allocating memory
------------------
-
-Where you would use ``kzalloc``, you should prefer ``kunit_kzalloc`` instead.
-KUnit will ensure the memory is freed once the test completes.
-
-This is particularly useful since it lets you use the ``KUNIT_ASSERT_EQ``
-macros to exit early from a test without having to worry about remembering to
-call ``kfree``.
-
-Example:

-
-.. code-block:: c
-

- void example_test_allocation(struct kunit *test)
- {
- char *buffer = kunit_kzalloc(test, 16, GFP_KERNEL);
- /* Ensure allocation succeeded. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buffer);
-
- KUNIT_ASSERT_STREQ(test, buffer, "");
- }
-
-
-Testing static functions
-------------------------
-
-If you don't want to expose functions or variables just for testing, one option
-is to conditionally ``#include`` the test file at the end of your .c file, e.g.

-
-.. code-block:: c
-

- /* In my_file.c */
-
- static int do_interesting_thing();
-
- #ifdef CONFIG_MY_KUNIT_TEST
- #include "my_kunit_test.c"
- #endif
-
-Injecting test-only code
-------------------------
-
-Similarly to the above, it can be useful to add test-specific logic.

-
-.. code-block:: c
-

- /* In my_file.h */
-
- #ifdef CONFIG_MY_KUNIT_TEST
- /* Defined in my_kunit_test.c */
- void test_only_hook(void);
- #else
- void test_only_hook(void) { }
- #endif
-
-This test-only code can be made more useful by accessing the current kunit
-test, see below.
-
-Accessing the current test
---------------------------
-
-In some cases, you need to call test-only code from outside the test file, e.g.
-like in the example above or if you're providing a fake implementation of an
-ops struct.
-There is a ``kunit_test`` field in ``task_struct``, so you can access it via
-``current->kunit_test``.
-
-Here's a slightly in-depth example of how one could implement "mocking":

-
-.. code-block:: c
-

- #include <linux/sched.h> /* for current */
-
- struct test_data {
- int foo_result;
- int want_foo_called_with;
- };
-
- static int fake_foo(int arg)
- {
- struct kunit *test = current->kunit_test;
- struct test_data *test_data = test->priv;
-
- KUNIT_EXPECT_EQ(test, test_data->want_foo_called_with, arg);
- return test_data->foo_result;
- }
-
- static void example_simple_test(struct kunit *test)
- {
- /* Assume priv is allocated in the suite's .init */
- struct test_data *test_data = test->priv;
-
- test_data->foo_result = 42;
- test_data->want_foo_called_with = 1;
-
- /* In a real test, we'd probably pass a pointer to fake_foo somewhere
- * like an ops struct, etc. instead of calling it directly. */
- KUNIT_EXPECT_EQ(test, fake_foo(1), 42);
- }
-
-
-Note: here we're able to get away with using ``test->priv``, but if you wanted
-something more flexible you could use a named ``kunit_resource``, see
-Documentation/dev-tools/kunit/api/test.rst.
-
-Failing the current test
-------------------------
-
-But sometimes, you might just want to fail the current test. In that case, we
-have ``kunit_fail_current_test(fmt, args...)`` which is defined in ``<kunit/test-bug.h>`` and
-doesn't require pulling in ``<kunit/test.h>``.
-
-E.g. say we had an option to enable some extra debug checks on some data structure:

-
-.. code-block:: c
-

- #include <kunit/test-bug.h>
-
- #ifdef CONFIG_EXTRA_DEBUG_CHECKS
- static void validate_my_data(struct data *data)
- {
- if (is_valid(data))
- return;
-
- kunit_fail_current_test("data %p is invalid", data);
-
- /* Normal, non-KUnit, error reporting code here. */
- }
- #else
- static void my_debug_function(void) { }
- #endif
-
-
-Customizing error messages
---------------------------
-
-Each of the ``KUNIT_EXPECT`` and ``KUNIT_ASSERT`` macros have a ``_MSG`` variant.
-These take a format string and arguments to provide additional context to the automatically generated error messages.

-
-.. code-block:: c
-

- char some_str[41];
- generate_sha1_hex_string(some_str);
-
- /* Before. Not easy to tell why the test failed. */
- KUNIT_EXPECT_EQ(test, strlen(some_str), 40);
-
- /* After. Now we see the offending string. */
- KUNIT_EXPECT_EQ_MSG(test, strlen(some_str), 40, "some_str='%s'", some_str);
-
-Alternatively, one can take full control over the error message by using ``KUNIT_FAIL()``, e.g.

-
-.. code-block:: c
-

- /* Before */
- KUNIT_EXPECT_EQ(test, some_setup_function(), 0);
-
- /* After: full control over the failure message. */
- if (some_setup_function())
- KUNIT_FAIL(test, "Failed to setup thing for testing");
-
-Next Steps
-==========
-* Optional: see the Documentation/dev-tools/kunit/usage.rst page for a more
- in-depth explanation of KUnit.
--
2.38.1.431.g37b22c650d-goog

[Sadiya Kazi]

On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development
<kuni...@googlegroups.com> wrote:
>
> usage.rst had most of the content of the tips.rst page copied over.
> But it's missing https://www.kernel.org/doc/html/v6.0/dev-tools/kunit/tips.html#customizing-error-messages
> Copy it over so we can retire tips.rst w/o losing content.
>
> And in that process, it also gained a duplicate section about how
> KUNIT_ASSERT_*() exit the test case early. Remove that.
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> ---

Thanks Daniel. This looks fine to me.
Reviewed-by: Sadiya Kazi <sadiy...@google.com>

Best Regards,
Sadiya

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221109003618.3784591-1-dlatypov%40google.com.

[Sadiya Kazi]

On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development
<kuni...@googlegroups.com> wrote:
>

> The existing wording implies that kunit_kmalloc_array() is "the method
> under test". We're actually testing the sort() function in that example.
> This is because the example was changed in commit 953574390634
> ("Documentation: KUnit: Rework writing page to focus on writing tests"),
> but the wording was not.
>
> Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
> macros from any function. Some users might be coming from a framework
> like gUnit where that'll compile but silently do the wrong thing.
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> ---

Thank you, Daniel. This looks fine to me except for a small typo in
this line "to abort
the test if we there's an allocation error". Also, I have reworded
that paragraph a bit
as below. Please feel free to ignore, if you do not agree:

In this example, to test the ``sort()`` function, we must be able to
allocate an array.
If there is an allocation error, the test is terminated using the function
``KUNIT ASSERT NOT ERR OR NULL()``.

Reviewed-by: Sadiya Kazi <sadiy...@google.com>

Best Regards,
Sadiya

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221109003618.3784591-2-dlatypov%40google.com.

[Daniel Latypov]

On Wed, Nov 9, 2022 at 9:07 PM Sadiya Kazi <sadiy...@google.com> wrote:
>
> On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development
> <kuni...@googlegroups.com> wrote:
> >
> > The existing wording implies that kunit_kmalloc_array() is "the method
> > under test". We're actually testing the sort() function in that example.
> > This is because the example was changed in commit 953574390634
> > ("Documentation: KUnit: Rework writing page to focus on writing tests"),
> > but the wording was not.
> >
> > Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
> > macros from any function. Some users might be coming from a framework
> > like gUnit where that'll compile but silently do the wrong thing.
> >
> > Signed-off-by: Daniel Latypov <dlat...@google.com>
> > ---
>
> Thank you, Daniel. This looks fine to me except for a small typo in
> this line "to abort
> the test if we there's an allocation error". Also, I have reworded
> that paragraph a bit
> as below. Please feel free to ignore, if you do not agree:
>
> In this example, to test the ``sort()`` function, we must be able to
> allocate an array.
> If there is an allocation error, the test is terminated using the function
> ``KUNIT ASSERT NOT ERR OR NULL()``.

Thanks for catching that.

Hmm, I slightly prefer the current structure since I like having the
<thing> being described near the start of the sentence as opposed to
the very end.
I'll wait a bit before sending a v3 to give time for anyone else to
chime in, if they want.

Snipping the email to the block in question:

[Sadiya Kazi]

On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development
<kuni...@googlegroups.com> wrote:
>

> From: David Gow <davi...@google.com>
>
> The contents of 'tips.rst' was mostly included in 'usage.rst' way back in
> commit 953574390634 ("Documentation: KUnit: Rework writing page to focus on writing tests"),
> but the tips page remained behind as well.
>
> The parent patches in this series fill in the gaps, so now 'tips.rst' is
> redundant.
> Therefore, delete 'tips.rst'.
>
> While I regret breaking any links to 'tips' which might exist
> externally, it's confusing to have two subtly different versions of the
> same content around.
>
> Signed-off-by: David Gow <davi...@google.com>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> ---
> v1 -> v2: rebased onto some parent patches to fix the missing sections
> in usage.rst and tweaked the commit message to reflect that.
> ---

Thank you. This looks fine to me.
Reviewed-by: Sadiya Kazi <sadiy...@google.com>

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221109003618.3784591-3-dlatypov%40google.com.

[Daniel Latypov]

usage.rst had most of the content of the tips.rst page copied over.
But it's missing https://www.kernel.org/doc/html/v6.0/dev-tools/kunit/tips.html#customizing-error-messages
Copy it over so we can retire tips.rst w/o losing content.

And in that process, it also gained a duplicate section about how
KUNIT_ASSERT_*() exit the test case early. Remove that.

Signed-off-by: Daniel Latypov <dlat...@google.com>

Reviewed-by: Sadiya Kazi <sadiy...@google.com>
---
Documentation/dev-tools/kunit/usage.rst | 49 ++++++++++++++++---------
1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 2737863ef365..b0a6c3bc0eeb 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -118,6 +118,37 @@ expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us

to bail out of the test case if the appropriate conditions are not satisfied to

-execution. In some cases, it is unsafe to continue. We can use the

-``KUNIT_ASSERT`` variant to exit on failure.
-
-.. code-block:: c
-
- void example_test_user_alloc_function(struct kunit *test)
- {
- void *object = alloc_some_object_for_me();
-
- /* Make sure we got a valid pointer back. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, object);
- do_something_with_object(object);
- }
-

Allocating Memory
-----------------


base-commit: 870f63b7cd78d0055902d839a60408f7428b4e84
--
2.38.1.431.g37b22c650d-goog

[Daniel Latypov]

The existing wording implies that kunit_kmalloc_array() is "the method
under test". We're actually testing the sort() function in that example.

This is because the example was changed in commit 953574390634

("Documentation: KUnit: Rework writing page to focus on writing tests"),

but the wording was not.

Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
macros from any function. Some users might be coming from a framework
like gUnit where that'll compile but silently do the wrong thing.

Signed-off-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: Sadiya Kazi <sadiy...@google.com>
---

Documentation/dev-tools/kunit/usage.rst | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index b0a6c3bc0eeb..22416ebb94ab 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst

@@ -112,11 +112,14 @@ terminates the test case if the condition is not satisfied. For example:
KUNIT_EXPECT_LE(test, a[i], a[i + 1]);
}

-In this example, the method under test should return pointer to a value. If the
-pointer returns null or an errno, we want to stop the test since the following

-expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us
-to bail out of the test case if the appropriate conditions are not satisfied to
-complete the test.

+In this example, we need to be able to allocate an array to test the ``sort()``
+function. So we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()`` to abort the test if

+there's an allocation error.

[Daniel Latypov]

From: David Gow <davi...@google.com>

The contents of 'tips.rst' was mostly included in 'usage.rst' way back in

commit 953574390634 ("Documentation: KUnit: Rework writing page to focus on writing tests"),

but the tips page remained behind as well.

The parent patches in this series fill in the gaps, so now 'tips.rst' is
redundant.
Therefore, delete 'tips.rst'.

While I regret breaking any links to 'tips' which might exist
externally, it's confusing to have two subtly different versions of the
same content around.

Signed-off-by: David Gow <davi...@google.com>

Signed-off-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: Sadiya Kazi <sadiy...@google.com>
---

v1 -> v2: rebased onto some parent patches to fix the missing sections
in usage.rst and tweaked the commit message to reflect that.
---

-execution. In some cases, it's unsafe to continue and you can use the

-``KUNIT_ASSERT`` variant to exit on failure.
-
-.. code-block:: c
-
- void example_test_user_alloc_function(struct kunit *test)
- {
- void *object = alloc_some_object_for_me();
-
- /* Make sure we got a valid pointer back. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, object);
- do_something_with_object(object);
- }
-

-Allocating memory
------------------
-
-Where you would use ``kzalloc``, you should prefer ``kunit_kzalloc`` instead.
-KUnit will ensure the memory is freed once the test completes.
-
-This is particularly useful since it lets you use the ``KUNIT_ASSERT_EQ``
-macros to exit early from a test without having to worry about remembering to
-call ``kfree``.
-
-Example:

-
-.. code-block:: c
-

- void example_test_allocation(struct kunit *test)
- {
- char *buffer = kunit_kzalloc(test, 16, GFP_KERNEL);
- /* Ensure allocation succeeded. */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buffer);
-
- KUNIT_ASSERT_STREQ(test, buffer, "");
- }
-
-
-Testing static functions
-------------------------
-
-If you don't want to expose functions or variables just for testing, one option
-is to conditionally ``#include`` the test file at the end of your .c file, e.g.

-
-.. code-block:: c
-

- /* In my_file.c */
-
- static int do_interesting_thing();
-
- #ifdef CONFIG_MY_KUNIT_TEST
- #include "my_kunit_test.c"
- #endif
-
-Injecting test-only code
-------------------------
-
-Similarly to the above, it can be useful to add test-specific logic.

-
-.. code-block:: c
-

- /* In my_file.h */
-
- #ifdef CONFIG_MY_KUNIT_TEST
- /* Defined in my_kunit_test.c */
- void test_only_hook(void);
- #else
- void test_only_hook(void) { }
- #endif
-
-This test-only code can be made more useful by accessing the current kunit
-test, see below.
-
-Accessing the current test
---------------------------
-
-In some cases, you need to call test-only code from outside the test file, e.g.
-like in the example above or if you're providing a fake implementation of an
-ops struct.
-There is a ``kunit_test`` field in ``task_struct``, so you can access it via
-``current->kunit_test``.
-
-Here's a slightly in-depth example of how one could implement "mocking":

-
-.. code-block:: c
-

-
-.. code-block:: c
-

- #include <kunit/test-bug.h>
-
- #ifdef CONFIG_EXTRA_DEBUG_CHECKS
- static void validate_my_data(struct data *data)
- {
- if (is_valid(data))
- return;
-
- kunit_fail_current_test("data %p is invalid", data);
-
- /* Normal, non-KUnit, error reporting code here. */
- }
- #else
- static void my_debug_function(void) { }
- #endif
-
-
-Customizing error messages
---------------------------
-

-Each of the ``KUNIT_EXPECT`` and ``KUNIT_ASSERT`` macros have a ``_MSG`` variant.
-These take a format string and arguments to provide additional context to the automatically generated error messages.

-
-.. code-block:: c
-

- char some_str[41];
- generate_sha1_hex_string(some_str);
-
- /* Before. Not easy to tell why the test failed. */
- KUNIT_EXPECT_EQ(test, strlen(some_str), 40);
-

- /* After. Now we see the offending string. */
- KUNIT_EXPECT_EQ_MSG(test, strlen(some_str), 40, "some_str='%s'", some_str);
-
-Alternatively, one can take full control over the error message by using ``KUNIT_FAIL()``, e.g.

-
-.. code-block:: c
-

[David Gow]

+1 for the patch from me (modulo the "we" typo Sadiya mentioned).

I otherwise also prefer Daniel's original here (though I'd possibly
merge it into one sentence, personally).
Maybe:
"In this example, as we need to be able to allocate an array in order
to test the sort function, we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()``
to abort the test if there's an allocation error."
or
"In this example, we need to allocate an array to test the sort
function. We therefore use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()``, which
will automatically abort the test if there's an allocation error."

But any of the above wordings are fine for me.

The note about ASSERT() working in any function is useful, though
there are definitely some "gotcha"s caused by killing the kthread
we'll need to resolve. (If there are any dangling references to things
on the stack, for example.) Still, not an issue for this bit of
documentation.

Reviewed-by: David Gow <davi...@google.com>

(Once the "we" typo is fixed.)

Cheers,
-- David

[Daniel Latypov]

On Mon, Nov 14, 2022 at 11:45 PM David Gow <davi...@google.com> wrote:

<snip>

> +1 for the patch from me (modulo the "we" typo Sadiya mentioned).
>
> I otherwise also prefer Daniel's original here (though I'd possibly
> merge it into one sentence, personally).
> Maybe:
> "In this example, as we need to be able to allocate an array in order
> to test the sort function, we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()``
> to abort the test if there's an allocation error."
> or
> "In this example, we need to allocate an array to test the sort
> function. We therefore use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()``, which
> will automatically abort the test if there's an allocation error."
>
> But any of the above wordings are fine for me.
>
> The note about ASSERT() working in any function is useful, though
> there are definitely some "gotcha"s caused by killing the kthread
> we'll need to resolve. (If there are any dangling references to things
> on the stack, for example.) Still, not an issue for this bit of
> documentation.
>
> Reviewed-by: David Gow <davi...@google.com>
>
> (Once the "we" typo is fixed.)

v3 is here, PTAL
https://lore.kernel.org/all/20221111182906.1...@google.com/

Copying the relevant section here:

+In this example, we need to be able to allocate an array to test the ``sort()``
+function. So we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()`` to abort the test if

+there's an allocation error.

+
+.. note::
+ In other test frameworks, ``ASSERT`` macros are often implemented by calling
+ ``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.

Daniel

[David Gow]

On Sat, Nov 12, 2022 at 2:29 AM 'Daniel Latypov' via KUnit Development
<kuni...@googlegroups.com> wrote:
>
> usage.rst had most of the content of the tips.rst page copied over.
> But it's missing https://www.kernel.org/doc/html/v6.0/dev-tools/kunit/tips.html#customizing-error-messages
> Copy it over so we can retire tips.rst w/o losing content.
>
> And in that process, it also gained a duplicate section about how
> KUNIT_ASSERT_*() exit the test case early. Remove that.
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> Reviewed-by: Sadiya Kazi <sadiy...@google.com>
> ---

Looks good to me, thanks!

Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221111182906.1377191-1-dlatypov%40google.com.

[David Gow]

On Sat, Nov 12, 2022 at 2:29 AM Daniel Latypov <dlat...@google.com> wrote:
>
> The existing wording implies that kunit_kmalloc_array() is "the method
> under test". We're actually testing the sort() function in that example.
> This is because the example was changed in commit 953574390634
> ("Documentation: KUnit: Rework writing page to focus on writing tests"),
> but the wording was not.
>
> Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
> macros from any function. Some users might be coming from a framework
> like gUnit where that'll compile but silently do the wrong thing.
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> Reviewed-by: Sadiya Kazi <sadiy...@google.com>
> ---

Looking good!

Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David