Re: [PATCH v2 3/3] Documentation: kunit: Remove redundant 'tips.rst' page

From: Sadiya Kazi
Date: Fri Nov 11 2022 - 00:58:44 EST

On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@xxxxxxxxxxxxxxxx> wrote:
>
> From: David Gow <davidgow@xxxxxxxxxx>
>
> 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 <davidgow@xxxxxxxxxx>
> Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx>
> ---
> 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 <sadiyakazi@xxxxxxxxxx>


> 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
>
> --
> 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+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221109003618.3784591-3-dlatypov%40google.com.