diff mbox series

[v2] Documentation: kunit: Add CLI args for kunit_tool

Message ID 20220721081026.1247067-1-sadiyakazi@google.com (mailing list archive)
State New
Delegated to: Brendan Higgins
Headers show
Series [v2] Documentation: kunit: Add CLI args for kunit_tool | expand

Commit Message

Sadiya Kazi July 21, 2022, 8:10 a.m. UTC
Run_wrapper.rst was missing some command line arguments. Added
additional args in the file.

Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>
---
Changes since V1:
https://lore.kernel.org/linux-kselftest/20220719092214.995965-1-sadiyakazi@google.com/
- Addressed most of the review comments from Maira and David, except
  removing the duplicate arguments as I felt its worth keeping them in
  the reference documentation as well as in context. We can improve them
  and differentiate their use cases in the future patches.


---
 Documentation/dev-tools/kunit/run_wrapper.rst | 60 ++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

Comments

Maíra Canal July 21, 2022, 11:08 a.m. UTC | #1
On 7/21/22 05:10, Sadiya Kazi wrote:
> Run_wrapper.rst was missing some command line arguments. Added
> additional args in the file.
> 
> Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>
> ---
> Changes since V1:
> https://lore.kernel.org/linux-kselftest/20220719092214.995965-1-sadiyakazi@google.com/
> - Addressed most of the review comments from Maira and David, except
>   removing the duplicate arguments as I felt its worth keeping them in
>   the reference documentation as well as in context. We can improve them
>   and differentiate their use cases in the future patches.
> 
> 
> ---
>  Documentation/dev-tools/kunit/run_wrapper.rst | 60 ++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/dev-tools/kunit/run_wrapper.rst b/Documentation/dev-tools/kunit/run_wrapper.rst
> index 5e560f2c5fca..600af7ac5f88 100644
> --- a/Documentation/dev-tools/kunit/run_wrapper.rst
> +++ b/Documentation/dev-tools/kunit/run_wrapper.rst
> @@ -233,7 +233,7 @@ Command-Line Arguments
>  ======================
>  
>  kunit_tool has a number of other command-line arguments which can
> -be useful for our test environment. Below the most commonly used
> +be useful for our test environment. Below are the most commonly used
>  command line arguments:
>  
>  - ``--help``: Lists all available options. To list common options,
> @@ -257,3 +257,61 @@ command line arguments:
>              added or modified. Instead, enable all tests
>              which have satisfied dependencies by adding
>              ``CONFIG_KUNIT_ALL_TESTS=y`` to your ``.kunitconfig``.
> +
> +- ``--kunitconfig``: Specifies the path or the directory of the ``.kunitconfig``
> +  file. For example:
> +
> +  - ``lib/kunit/.kunitconfig`` can be the path of the file.
> +
> +  - ``lib/kunit`` can be the directory in which the file is located.
> +
> +  This file is used to build and run with a predefined set of tests
> +  and their dependencies. For example, to run tests for a given subsystem.
> +
> +- ``--kconfig_add``: Specifies additional configuration options to be
> +  appended to the ``.kunitconfig`` file.
> +  For example, ``./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y``.

Small nit pick: I would rather do:

```
./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y
```

> +
> +- ``--arch``: Runs tests on the specified architecture. The architecture
> +  specified must match the Kbuild ARCH environment variable.
> +  For example, i386, x86_64, arm, um, etc. Non-UML architectures run on QEMU.
> +  Default is `um`.
> +
> +- ``--cross_compile``: Specifies the Kbuild toolchain. It passes the
> +  same argument as passed to the ``CROSS_COMPILE`` variable used by
> +  Kbuild. This will be the prefix for the toolchain
> +  binaries such as GCC. For example:
> +
> +  - ``sparc64-linux-gnu-`` if we have the sparc toolchain installed on
> +    our system.
> +
> +  - ``$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux``
> +    if we have downloaded the microblaze toolchain from the 0-day
> +    website to a specified path in our home directory called toolchains.
> +
> +- ``--qemu_config``: Specifies the path to a file containing a
> +  custom qemu architecture definition. This should be a python file
> +  containing a `QemuArchParams` object.

Nit: choose a standard for referring to qemu. Either "qemu" or "QEMU" is
great for me, but it is ideal that you chose one and stick with it.
Here, you used "qemu" and on the next argument, you used "QEMU".

> +
> +- ``--qemu_args``: Specifies additional QEMU arguments, for example, "-smp 8".
> +
> +- ``--jobs``: Specifies the number of jobs (commands) to run simultaneously.
> +  By default, this is set to the number of cores on your system.
> +
> +- ``--timeout``: Specifies the maximum number of seconds allowed for all tests to run.
> +  This does not include the time taken to build the tests.
> +
> +- ``--kernel_args``: Specifies additional kernel command-line arguments. Might be repeated.
> +
> +- ``--run_isolated``: If set, boots the kernel for each individual suite/test.
> +  This is useful for debugging a non-hermetic test, one that
> +  might pass/fail based on what ran before it.
> +
> +- ``--raw_output``: If set, generates unformatted output from kernel. Possible options are:
> +
> +   - ``all``: To view the full kernel output, use ``--raw_output=all``.
> +
> +   - ``kunit``: This is the default option and filters to KUnit output. Use ``--raw_output`` or ``--raw_output=kunit``.
> +
> +- ``--json``: If set, stores the test results in a JSON format and prints to `stdout` or
> +  saves to a file if a filename is specified.

Anyway, the documentation is pretty good and informative! The small nits
I pointed out are optional. So,

Reviewed-by: Maíra Canal <mairacanal@riseup.net>

Best Regards,
- Maíra Canal
David Gow July 21, 2022, 11:54 p.m. UTC | #2
On Thu, Jul 21, 2022 at 4:26 PM Sadiya Kazi <sadiyakazi@google.com> wrote:
>
> Run_wrapper.rst was missing some command line arguments. Added
> additional args in the file.
>
> Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>
> ---
> Changes since V1:
> https://lore.kernel.org/linux-kselftest/20220719092214.995965-1-sadiyakazi@google.com/
> - Addressed most of the review comments from Maira and David, except
>   removing the duplicate arguments as I felt its worth keeping them in
>   the reference documentation as well as in context. We can improve them
>   and differentiate their use cases in the future patches.
>
>

Looks good to me. A couple of super-minor suggestions below, and a
note about how KASAN/UML support hasn't fully landed yet (but I still
like it as an example, so maybe leave it as-is).

None of these are deal breakers, though, and I'd be okay with this
going in as-is, as well.

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

Cheers,
-- David

> ---
>  Documentation/dev-tools/kunit/run_wrapper.rst | 60 ++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/run_wrapper.rst b/Documentation/dev-tools/kunit/run_wrapper.rst
> index 5e560f2c5fca..600af7ac5f88 100644
> --- a/Documentation/dev-tools/kunit/run_wrapper.rst
> +++ b/Documentation/dev-tools/kunit/run_wrapper.rst
> @@ -233,7 +233,7 @@ Command-Line Arguments
>  ======================
>
>  kunit_tool has a number of other command-line arguments which can
> -be useful for our test environment. Below the most commonly used
> +be useful for our test environment. Below are the most commonly used
>  command line arguments:
>
>  - ``--help``: Lists all available options. To list common options,
> @@ -257,3 +257,61 @@ command line arguments:
>              added or modified. Instead, enable all tests
>              which have satisfied dependencies by adding
>              ``CONFIG_KUNIT_ALL_TESTS=y`` to your ``.kunitconfig``.
> +
> +- ``--kunitconfig``: Specifies the path or the directory of the ``.kunitconfig``
> +  file. For example:
> +
> +  - ``lib/kunit/.kunitconfig`` can be the path of the file.
> +
> +  - ``lib/kunit`` can be the directory in which the file is located.
> +
> +  This file is used to build and run with a predefined set of tests
> +  and their dependencies. For example, to run tests for a given subsystem.
> +
> +- ``--kconfig_add``: Specifies additional configuration options to be
> +  appended to the ``.kunitconfig`` file.
> +  For example, ``./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y``.
> +

Just FYI, this example won't work as-is until KASAN for UML is merged.
It's already sitting in uml/next, so this shouldn't be a problem:
https://git.kernel.org/pub/scm/linux/kernel/git/uml/linux.git/commit/?h=next&id=5b301409e8bc5d7fad2ee138be44c5c529dd0874

But if you want to try it on 5.19 or the current kunit branch, you'll
need to add, e.g., --arch=x86_64

> +- ``--arch``: Runs tests on the specified architecture. The architecture
> +  specified must match the Kbuild ARCH environment variable.

I'm not quite sold on 'must match the Kbuild ARCH environment
variable'. That seems to imply to me that you need to set ARCH= _and_
use --arch. Instead, --arch itself sets ARCH=, so the values use the
same names.

> +  For example, i386, x86_64, arm, um, etc. Non-UML architectures run on QEMU.
> +  Default is `um`.
> +
> +- ``--cross_compile``: Specifies the Kbuild toolchain. It passes the
> +  same argument as passed to the ``CROSS_COMPILE`` variable used by
> +  Kbuild. This will be the prefix for the toolchain
> +  binaries such as GCC. For example:
> +
> +  - ``sparc64-linux-gnu-`` if we have the sparc toolchain installed on
> +    our system.
> +
> +  - ``$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux``
> +    if we have downloaded the microblaze toolchain from the 0-day
> +    website to a specified path in our home directory called toolchains.
> +
> +- ``--qemu_config``: Specifies the path to a file containing a
> +  custom qemu architecture definition. This should be a python file
> +  containing a `QemuArchParams` object.
> +
> +- ``--qemu_args``: Specifies additional QEMU arguments, for example, "-smp 8".
> +
> +- ``--jobs``: Specifies the number of jobs (commands) to run simultaneously.4
> +  By default, this is set to the number of cores on your system.
> +
> +- ``--timeout``: Specifies the maximum number of seconds allowed for all tests to run.
> +  This does not include the time taken to build the tests.
> +
> +- ``--kernel_args``: Specifies additional kernel command-line arguments. Might be repeated.

Nit: maybe "can be repeated"? As it's the reader of this documentation
who will likely be doing the repeating. Or "may be repeated"?
> +
> +- ``--run_isolated``: If set, boots the kernel for each individual suite/test.
> +  This is useful for debugging a non-hermetic test, one that
> +  might pass/fail based on what ran before it.
> +
> +- ``--raw_output``: If set, generates unformatted output from kernel. Possible options are:
> +
> +   - ``all``: To view the full kernel output, use ``--raw_output=all``.
> +
> +   - ``kunit``: This is the default option and filters to KUnit output. Use ``--raw_output`` or ``--raw_output=kunit``.
> +
> +- ``--json``: If set, stores the test results in a JSON format and prints to `stdout` or
> +  saves to a file if a filename is specified.
> --
> 2.37.0.170.g444d1eabd0-goog
>
Brendan Higgins July 22, 2022, 12:24 a.m. UTC | #3
On Thu, Jul 21, 2022 at 7:54 PM David Gow <davidgow@google.com> wrote:
>
> On Thu, Jul 21, 2022 at 4:26 PM Sadiya Kazi <sadiyakazi@google.com> wrote:

[...]

> >  Documentation/dev-tools/kunit/run_wrapper.rst | 60 ++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/run_wrapper.rst b/Documentation/dev-tools/kunit/run_wrapper.rst
> > index 5e560f2c5fca..600af7ac5f88 100644
> > --- a/Documentation/dev-tools/kunit/run_wrapper.rst
> > +++ b/Documentation/dev-tools/kunit/run_wrapper.rst

[...]

> > @@ -257,3 +257,61 @@ command line arguments:
> >              added or modified. Instead, enable all tests
> >              which have satisfied dependencies by adding
> >              ``CONFIG_KUNIT_ALL_TESTS=y`` to your ``.kunitconfig``.
> > +
> > +- ``--kunitconfig``: Specifies the path or the directory of the ``.kunitconfig``
> > +  file. For example:
> > +
> > +  - ``lib/kunit/.kunitconfig`` can be the path of the file.
> > +
> > +  - ``lib/kunit`` can be the directory in which the file is located.
> > +
> > +  This file is used to build and run with a predefined set of tests
> > +  and their dependencies. For example, to run tests for a given subsystem.
> > +
> > +- ``--kconfig_add``: Specifies additional configuration options to be
> > +  appended to the ``.kunitconfig`` file.
> > +  For example, ``./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y``.
> > +
>
> Just FYI, this example won't work as-is until KASAN for UML is merged.
> It's already sitting in uml/next, so this shouldn't be a problem:
> https://git.kernel.org/pub/scm/linux/kernel/git/uml/linux.git/commit/?h=next&id=5b301409e8bc5d7fad2ee138be44c5c529dd0874
>
> But if you want to try it on 5.19 or the current kunit branch, you'll
> need to add, e.g., --arch=x86_64
>
> > +- ``--arch``: Runs tests on the specified architecture. The architecture
> > +  specified must match the Kbuild ARCH environment variable.
>
> I'm not quite sold on 'must match the Kbuild ARCH environment
> variable'. That seems to imply to me that you need to set ARCH= _and_
> use --arch. Instead, --arch itself sets ARCH=, so the values use the
> same names.

Agreed, I was just about to reply with a comment to the same effect.

> > +  For example, i386, x86_64, arm, um, etc. Non-UML architectures run on QEMU.
> > +  Default is `um`.
> > +
> > +- ``--cross_compile``: Specifies the Kbuild toolchain. It passes the
> > +  same argument as passed to the ``CROSS_COMPILE`` variable used by
> > +  Kbuild. This will be the prefix for the toolchain
> > +  binaries such as GCC. For example:
> > +
> > +  - ``sparc64-linux-gnu-`` if we have the sparc toolchain installed on
> > +    our system.
> > +
> > +  - ``$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux``
> > +    if we have downloaded the microblaze toolchain from the 0-day
> > +    website to a specified path in our home directory called toolchains.
> > +
> > +- ``--qemu_config``: Specifies the path to a file containing a
> > +  custom qemu architecture definition. This should be a python file
> > +  containing a `QemuArchParams` object.
> > +
> > +- ``--qemu_args``: Specifies additional QEMU arguments, for example, "-smp 8".
> > +
> > +- ``--jobs``: Specifies the number of jobs (commands) to run simultaneously.4
> > +  By default, this is set to the number of cores on your system.
> > +
> > +- ``--timeout``: Specifies the maximum number of seconds allowed for all tests to run.
> > +  This does not include the time taken to build the tests.
> > +
> > +- ``--kernel_args``: Specifies additional kernel command-line arguments. Might be repeated.
>
> Nit: maybe "can be repeated"? As it's the reader of this documentation
> who will likely be doing the repeating. Or "may be repeated"?

+1

[...]
Brendan Higgins July 22, 2022, 12:27 a.m. UTC | #4
On Thu, Jul 21, 2022 at 4:26 AM Sadiya Kazi <sadiyakazi@google.com> wrote:
>
> Run_wrapper.rst was missing some command line arguments. Added
> additional args in the file.
>
> Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>

Aside from the small nits pointed out by Maíra and David, this looks good to me.

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Daniel Latypov July 22, 2022, 12:54 a.m. UTC | #5
On Thu, Jul 21, 2022 at 1:26 AM 'Sadiya Kazi' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> Run_wrapper.rst was missing some command line arguments. Added
> additional args in the file.
>
> Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

Looks good!
A minor suggestion down below to go along with what everyone else has said.

> +- ``--qemu_config``: Specifies the path to a file containing a
> +  custom qemu architecture definition. This should be a python file
> +  containing a `QemuArchParams` object.
> +
> +- ``--qemu_args``: Specifies additional QEMU arguments, for example, "-smp 8".

Minor nit: I think ``-smp 8`` would be a bit better here.
It feels like it would fit what we did with other example arguments.
diff mbox series

Patch

diff --git a/Documentation/dev-tools/kunit/run_wrapper.rst b/Documentation/dev-tools/kunit/run_wrapper.rst
index 5e560f2c5fca..600af7ac5f88 100644
--- a/Documentation/dev-tools/kunit/run_wrapper.rst
+++ b/Documentation/dev-tools/kunit/run_wrapper.rst
@@ -233,7 +233,7 @@  Command-Line Arguments
 ======================
 
 kunit_tool has a number of other command-line arguments which can
-be useful for our test environment. Below the most commonly used
+be useful for our test environment. Below are the most commonly used
 command line arguments:
 
 - ``--help``: Lists all available options. To list common options,
@@ -257,3 +257,61 @@  command line arguments:
             added or modified. Instead, enable all tests
             which have satisfied dependencies by adding
             ``CONFIG_KUNIT_ALL_TESTS=y`` to your ``.kunitconfig``.
+
+- ``--kunitconfig``: Specifies the path or the directory of the ``.kunitconfig``
+  file. For example:
+
+  - ``lib/kunit/.kunitconfig`` can be the path of the file.
+
+  - ``lib/kunit`` can be the directory in which the file is located.
+
+  This file is used to build and run with a predefined set of tests
+  and their dependencies. For example, to run tests for a given subsystem.
+
+- ``--kconfig_add``: Specifies additional configuration options to be
+  appended to the ``.kunitconfig`` file.
+  For example, ``./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y``.
+
+- ``--arch``: Runs tests on the specified architecture. The architecture
+  specified must match the Kbuild ARCH environment variable.
+  For example, i386, x86_64, arm, um, etc. Non-UML architectures run on QEMU.
+  Default is `um`.
+
+- ``--cross_compile``: Specifies the Kbuild toolchain. It passes the
+  same argument as passed to the ``CROSS_COMPILE`` variable used by
+  Kbuild. This will be the prefix for the toolchain
+  binaries such as GCC. For example:
+
+  - ``sparc64-linux-gnu-`` if we have the sparc toolchain installed on
+    our system.
+
+  - ``$HOME/toolchains/microblaze/gcc-9.2.0-nolibc/microblaze-linux/bin/microblaze-linux``
+    if we have downloaded the microblaze toolchain from the 0-day
+    website to a specified path in our home directory called toolchains.
+
+- ``--qemu_config``: Specifies the path to a file containing a
+  custom qemu architecture definition. This should be a python file
+  containing a `QemuArchParams` object.
+
+- ``--qemu_args``: Specifies additional QEMU arguments, for example, "-smp 8".
+
+- ``--jobs``: Specifies the number of jobs (commands) to run simultaneously.
+  By default, this is set to the number of cores on your system.
+
+- ``--timeout``: Specifies the maximum number of seconds allowed for all tests to run.
+  This does not include the time taken to build the tests.
+
+- ``--kernel_args``: Specifies additional kernel command-line arguments. Might be repeated.
+
+- ``--run_isolated``: If set, boots the kernel for each individual suite/test.
+  This is useful for debugging a non-hermetic test, one that
+  might pass/fail based on what ran before it.
+
+- ``--raw_output``: If set, generates unformatted output from kernel. Possible options are:
+
+   - ``all``: To view the full kernel output, use ``--raw_output=all``.
+
+   - ``kunit``: This is the default option and filters to KUnit output. Use ``--raw_output`` or ``--raw_output=kunit``.
+
+- ``--json``: If set, stores the test results in a JSON format and prints to `stdout` or
+  saves to a file if a filename is specified.