rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones
@ 2024-01-22  5:42 Dirk Behme
  2024-01-22 10:28 ` Trevor Gross
  2024-01-22 10:57 ` Miguel Ojeda
  0 siblings, 2 replies; 3+ messages in thread
From: Dirk Behme @ 2024-01-22  5:42 UTC (permalink / raw)
  To: rust-for-linux; +Cc: dirk.behme, Miguel Ojeda

Rust documentation tests are automatically converted into KUnit
tests. The mainline commit adding this feature

commit a66d733da801 ("rust: support running Rust documentation tests as KUnit ones")

from Mingual has a very nice commit message with a lot details
for this. To not 'hide' that just in a commit message pick main
parts of it and add it to the documentation. And add a short info
how to enable this.

Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 Documentation/rust/general-information.rst | 94 ++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/Documentation/rust/general-information.rst b/Documentation/rust/general-information.rst
index 58283f4894ea0..d68460a2fea13 100644
--- a/Documentation/rust/general-information.rst
+++ b/Documentation/rust/general-information.rst
@@ -100,3 +100,97 @@ Additionally, there are the ``#[test]`` tests. These can be run using
 This requires the kernel .config and downloads external repos. It
 runs the ``#[test]`` tests on the host (currently) and thus is fairly
 limited in what these tests can test.
+
+KUnit tests == documentation tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Rust has documentation tests: these are typically examples of
+usage of any item (e.g. function, struct, module...).
+
+They are very convenient because they are just written
+alongside the documentation. For instance::
+
+	/// Sums two numbers.
+	///
+	/// ```
+	/// assert_eq!(mymod::f(10, 20), 30);
+	/// ```
+	pub fn f(a: i32, b: i32) -> i32 {
+	    a + b
+	}
+
+In userspace, the tests are collected and run via `rustdoc`.
+Using the tool as-is would be useful already, since it allows
+to compile-test most tests (thus enforcing they are kept
+in sync with the code they document) and run those that do not
+depend on in-kernel APIs.
+
+However, by transforming the tests into a KUnit test suite,
+they can also be run inside the kernel. Moreover, the tests
+get to be compiled as other Rust kernel objects instead of
+targeting userspace.
+
+On top of that, the integration with KUnit means the Rust
+support gets to reuse the existing testing facilities. For
+instance, the kernel log would look like::
+
+	KTAP version 1
+	1..1
+	    KTAP version 1
+	    # Subtest: rust_doctests_kernel
+	    1..59
+	    # rust_doctest_kernel_build_assert_rs_0.location: rust/kernel/build_assert.rs:13
+	    ok 1 rust_doctest_kernel_build_assert_rs_0
+	    # rust_doctest_kernel_build_assert_rs_1.location: rust/kernel/build_assert.rs:56
+	    ok 2 rust_doctest_kernel_build_assert_rs_1
+	    # rust_doctest_kernel_init_rs_0.location: rust/kernel/init.rs:122
+	    ok 3 rust_doctest_kernel_init_rs_0
+	    ...
+	    # rust_doctest_kernel_types_rs_2.location: rust/kernel/types.rs:150
+	    ok 59 rust_doctest_kernel_types_rs_2
+	# rust_doctests_kernel: pass:59 fail:0 skip:0 total:59
+	# Totals: pass:59 fail:0 skip:0 total:59
+	ok 1 rust_doctests_kernel
+
+Tests using the ``?`` operator are also supported as usual, e.g.::
+
+	/// ```
+	/// # use kernel::{spawn_work_item, workqueue};
+	/// spawn_work_item!(workqueue::system(), || pr_info!("x"))?;
+	/// # Ok::<(), Error>(())
+	/// ```
+
+The tests are also compiled with Clippy under ``CLIPPY=1``, just
+like normal code, thus also benefitting from extra linting.
+
+In order for developers to easily see from which original line
+a failed doctests came from, a KTAP diagnostic line is printed
+to the log, containing the location (file and line) of the
+original test (i.e. instead of the location in the generated
+Rust file)::
+
+	# rust_doctest_kernel_types_rs_2.location: rust/kernel/types.rs:150
+
+A notable difference from KUnit C tests is that the Rust tests
+appear to assert using the usual ``assert!`` and ``assert_eq!``
+macros from the Rust standard library (``core``). We provide
+a custom version that forwards the call to KUnit instead.
+Importantly, these macros do not require passing context,
+unlike the KUnit C ones (i.e. ``struct kunit *``). This makes
+them easier to use, and readers of the documentation do not need
+to care about which testing framework is used. In addition, it
+may allow us to test third-party code more easily in the future.
+
+However, a current limitation is that KUnit does not support
+assertions in other tasks. Thus we presently simply print an
+error to the kernel log if an assertion actually failed.
+
+To use this it is required to enable::
+
+	CONFIG_KUNIT
+	   Kernel hacking -> Kernel Testing and Coverage -> KUnit - Enable support for unit tests
+	CONFIG_RUST_KERNEL_DOCTESTS
+	   Kernel hacking -> Rust hacking -> Doctests for the `kernel` crate
+
+in the kernel config system. Note that these options should
+not be enabled in a production system.
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones
  2024-01-22  5:42 [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones Dirk Behme
@ 2024-01-22 10:28 ` Trevor Gross
  2024-01-22 10:57 ` Miguel Ojeda
  1 sibling, 0 replies; 3+ messages in thread
From: Trevor Gross @ 2024-01-22 10:28 UTC (permalink / raw)
  To: Dirk Behme; +Cc: rust-for-linux, Miguel Ojeda

On Sun, Jan 21, 2024 at 11:42 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Rust documentation tests are automatically converted into KUnit
> tests. The mainline commit adding this feature
>
> commit a66d733da801 ("rust: support running Rust documentation tests as KUnit ones")
>
> from Mingual has a very nice commit message with a lot details

Miguel? :)

> for this. To not 'hide' that just in a commit message pick main
> parts of it and add it to the documentation. And add a short info
> how to enable this.
>
> Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  Documentation/rust/general-information.rst | 94 ++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/Documentation/rust/general-information.rst b/Documentation/rust/general-information.rst
> index 58283f4894ea0..d68460a2fea13 100644
> --- a/Documentation/rust/general-information.rst
> +++ b/Documentation/rust/general-information.rst
> @@ -100,3 +100,97 @@ Additionally, there are the ``#[test]`` tests. These can be run using
>  This requires the kernel .config and downloads external repos. It
>  runs the ``#[test]`` tests on the host (currently) and thus is fairly
>  limited in what these tests can test.
> +
> +KUnit tests == documentation tests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Rust has documentation tests: these are typically examples of
> +usage of any item (e.g. function, struct, module...).
> +
> +They are very convenient because they are just written
> +alongside the documentation. For instance::
> +
> +       /// Sums two numbers.
> +       ///
> +       /// ```
> +       /// assert_eq!(mymod::f(10, 20), 30);
> +       /// ```
> +       pub fn f(a: i32, b: i32) -> i32 {
> +           a + b
> +       }
> +
> +In userspace, the tests are collected and run via `rustdoc`.
> +Using the tool as-is would be useful already, since it allows
> +to compile-test most tests (thus enforcing they are kept
> +in sync with the code they document) and run those that do not
> +depend on in-kernel APIs.

Maybe "to compile-test most tests" -> "verifying that examples
compile" and "and run those" -> ", as well as running those"

> +However, by transforming the tests into a KUnit test suite,
> +they can also be run inside the kernel. Moreover, the tests
> +get to be compiled as other Rust kernel objects instead of
> +targeting userspace.

This starts describing the benefits of being KUnit without first
mentioning that they do run as KUnit. Maybe something like

```
For the kernel, however, these tests get transformed into KUnit test
suites. This means that doctests get compiled as Rust kernel objects,
allowing them to run against a built kernel. Like
```

> +On top of that, the integration with KUnit means the Rust
> +support gets to reuse the existing testing facilities. For
> +instance, the kernel log would look like::

If using the above suggestion, then perhaps the first sentence could become

```
A benefit of this KUnit integration is that Rust doctests get to reuse
existing testing facilities.
```

> +       KTAP version 1
> +       1..1
> +           KTAP version 1
> +           # Subtest: rust_doctests_kernel
> +           1..59
> +           # rust_doctest_kernel_build_assert_rs_0.location: rust/kernel/build_assert.rs:13
> +           ok 1 rust_doctest_kernel_build_assert_rs_0
> +           # rust_doctest_kernel_build_assert_rs_1.location: rust/kernel/build_assert.rs:56
> +           ok 2 rust_doctest_kernel_build_assert_rs_1
> +           # rust_doctest_kernel_init_rs_0.location: rust/kernel/init.rs:122
> +           ok 3 rust_doctest_kernel_init_rs_0
> +           ...
> +           # rust_doctest_kernel_types_rs_2.location: rust/kernel/types.rs:150
> +           ok 59 rust_doctest_kernel_types_rs_2
> +       # rust_doctests_kernel: pass:59 fail:0 skip:0 total:59
> +       # Totals: pass:59 fail:0 skip:0 total:59
> +       ok 1 rust_doctests_kernel
> +
> +Tests using the ``?`` operator are also supported as usual, e.g.::
> +
> +       /// ```
> +       /// # use kernel::{spawn_work_item, workqueue};
> +       /// spawn_work_item!(workqueue::system(), || pr_info!("x"))?;
> +       /// # Ok::<(), Error>(())
> +       /// ```
> +
> +The tests are also compiled with Clippy under ``CLIPPY=1``, just
> +like normal code, thus also benefitting from extra linting.
> +
> +In order for developers to easily see from which original line
> +a failed doctests came from, a KTAP diagnostic line is printed
> +to the log, containing the location (file and line) of the

"from which original line a failed doctests came from" -> "which line
of doctest code caused a failure" or similar

Would split "to the log. This contains..." into two sentences

> +original test (i.e. instead of the location in the generated
> +Rust file)::
> +
> +       # rust_doctest_kernel_types_rs_2.location: rust/kernel/types.rs:150
> +
> +A notable difference from KUnit C tests is that the Rust tests
> +appear to assert using the usual ``assert!`` and ``assert_eq!``
> +macros from the Rust standard library (``core``). We provide

"A notable difference from KUnit C tests is that the" at the beginning
of the sentence could probably be dropped, since it's just talking
about what Rust does here. Or add a sentence about how C does asserts
differently.

> +a custom version that forwards the call to KUnit instead.
> +Importantly, these macros do not require passing context,
> +unlike the KUnit C ones (i.e. ``struct kunit *``). This makes

"the KUnit C ones" -> "those for KUnit testing" or similar

> +them easier to use, and readers of the documentation do not need
> +to care about which testing framework is used. In addition, it
> +may allow us to test third-party code more easily in the future.
> +
> +However, a current limitation is that KUnit does not support

I don't think the "However" is needed here

> +assertions in other tasks. Thus we presently simply print an

Comma after "Thus"

> +error to the kernel log if an assertion actually failed.
> +
> +To use this it is required to enable::

"To use this it is required to enable" -> "To use KUnit doctests, the
following must be enabled"

> +
> +       CONFIG_KUNIT
> +          Kernel hacking -> Kernel Testing and Coverage -> KUnit - Enable support for unit tests
> +       CONFIG_RUST_KERNEL_DOCTESTS
> +          Kernel hacking -> Rust hacking -> Doctests for the `kernel` crate
> +
> +in the kernel config system. Note that these options should
> +not be enabled in a production system.
> --
> 2.28.0

Overall notes:

- Testing probably deserves its own page, this could be a good time to
split it off

- The section above this diff talks about both doctests and #[test]
tests. You could probably put the #[test] info into a new section and
merge the existing doctest info into this patch's new section.

- Regular rustdoc has a surprising caveat that doctests are not run
for nonpublic functions [1]. I am not sure if this limitation applies
in the kernel, but I think it is worth a mention either way.

---

Content looks pretty good, just had some optional wording suggestions.
Thanks, we've needed some more complete information about doctests.

- Trevor

[1]: https://github.com/rust-lang/rust/issues/50784

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones
  2024-01-22  5:42 [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones Dirk Behme
  2024-01-22 10:28 ` Trevor Gross
@ 2024-01-22 10:57 ` Miguel Ojeda
  1 sibling, 0 replies; 3+ messages in thread
From: Miguel Ojeda @ 2024-01-22 10:57 UTC (permalink / raw)
  To: Dirk Behme; +Cc: rust-for-linux, Miguel Ojeda

Hi Dirk,

Thanks for sending this! A few nits below.

On Mon, Jan 22, 2024 at 6:42 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Rust documentation tests are automatically converted into KUnit
> tests. The mainline commit adding this feature

Perhaps remove "mainline", i.e. it is the default.

> commit a66d733da801 ("rust: support running Rust documentation tests as KUnit ones")
>
> from Mingual has a very nice commit message with a lot details

Typo ;)

> for this. To not 'hide' that just in a commit message pick main
> parts of it and add it to the documentation. And add a short info

Perhaps: "...message, pick the main parts and add..."

> Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

`Co-developed-by` needs to go together with `Signed-off-by` (but the
latter cannot be added without permission).

In addition, there are also two ways of using it: you can give main
authorship to yourself or to one of the co-developers. It depends on
what you agree with the others / what is fair. Here, I wrote most of
the text, but I don't mind if you want to keep the main authorship,
especially if you are willing to put the work to improve the text with
the feedback from Trevor etc. :)

> +KUnit tests == documentation tests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> +Rust has documentation tests: these are typically examples of
> +usage of any item (e.g. function, struct, module...).

I think the previous section before this should be re-arranged
somehow, since it already introduces doctests. And it should probably
be moved into its own page like Trevor mentioned. See also my other
comment below.

> +They are very convenient because they are just written
> +alongside the documentation. For instance::
> +
> +       /// Sums two numbers.
> +       ///
> +       /// ```
> +       /// assert_eq!(mymod::f(10, 20), 30);
> +       /// ```
> +       pub fn f(a: i32, b: i32) -> i32 {
> +           a + b
> +       }

Two blocks are missing the `.. code-block:: rust` we use elsewhere.

> +In userspace, the tests are collected and run via `rustdoc`.

Double-quotes?

> +Using the tool as-is would be useful already, since it allows
> +to compile-test most tests (thus enforcing they are kept
> +in sync with the code they document) and run those that do not
> +depend on in-kernel APIs.
> +
> +However, by transforming the tests into a KUnit test suite,
> +they can also be run inside the kernel. Moreover, the tests
> +get to be compiled as other Rust kernel objects instead of
> +targeting userspace.
> +
> +On top of that, the integration with KUnit means the Rust
> +support gets to reuse the existing testing facilities. For
> +instance, the kernel log would look like::

Hmm... I originally wrote this in the context of a commit message, so
some of these paragraphs (and others, e.g. the "A notable difference
from..." below) feel written from that context (i.e. mixing
implementation details).

For instance, here, "Using the tool as-is would be useful already" is
justifying the reason for introducing extra complexity to transform
the tests into KUnit ones, rather than using `rustdoc` as-is. In other
words, it is more about justifying why the changes (i.e. the commit)
were introduced, rather than explaining a user how to use Rust
doctests in the kernel.

So I think some bits here and there could be cut and/or rearranged to
sound more like documentation.

> +Tests using the ``?`` operator are also supported as usual, e.g.::

Since we can use hyperlinks here and since the question mark operator
is a novelty when coming from the C side, it could be nice to link to
some docs about it, e.g. to
https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator

> +To use this it is required to enable::
> +
> +       CONFIG_KUNIT
> +          Kernel hacking -> Kernel Testing and Coverage -> KUnit - Enable support for unit tests
> +       CONFIG_RUST_KERNEL_DOCTESTS
> +          Kernel hacking -> Rust hacking -> Doctests for the `kernel` crate

If we move this into another page, this probably could be in another
"Usage" or similar section (and move the KUnit script parts here,
too), while the other text could be in an "Introduction" section or
similar.

> +in the kernel config system. Note that these options should
> +not be enabled in a production system.

This is true, though I am not sure if we should duplicate it here --
it is in KUnit's docs (though perhaps where it should be is in
`CONFIG_KUNIT`'s description, i.e. that way one has to see it to
enable it even if one does not read the docs).

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-22 10:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-22  5:42 [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones Dirk Behme
2024-01-22 10:28 ` Trevor Gross
2024-01-22 10:57 ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).