On Thu, Sep 29, 2022 at 9:26 PM Bagas Sanjaya bagasdotme@gmail.com wrote:

The wordings of step-by-step instructions on writing the first Kunit test are instructing readers to write codes without knowing what these are about. Rewrite these instructions to include the purpose of written code.

While at it, align the code blocks of these contents.

Signed-off-by: Bagas Sanjaya bagasdotme@gmail.com

Changes since v1 [1]:

• Fix jumped list numbering on writing the feature

This patch is based on Khalid's full path to .kunitconfig patch [2].

While I like the idea behind this, the wording probably needs a bit of tweaking. In addition, by describing everything in too much detail, I fear we might just be adding some needless redundancy. My suspicion is that everyone who's going to be writing KUnit tests already knows C (or has access to better learning materials than this), so we're unlikely to need to describe in detail that, e.g., misc_example_add() adds two numbers together when the code is right there,

Also, I preferred v1 of the full path to .kunitconfig patch, so this will need rebasing on top of whatever the final version of that turns out to be.

In any case, further notes are inline, below.

Cheers, -- David

Documentation/dev-tools/kunit/start.rst | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst index 7999874dc4ddb3..c0a5adf6d8d665 100644 --- a/Documentation/dev-tools/kunit/start.rst +++ b/Documentation/dev-tools/kunit/start.rst @@ -131,17 +131,19 @@ are built-in. Otherwise the module will need to be loaded.

Writing Your First Test

-In your kernel repository, let's add some code that we can test. +In your kernel repository, let's add some code that we can test. For this +purpose, we are going to add simple addition driver.

A few issues here: 1. This should be "add _a_ simple addition driver". 2. That being said, I think we could simplify this further, and just have "let's add a function to test, which adds two numbers together and lives in its own driver. 3. This is an existing issue, but could we use "In _the_ kernel repository" or "In _our_ kernel repository" to better match "let's"?

-1. Create a file ``drivers/misc/example.h``, which includes: +1. Write the feature that will be tested. First, write the declaration

• for ``misc_example_add()`` in ``drivers/misc/example.h``:

As noted above, I think this is starting to verge on needless redundancy here. I also think there was some benefit in pointing out that this file is new, so needs to be created.

-.. code-block:: c

• .. code-block:: c

Why are all of these code-block declarations being indented? It doesn't seem to affect the actual documentation build, so I guess it's harmless, but it'd be better not to have it change unnecessarily and clutter up the diff.

    int misc_example_add(int left, int right);

-2. Create a file ``drivers/misc/example.c``, which includes:

• Then implement the function in ``drivers/misc/example.c``:

-.. code-block:: c

• .. code-block:: c

Again, code-block indentation?

    #include <linux/errno.h>

@@ -152,24 +154,25 @@ In your kernel repository, let's add some code that we can test. return left + right; }

-3. Add the following lines to ``drivers/misc/Kconfig``: +2. Add Kconfig menu entry for the feature to ``drivers/misc/Kconfig``:

This needs rewording to add back an article ("a" or "the"), and we probably want to call this a "Kconfig entry" rather than a "Kconfig menu entry". Maybe "Add a Kconfig entry for the driver..."?

-.. code-block:: kconfig

• .. code-block:: kconfig

Indentation again?

    config MISC_EXAMPLE
            bool "My example"

-4. Add the following lines to ``drivers/misc/Makefile``: +3. Add the kbuild goal that will build the feature to

• ``drivers/misc/Makefile``:

Kbuild goal? I've never heard of this being called a Kbuild goal before?

How about a "make target"?

-.. code-block:: make

• .. code-block:: make

Indentation?

    obj-$(CONFIG_MISC_EXAMPLE) += example.o

Now we are ready to write the test cases.

-1. Add the below test case in ``drivers/misc/example_test.c``: +1. Write the test in ``drivers/misc/example_test.c``:

I really like this one!

-.. code-block:: c

• .. code-block:: c

Indentation.

    #include <kunit/test.h>
    #include "example.h"

@@ -202,31 +205,32 @@ Now we are ready to write the test cases. }; kunit_test_suite(misc_example_test_suite);

-2. Add the following lines to ``drivers/misc/Kconfig``: +2. Add following Kconfig entry for the test to ``drivers/misc/Kconfig``:

"Add _the_ following Kconfig entry".

-.. code-block:: kconfig

• .. code-block:: kconfig

Indentation?

    config MISC_EXAMPLE_TEST
            tristate "Test for my example" if !KUNIT_ALL_TESTS
            depends on MISC_EXAMPLE && KUNIT=y
            default KUNIT_ALL_TESTS

-3. Add the following lines to ``drivers/misc/Makefile``: +3. Add kbuild goal of the test to ``drivers/misc/Makefile``:

Add a "make target" for the test?

-.. code-block:: make

• .. code-block:: make

Indentation?

    obj-$(CONFIG_MISC_EXAMPLE_TEST) += example_test.o

-4. Add following configuration fragments to ``.kunit/.kunitconfig``: +4. Add following configuration fragments for the test to

• ``.kunit/.kunitconfig``:

"Add _the_ following".

I still slightly prefer "config entries" or similar here...

-.. code-block:: none

• .. code-block:: none

Indentation?

    CONFIG_MISC_EXAMPLE=y
    CONFIG_MISC_EXAMPLE_TEST=y

5. Run the test:

-.. code-block:: bash

• .. code-block:: bash

Indentation?

    ./tools/testing/kunit/kunit.py run

-- An old man doll... just what I always wanted! - Clara