From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Dirk Behme <dirk.behme@de.bosch.com>
Cc: rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>
Subject: Re: [PATCH] docs: rust: Add description of Rust documentation test as KUnit ones
Date: Mon, 22 Jan 2024 11:57:39 +0100 [thread overview]
Message-ID: <CANiq72nCtbFt3Tysr6AdUNHdw7QBVS+7dTwck-BD7H_XfF+CEQ@mail.gmail.com> (raw)
In-Reply-To: <20240122054219.779928-1-dirk.behme@de.bosch.com>
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
prev parent reply other threads:[~2024-01-22 10:57 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANiq72nCtbFt3Tysr6AdUNHdw7QBVS+7dTwck-BD7H_XfF+CEQ@mail.gmail.com \
--to=miguel.ojeda.sandonis@gmail.com \
--cc=dirk.behme@de.bosch.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).