Re: [PATCH 5/6] rust: support running Rust documentation tests as KUnit ones

From: Alice Ryhl
Date: Wed Jun 14 2023 - 16:38:21 EST


Miguel Ojeda <ojeda@xxxxxxxxxx> writes:
> 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
> # Doctest from line 13
> ok 1 rust_doctest_kernel_build_assert_rs_0
> # Doctest from line 56
> ok 2 rust_doctest_kernel_build_assert_rs_1
> # Doctest from line 122
> ok 3 rust_doctest_kernel_init_rs_0
> ...
> # Doctest from line 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
>
> Therefore, add support for running Rust documentation tests
> in KUnit. Some other notes about the current implementation
> and support follow.
>
> The transformation is performed by a couple scripts written
> as Rust hostprogs.
>
> 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.
>
> The names of the tests are currently automatically generated.
> This allows to reduce the burden for documentation writers,
> while keeping them fairly stable for bisection. This is an
> improvement over the `rustdoc`-generated names, which include
> the line number; but ideally we would like to get `rustdoc` to
> provide the Rust item path and a number (for multiple examples
> in a single documented Rust item).
>
> 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. In the future, we may be able to use a proper KUnit
> facility to append this sort of information instead.
>
> 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. This
> should be revisited to properly fail the test, perhaps saving
> the context somewhere else, or letting KUnit handle it.
>
> Signed-off-by: Miguel Ojeda <ojeda@xxxxxxxxxx>

(One nit later below.)

Reviewed-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>

> +fn main() {
> + let mut stdin = std::io::stdin().lock();
> + let mut body = String::new();
> + stdin.read_to_string(&mut body).unwrap();
> +
> + // Find the generated function name looking for the inner function inside `main()`.
> + //
> + // The line we are looking for looks like one of the following:
> + //
> + // ```
> + // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_28_0() {
> + // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl core::fmt::Debug> {
> + // ```
> + //
> + // It should be unlikely that doctest code matches such lines (when code is formatted properly).
> + let rustdoc_function_name = body
> + .lines()
> + .find_map(|line| {
> + Some(
> + line.split_once("fn main() {")?
> + .1
> + .split_once("fn ")?
> + .1
> + .split_once("()")?
> + .0,
> + )
> + .filter(|x| x.chars().all(|c| c.is_alphanumeric() || c == '_'))
> + })
> + .expect("No test function found in `rustdoc`'s output.");
> +
> + // Qualify `Result` to avoid the collision with our own `Result` coming from the prelude.
> + let body = body.replace(
> + &format!("{rustdoc_function_name}() -> Result<(), impl core::fmt::Debug> {{"),
> + &format!("{rustdoc_function_name}() -> core::result::Result<(), impl core::fmt::Debug> {{"),
> + );
> +
> + // For tests that get generated with `Result`, like above, `rustdoc` generates an `unwrap()` on
> + // the return value to check there were no returned errors. Instead, we use our assert macro
> + // since we want to just fail the test, not panic the kernel.
> + //
> + // We save the result in a variable so that the failed assertion message looks nicer.
> + let body = body.replace(
> + &format!("}} {rustdoc_function_name}().unwrap() }}"),
> + &format!("}} let test_return_value = {rustdoc_function_name}(); assert!(test_return_value.is_ok()); }}"),
> + );
> +
> + // Figure out a smaller test name based on the generated function name.
> + let name = rustdoc_function_name.split_once("_rust_kernel_").unwrap().1;
> +
> + let path = format!("rust/test/doctests/kernel/{name}");
> +
> + write!(BufWriter::new(File::create(path).unwrap()), "{body}").unwrap();

This could just be

std::fs::write(path, body.as_bytes());

Alice