[PATCH 0/7] Rust KUnit `#[test]` support improvements

[PATCH 0/7] Rust KUnit `#[test]` support improvements

[Miguel Ojeda]

Improvements that build on top of the very basic `#[test]` support merged in
v6.15.

They are fairly minimal changes, but they allow us to map `assert*!`s back to
KUnit, plus to add support for test functions that return `Result`s.

In essence, they get our `#[test]`s essentially on par with the documentation
tests.

I also took the chance to convert some host `#[test]`s we had to KUnit in order
to showcase the feature.

Finally, I added documentation that was lacking from the original submission.

I hope this helps.

Miguel Ojeda (7):
  rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s
  rust: kunit: support checked `-> Result`s in KUnit `#[test]`s
  rust: add `kunit_tests` to the prelude
  rust: str: convert `rusttest` tests into KUnit
  rust: str: take advantage of the `-> Result` support in KUnit
    `#[test]`'s
  Documentation: rust: rename `#[test]`s to "`rusttest` host tests"
  Documentation: rust: testing: add docs on the new KUnit `#[test]`
    tests

 Documentation/rust/testing.rst | 80 ++++++++++++++++++++++++++++++++--
 init/Kconfig                   |  3 ++
 rust/Makefile                  |  3 +-
 rust/kernel/kunit.rs           | 29 ++++++++++--
 rust/kernel/prelude.rs         |  2 +-
 rust/kernel/str.rs             | 76 ++++++++++++++------------------
 rust/macros/helpers.rs         | 16 +++++++
 rust/macros/kunit.rs           | 31 ++++++++++++-
 rust/macros/lib.rs             |  6 ++-
 9 files changed, 191 insertions(+), 55 deletions(-)


base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
--
2.49.0

[PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

[Miguel Ojeda]

The KUnit `#[test]` support that landed recently is very basic and does
not map the `assert*!` macros into KUnit like the doctests do, so they
panic at the moment.

Thus implement the custom mapping in a similar way to doctests, reusing
the infrastructure there.

In Rust 1.88.0, the `file()` method in `Span` may be stable [1]. However,
it was changed recently (from `SourceFile`), so we need to do something
different in previous versions. Thus create a helper for it and use it
to get the path.

With this, a failing test suite like:

    #[kunit_tests(my_test_suite)]
    mod tests {
        use super::*;

        #[test]
        fn my_first_test() {
            assert_eq!(42, 43);
        }

        #[test]
        fn my_second_test() {
            assert!(42 >= 43);
        }
    }

will properly map back to KUnit, printing something like:

    [    1.924325]     KTAP version 1
    [    1.924421]     # Subtest: my_test_suite
    [    1.924506]     # speed: normal
    [    1.924525]     1..2
    [    1.926385]     # my_first_test: ASSERTION FAILED at rust/kernel/lib.rs:251
    [    1.926385]     Expected 42 == 43 to be true, but is false
    [    1.928026]     # my_first_test.speed: normal
    [    1.928075]     not ok 1 my_first_test
    [    1.928723]     # my_second_test: ASSERTION FAILED at rust/kernel/lib.rs:256
    [    1.928723]     Expected 42 >= 43 to be true, but is false
    [    1.929834]     # my_second_test.speed: normal
    [    1.929868]     not ok 2 my_second_test
    [    1.930032] # my_test_suite: pass:0 fail:2 skip:0 total:2
    [    1.930153] # Totals: pass:0 fail:2 skip:0 total

Link: https://github.com/rust-lang/rust/pull/140514 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 init/Kconfig           |  3 +++
 rust/Makefile          |  3 ++-
 rust/kernel/kunit.rs   |  1 -
 rust/macros/helpers.rs | 16 ++++++++++++++++
 rust/macros/kunit.rs   | 28 +++++++++++++++++++++++++++-
 rust/macros/lib.rs     |  4 ++++
 6 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 63f5974b9fa6..5f442c64c47b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
 config RUSTC_HAS_COERCE_POINTEE
 	def_bool RUSTC_VERSION >= 108400
 
+config RUSTC_HAS_SPAN_FILE
+	def_bool RUSTC_VERSION >= 108800
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/Makefile b/rust/Makefile
index 3aca903a7d08..075b38a24997 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -402,7 +402,8 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
 		-Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \
 		--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
 		--crate-type proc-macro \
-		--crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $<
+		--crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) \
+		@$(objtree)/include/generated/rustc_cfg $<
 
 # Procedural macros can only be used with the `rustc` that compiled it.
 $(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 1604fb6a5b1b..2659895d4c5d 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -323,7 +323,6 @@ mod tests {
 
     #[test]
     fn rust_test_kunit_example_test() {
-        #![expect(clippy::eq_op)]
         assert_eq!(1 + 1, 2);
     }
 
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index a3ee27e29a6f..57c3b0f0c194 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -86,3 +86,19 @@ pub(crate) fn function_name(input: TokenStream) -> Option<Ident> {
     }
     None
 }
+
+pub(crate) fn file() -> String {
+    #[cfg(not(CONFIG_RUSTC_HAS_SPAN_FILE))]
+    {
+        proc_macro::Span::call_site()
+            .source_file()
+            .path()
+            .to_string_lossy()
+            .into_owned()
+    }
+
+    #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)]
+    {
+        proc_macro::Span::call_site().file()
+    }
+}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index 4f553ecf40c0..eb4f2afdbe43 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -101,6 +101,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
     // ```
     let mut kunit_macros = "".to_owned();
     let mut test_cases = "".to_owned();
+    let mut assert_macros = "".to_owned();
+    let path = crate::helpers::file();
     for test in &tests {
         let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
         let kunit_wrapper = format!(
@@ -114,6 +116,27 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
             test, kunit_wrapper_fn_name
         )
         .unwrap();
+        writeln!(
+            assert_macros,
+            r#"
+/// Overrides the usual [`assert!`] macro with one that calls KUnit instead.
+#[allow(unused)]
+macro_rules! assert {{
+    ($cond:expr $(,)?) => {{{{
+        kernel::kunit_assert!("{test}", "{path}", 0, $cond);
+    }}}}
+}}
+
+/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead.
+#[allow(unused)]
+macro_rules! assert_eq {{
+    ($left:expr, $right:expr $(,)?) => {{{{
+        kernel::kunit_assert_eq!("{test}", "{path}", 0, $left, $right);
+    }}}}
+}}
+        "#
+        )
+        .unwrap();
     }
 
     writeln!(kunit_macros).unwrap();
@@ -152,7 +175,10 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
         }
     }
 
-    let mut new_body = TokenStream::from_iter(new_body);
+    let body = new_body;
+    let mut new_body = TokenStream::new();
+    new_body.extend::<TokenStream>(assert_macros.parse().unwrap());
+    new_body.extend(body);
     new_body.extend::<TokenStream>(kunit_macros.parse().unwrap());
 
     tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, new_body)));
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 9acaa68c974e..8bd7906276be 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -6,6 +6,10 @@
 // and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is
 // touched by Kconfig when the version string from the compiler changes.
 
+// TODO: check that when Rust 1.88.0 is released, this would be enough:
+// #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
+#![feature(proc_macro_span)]
+
 #[macro_use]
 mod quote;
 mod concat_idents;
-- 
2.49.0

Re: [PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

[Tamir Duberstein]

On Fri, May 2, 2025 at 5:53 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> The KUnit `#[test]` support that landed recently is very basic and does
> not map the `assert*!` macros into KUnit like the doctests do, so they
> panic at the moment.
>
> Thus implement the custom mapping in a similar way to doctests, reusing
> the infrastructure there.
>
> In Rust 1.88.0, the `file()` method in `Span` may be stable [1]. However,
> it was changed recently (from `SourceFile`), so we need to do something
> different in previous versions. Thus create a helper for it and use it
> to get the path.
>
> With this, a failing test suite like:
>
>     #[kunit_tests(my_test_suite)]
>     mod tests {
>         use super::*;
>
>         #[test]
>         fn my_first_test() {
>             assert_eq!(42, 43);
>         }
>
>         #[test]
>         fn my_second_test() {
>             assert!(42 >= 43);
>         }
>     }
>
> will properly map back to KUnit, printing something like:
>
>     [    1.924325]     KTAP version 1
>     [    1.924421]     # Subtest: my_test_suite
>     [    1.924506]     # speed: normal
>     [    1.924525]     1..2
>     [    1.926385]     # my_first_test: ASSERTION FAILED at rust/kernel/lib.rs:251
>     [    1.926385]     Expected 42 == 43 to be true, but is false
>     [    1.928026]     # my_first_test.speed: normal
>     [    1.928075]     not ok 1 my_first_test
>     [    1.928723]     # my_second_test: ASSERTION FAILED at rust/kernel/lib.rs:256
>     [    1.928723]     Expected 42 >= 43 to be true, but is false
>     [    1.929834]     # my_second_test.speed: normal
>     [    1.929868]     not ok 2 my_second_test
>     [    1.930032] # my_test_suite: pass:0 fail:2 skip:0 total:2
>     [    1.930153] # Totals: pass:0 fail:2 skip:0 total
>
> Link: https://github.com/rust-lang/rust/pull/140514 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  init/Kconfig           |  3 +++
>  rust/Makefile          |  3 ++-
>  rust/kernel/kunit.rs   |  1 -
>  rust/macros/helpers.rs | 16 ++++++++++++++++
>  rust/macros/kunit.rs   | 28 +++++++++++++++++++++++++++-
>  rust/macros/lib.rs     |  4 ++++
>  6 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6..5f442c64c47b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>         def_bool RUSTC_VERSION >= 108400
>
> +config RUSTC_HAS_SPAN_FILE
> +       def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>         int
>         default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/Makefile b/rust/Makefile
> index 3aca903a7d08..075b38a24997 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -402,7 +402,8 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>                 -Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \
>                 --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>                 --crate-type proc-macro \
> -               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $<
> +               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) \
> +               @$(objtree)/include/generated/rustc_cfg $<
>
>  # Procedural macros can only be used with the `rustc` that compiled it.
>  $(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 1604fb6a5b1b..2659895d4c5d 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -323,7 +323,6 @@ mod tests {
>
>      #[test]
>      fn rust_test_kunit_example_test() {
> -        #![expect(clippy::eq_op)]

How come this vanished?

>          assert_eq!(1 + 1, 2);
>      }
>
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index a3ee27e29a6f..57c3b0f0c194 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -86,3 +86,19 @@ pub(crate) fn function_name(input: TokenStream) -> Option<Ident> {
>      }
>      None
>  }
> +
> +pub(crate) fn file() -> String {
> +    #[cfg(not(CONFIG_RUSTC_HAS_SPAN_FILE))]
> +    {
> +        proc_macro::Span::call_site()
> +            .source_file()
> +            .path()
> +            .to_string_lossy()
> +            .into_owned()
> +    }
> +
> +    #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)]
> +    {
> +        proc_macro::Span::call_site().file()
> +    }
> +}
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index 4f553ecf40c0..eb4f2afdbe43 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -101,6 +101,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>      // ```
>      let mut kunit_macros = "".to_owned();
>      let mut test_cases = "".to_owned();
> +    let mut assert_macros = "".to_owned();

nit: why not String::new() for all these?

> +    let path = crate::helpers::file();
>      for test in &tests {
>          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
>          let kunit_wrapper = format!(
> @@ -114,6 +116,27 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>              test, kunit_wrapper_fn_name
>          )
>          .unwrap();
> +        writeln!(
> +            assert_macros,
> +            r#"
> +/// Overrides the usual [`assert!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert {{
> +    ($cond:expr $(,)?) => {{{{
> +        kernel::kunit_assert!("{test}", "{path}", 0, $cond);
> +    }}}}
> +}}
> +
> +/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert_eq {{
> +    ($left:expr, $right:expr $(,)?) => {{{{
> +        kernel::kunit_assert_eq!("{test}", "{path}", 0, $left, $right);
> +    }}}}
> +}}
> +        "#
> +        )
> +        .unwrap();
>      }
>
>      writeln!(kunit_macros).unwrap();
> @@ -152,7 +175,10 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>          }
>      }
>
> -    let mut new_body = TokenStream::from_iter(new_body);
> +    let body = new_body;
> +    let mut new_body = TokenStream::new();
> +    new_body.extend::<TokenStream>(assert_macros.parse().unwrap());
> +    new_body.extend(body);

Could we do this (pushing `assert_macros`) before the block above to
avoid this body/new_body name juggling?

>      new_body.extend::<TokenStream>(kunit_macros.parse().unwrap());
>
>      tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, new_body)));
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9acaa68c974e..8bd7906276be 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -6,6 +6,10 @@
>  // and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is
>  // touched by Kconfig when the version string from the compiler changes.
>
> +// TODO: check that when Rust 1.88.0 is released, this would be enough:
> +// #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
> +#![feature(proc_macro_span)]
> +
>  #[macro_use]
>  mod quote;
>  mod concat_idents;
> --
> 2.49.0
>
>

Re: [PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

[Miguel Ojeda]

On Sun, May 4, 2025 at 7:42 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> How come this vanished?

It doesn't lint anymore -- the lint only appears to be intended to
work with the standard `assert_eq!` macro (and related ones), if I am
reading its source code correctly.

I created an issue upstream and linked it into our Clippy metalist,
similar to the custom `dbg!` request:

    https://github.com/rust-lang/rust-clippy/issues/14903

> nit: why not String::new() for all these?

I prefer that too, but I kept it consistent with the other lines. We
could put that as a "good first issue" unless someone gives a reason
to prefer other methods.

> Could we do this (pushing `assert_macros`) before the block above to
> avoid this body/new_body name juggling?

We can use a new variable, changing the line below (i.e. it is clear
then that we are "assembling the final body") -- I did that.

Moving the new variable then is also possible, but I think it makes it
a bit harder to see the three "main parts" that we assemble into the
final body.

But if you have a better approach or I misunderstood, please of course
feel free to send a patch (or maybe a "good first issue", since that
would be a good one I think).

Thanks!

Cheers,
Miguel

Re: [PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 7366 bytes --]

On Sat, 3 May 2025 at 05:51, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> The KUnit `#[test]` support that landed recently is very basic and does
> not map the `assert*!` macros into KUnit like the doctests do, so they
> panic at the moment.
>
> Thus implement the custom mapping in a similar way to doctests, reusing
> the infrastructure there.
>
> In Rust 1.88.0, the `file()` method in `Span` may be stable [1]. However,
> it was changed recently (from `SourceFile`), so we need to do something
> different in previous versions. Thus create a helper for it and use it
> to get the path.
>
> With this, a failing test suite like:
>
>     #[kunit_tests(my_test_suite)]
>     mod tests {
>         use super::*;
>
>         #[test]
>         fn my_first_test() {
>             assert_eq!(42, 43);
>         }
>
>         #[test]
>         fn my_second_test() {
>             assert!(42 >= 43);
>         }
>     }
>
> will properly map back to KUnit, printing something like:
>
>     [    1.924325]     KTAP version 1
>     [    1.924421]     # Subtest: my_test_suite
>     [    1.924506]     # speed: normal
>     [    1.924525]     1..2
>     [    1.926385]     # my_first_test: ASSERTION FAILED at rust/kernel/lib.rs:251
>     [    1.926385]     Expected 42 == 43 to be true, but is false
>     [    1.928026]     # my_first_test.speed: normal
>     [    1.928075]     not ok 1 my_first_test
>     [    1.928723]     # my_second_test: ASSERTION FAILED at rust/kernel/lib.rs:256
>     [    1.928723]     Expected 42 >= 43 to be true, but is false
>     [    1.929834]     # my_second_test.speed: normal
>     [    1.929868]     not ok 2 my_second_test
>     [    1.930032] # my_test_suite: pass:0 fail:2 skip:0 total:2
>     [    1.930153] # Totals: pass:0 fail:2 skip:0 total
>
> Link: https://github.com/rust-lang/rust/pull/140514 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

Nice! While I do think there may still be some cases where we might
want to use KUnit-specific macros in the future (particularly if we
have more complex, multithreaded test contexts), this is definitely
better for most cases.

I also managed to test it against the 1.88 nightly, and the message is
looking good.

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

Cheers,
-- David


>  init/Kconfig           |  3 +++
>  rust/Makefile          |  3 ++-
>  rust/kernel/kunit.rs   |  1 -
>  rust/macros/helpers.rs | 16 ++++++++++++++++
>  rust/macros/kunit.rs   | 28 +++++++++++++++++++++++++++-
>  rust/macros/lib.rs     |  4 ++++
>  6 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6..5f442c64c47b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>         def_bool RUSTC_VERSION >= 108400
>
> +config RUSTC_HAS_SPAN_FILE
> +       def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>         int
>         default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/Makefile b/rust/Makefile
> index 3aca903a7d08..075b38a24997 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -402,7 +402,8 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>                 -Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \
>                 --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>                 --crate-type proc-macro \
> -               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $<
> +               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) \
> +               @$(objtree)/include/generated/rustc_cfg $<
>
>  # Procedural macros can only be used with the `rustc` that compiled it.
>  $(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 1604fb6a5b1b..2659895d4c5d 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -323,7 +323,6 @@ mod tests {
>
>      #[test]
>      fn rust_test_kunit_example_test() {
> -        #![expect(clippy::eq_op)]
>          assert_eq!(1 + 1, 2);
>      }
>
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index a3ee27e29a6f..57c3b0f0c194 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -86,3 +86,19 @@ pub(crate) fn function_name(input: TokenStream) -> Option<Ident> {
>      }
>      None
>  }
> +
> +pub(crate) fn file() -> String {
> +    #[cfg(not(CONFIG_RUSTC_HAS_SPAN_FILE))]
> +    {
> +        proc_macro::Span::call_site()
> +            .source_file()
> +            .path()
> +            .to_string_lossy()
> +            .into_owned()
> +    }
> +
> +    #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)]
> +    {
> +        proc_macro::Span::call_site().file()
> +    }
> +}
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index 4f553ecf40c0..eb4f2afdbe43 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -101,6 +101,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>      // ```
>      let mut kunit_macros = "".to_owned();
>      let mut test_cases = "".to_owned();
> +    let mut assert_macros = "".to_owned();
> +    let path = crate::helpers::file();
>      for test in &tests {
>          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
>          let kunit_wrapper = format!(
> @@ -114,6 +116,27 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>              test, kunit_wrapper_fn_name
>          )
>          .unwrap();
> +        writeln!(
> +            assert_macros,
> +            r#"
> +/// Overrides the usual [`assert!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert {{
> +    ($cond:expr $(,)?) => {{{{
> +        kernel::kunit_assert!("{test}", "{path}", 0, $cond);
> +    }}}}
> +}}
> +
> +/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert_eq {{
> +    ($left:expr, $right:expr $(,)?) => {{{{
> +        kernel::kunit_assert_eq!("{test}", "{path}", 0, $left, $right);
> +    }}}}
> +}}
> +        "#
> +        )
> +        .unwrap();
>      }
>
>      writeln!(kunit_macros).unwrap();
> @@ -152,7 +175,10 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>          }
>      }
>
> -    let mut new_body = TokenStream::from_iter(new_body);
> +    let body = new_body;
> +    let mut new_body = TokenStream::new();
> +    new_body.extend::<TokenStream>(assert_macros.parse().unwrap());
> +    new_body.extend(body);
>      new_body.extend::<TokenStream>(kunit_macros.parse().unwrap());
>
>      tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, new_body)));
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9acaa68c974e..8bd7906276be 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -6,6 +6,10 @@
>  // and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is
>  // touched by Kconfig when the version string from the compiler changes.
>
> +// TODO: check that when Rust 1.88.0 is released, this would be enough:
> +// #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
> +#![feature(proc_macro_span)]
> +
>  #[macro_use]
>  mod quote;
>  mod concat_idents;
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

[Miguel Ojeda]

On Mon, May 5, 2025 at 8:02 AM David Gow <davidgow@google.com> wrote:
>
> Nice! While I do think there may still be some cases where we might
> want to use KUnit-specific macros in the future (particularly if we
> have more complex, multithreaded test contexts), this is definitely
> better for most cases.
>
> I also managed to test it against the 1.88 nightly, and the message is
> looking good.
>
> Reviewed-by: David Gow <davidgow@google.com>

Thanks for all the feedback and for testing it, David! Very much appreciated.

Given you were happy with this first version already and that others
want to use it soon, I did a few final minor tweaks and I am applying
it.

Cheers,
Miguel

[PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[Miguel Ojeda]

Currently, return values of KUnit `#[test]` functions are ignored.

Thus introduce support for `-> Result` functions by checking their
returned values.

At the same time, require that test functions return `()` or `Result<T,
E>`, which should avoid mistakes, especially with non-`#[must_use]`
types. Other types can be supported in the future if needed.

With this, a failing test like:

    #[test]
    fn my_test() -> Result {
        f()?;
        Ok(())
    }

will output:

    [    3.744214]     KTAP version 1
    [    3.744287]     # Subtest: my_test_suite
    [    3.744378]     # speed: normal
    [    3.744399]     1..1
    [    3.745817]     # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321
    [    3.745817]     Expected is_test_result_ok(my_test()) to be true, but is false
    [    3.747152]     # my_test.speed: normal
    [    3.747199]     not ok 1 my_test
    [    3.747345] not ok 4 my_test_suite

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/kunit.rs | 25 +++++++++++++++++++++++++
 rust/macros/kunit.rs |  3 ++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 2659895d4c5d..f43e3ed460c2 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -164,6 +164,31 @@ macro_rules! kunit_assert_eq {
     }};
 }
 
+trait TestResult {
+    fn is_test_result_ok(&self) -> bool;
+}
+
+impl TestResult for () {
+    fn is_test_result_ok(&self) -> bool {
+        true
+    }
+}
+
+impl<T, E> TestResult for Result<T, E> {
+    fn is_test_result_ok(&self) -> bool {
+        self.is_ok()
+    }
+}
+
+/// Returns whether a test result is to be considered OK.
+///
+/// This will be `assert!`ed from the generated tests.
+#[doc(hidden)]
+#[expect(private_bounds)]
+pub fn is_test_result_ok(t: impl TestResult) -> bool {
+    t.is_test_result_ok()
+}
+
 /// Represents an individual test case.
 ///
 /// The [`kunit_unsafe_test_suite!`] macro expects a NULL-terminated list of valid test cases.
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index eb4f2afdbe43..9681775d160a 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -105,8 +105,9 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
     let path = crate::helpers::file();
     for test in &tests {
         let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
+        // An extra `use` is used here to reduce the length of the message.
         let kunit_wrapper = format!(
-            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
+            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ use kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({}())); }}",
             kunit_wrapper_fn_name, test
         );
         writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
-- 
2.49.0

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[Tamir Duberstein]

On Fri, May 2, 2025 at 5:53 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Currently, return values of KUnit `#[test]` functions are ignored.
>
> Thus introduce support for `-> Result` functions by checking their
> returned values.
>
> At the same time, require that test functions return `()` or `Result<T,
> E>`, which should avoid mistakes, especially with non-`#[must_use]`
> types. Other types can be supported in the future if needed.

Why not restrict this to Result<(), E>?

>
> With this, a failing test like:
>
>     #[test]
>     fn my_test() -> Result {
>         f()?;
>         Ok(())
>     }
>
> will output:
>
>     [    3.744214]     KTAP version 1
>     [    3.744287]     # Subtest: my_test_suite
>     [    3.744378]     # speed: normal
>     [    3.744399]     1..1
>     [    3.745817]     # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321
>     [    3.745817]     Expected is_test_result_ok(my_test()) to be true, but is false

Is it possible to include the error in the output?

>     [    3.747152]     # my_test.speed: normal
>     [    3.747199]     not ok 1 my_test
>     [    3.747345] not ok 4 my_test_suite
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/kunit.rs | 25 +++++++++++++++++++++++++
>  rust/macros/kunit.rs |  3 ++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 2659895d4c5d..f43e3ed460c2 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -164,6 +164,31 @@ macro_rules! kunit_assert_eq {
>      }};
>  }
>
> +trait TestResult {
> +    fn is_test_result_ok(&self) -> bool;
> +}
> +
> +impl TestResult for () {
> +    fn is_test_result_ok(&self) -> bool {
> +        true
> +    }
> +}
> +
> +impl<T, E> TestResult for Result<T, E> {
> +    fn is_test_result_ok(&self) -> bool {
> +        self.is_ok()
> +    }
> +}
> +
> +/// Returns whether a test result is to be considered OK.
> +///
> +/// This will be `assert!`ed from the generated tests.
> +#[doc(hidden)]
> +#[expect(private_bounds)]
> +pub fn is_test_result_ok(t: impl TestResult) -> bool {
> +    t.is_test_result_ok()
> +}
> +
>  /// Represents an individual test case.
>  ///
>  /// The [`kunit_unsafe_test_suite!`] macro expects a NULL-terminated list of valid test cases.
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index eb4f2afdbe43..9681775d160a 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -105,8 +105,9 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>      let path = crate::helpers::file();
>      for test in &tests {
>          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
> +        // An extra `use` is used here to reduce the length of the message.
>          let kunit_wrapper = format!(
> -            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
> +            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ use kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({}())); }}",
>              kunit_wrapper_fn_name, test
>          );
>          writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
> --
> 2.49.0
>
>

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[Miguel Ojeda]

On Sun, May 4, 2025 at 7:34 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Why not restrict this to Result<(), E>?

I guess it is an option -- not sure if there may be a use case.

> Is it possible to include the error in the output?

I thought about giving some more context somehow and perhaps printing
it "manually" in the log, possibly in a KUnit `# ...`. David can
probably suggest the "proper" way.

Thanks!

Cheers,
Miguel

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

On Mon, 5 May 2025 at 02:00, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 7:34 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Why not restrict this to Result<(), E>?
>
> I guess it is an option -- not sure if there may be a use case.
>
> > Is it possible to include the error in the output?
>
> I thought about giving some more context somehow and perhaps printing
> it "manually" in the log, possibly in a KUnit `# ...`. David can
> probably suggest the "proper" way.

Yeah, writing it to the log is fine: probably the best way of handling
this would be to have a kunit assertion macro for "assert_is_ok!()",
which prints the error nicely.
We could just use the existing kernel::kunit::err() function (which
could use some improvement, but is a good start), or even add a proper
assertion formatter which handles rust types via %pA.

That being said, there's no guarantee that the Err branch of a
Result<> can be printed: so there'd need to be some magic to handle
both the case where (e.g.) Err derives Debug, and the case where it
doesn't (in which case, we're basically stuck with what we've got
here: "expected is_ok()" or similar.

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3338 bytes --]

On Sat, 3 May 2025 at 05:51, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Currently, return values of KUnit `#[test]` functions are ignored.
>
> Thus introduce support for `-> Result` functions by checking their
> returned values.
>
> At the same time, require that test functions return `()` or `Result<T,
> E>`, which should avoid mistakes, especially with non-`#[must_use]`
> types. Other types can be supported in the future if needed.
>
> With this, a failing test like:
>
>     #[test]
>     fn my_test() -> Result {
>         f()?;
>         Ok(())
>     }
>
> will output:
>
>     [    3.744214]     KTAP version 1
>     [    3.744287]     # Subtest: my_test_suite
>     [    3.744378]     # speed: normal
>     [    3.744399]     1..1
>     [    3.745817]     # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321
>     [    3.745817]     Expected is_test_result_ok(my_test()) to be true, but is false
>     [    3.747152]     # my_test.speed: normal
>     [    3.747199]     not ok 1 my_test
>     [    3.747345] not ok 4 my_test_suite
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

This is a neat idea. I think a lot of tests will still want to go with
the () return value, but having both as an option seems like a good
idea to me.

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

Cheers,
-- David


>  rust/kernel/kunit.rs | 25 +++++++++++++++++++++++++
>  rust/macros/kunit.rs |  3 ++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 2659895d4c5d..f43e3ed460c2 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -164,6 +164,31 @@ macro_rules! kunit_assert_eq {
>      }};
>  }
>
> +trait TestResult {
> +    fn is_test_result_ok(&self) -> bool;
> +}
> +
> +impl TestResult for () {
> +    fn is_test_result_ok(&self) -> bool {
> +        true
> +    }
> +}
> +
> +impl<T, E> TestResult for Result<T, E> {
> +    fn is_test_result_ok(&self) -> bool {
> +        self.is_ok()
> +    }
> +}
> +
> +/// Returns whether a test result is to be considered OK.
> +///
> +/// This will be `assert!`ed from the generated tests.
> +#[doc(hidden)]
> +#[expect(private_bounds)]
> +pub fn is_test_result_ok(t: impl TestResult) -> bool {
> +    t.is_test_result_ok()
> +}
> +
>  /// Represents an individual test case.
>  ///
>  /// The [`kunit_unsafe_test_suite!`] macro expects a NULL-terminated list of valid test cases.
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index eb4f2afdbe43..9681775d160a 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -105,8 +105,9 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>      let path = crate::helpers::file();
>      for test in &tests {
>          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
> +        // An extra `use` is used here to reduce the length of the message.
>          let kunit_wrapper = format!(
> -            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
> +            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ use kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({}())); }}",
>              kunit_wrapper_fn_name, test
>          );
>          writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[Boqun Feng]

On Mon, May 05, 2025 at 02:02:09PM +0800, David Gow wrote:
> On Sat, 3 May 2025 at 05:51, Miguel Ojeda <ojeda@kernel.org> wrote:
> >
> > Currently, return values of KUnit `#[test]` functions are ignored.
> >
> > Thus introduce support for `-> Result` functions by checking their
> > returned values.
> >
> > At the same time, require that test functions return `()` or `Result<T,
> > E>`, which should avoid mistakes, especially with non-`#[must_use]`
> > types. Other types can be supported in the future if needed.
> >
> > With this, a failing test like:
> >
> >     #[test]
> >     fn my_test() -> Result {
> >         f()?;
> >         Ok(())
> >     }
> >
> > will output:
> >
> >     [    3.744214]     KTAP version 1
> >     [    3.744287]     # Subtest: my_test_suite
> >     [    3.744378]     # speed: normal
> >     [    3.744399]     1..1
> >     [    3.745817]     # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321
> >     [    3.745817]     Expected is_test_result_ok(my_test()) to be true, but is false
> >     [    3.747152]     # my_test.speed: normal
> >     [    3.747199]     not ok 1 my_test
> >     [    3.747345] not ok 4 my_test_suite
> >
> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> > ---
> 
> This is a neat idea. I think a lot of tests will still want to go with
> the () return value, but having both as an option seems like a good
> idea to me.
> 

Handling the return value of a test is definitely a good thing, but I'm
not sure returning `Err` should be treated as assertion failure
though. Considering:

    #[test]
    fn foo() -> Result {
        let b = KBox::new(...)?; // need to allocate memory for the test
	use_box(b);
	assert!(...);
    }

if the test runs without enough memory, then KBox::new() would return a
Err(ENOMEM), and technically, it's not a test failure rather the test
has been skipped because of no enough resource.

I would suggest we handle the `Err` returns specially (mark as "SKIPPED"
maybe?).

Thoughts?

Regards,
Boqun

> Reviewed-by: David Gow <davidgow@google.com>
> 
> Cheers,
> -- David
> 
> 
> >  rust/kernel/kunit.rs | 25 +++++++++++++++++++++++++
> >  rust/macros/kunit.rs |  3 ++-
> >  2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> > index 2659895d4c5d..f43e3ed460c2 100644
> > --- a/rust/kernel/kunit.rs
> > +++ b/rust/kernel/kunit.rs
> > @@ -164,6 +164,31 @@ macro_rules! kunit_assert_eq {
> >      }};
> >  }
> >
> > +trait TestResult {
> > +    fn is_test_result_ok(&self) -> bool;
> > +}
> > +
> > +impl TestResult for () {
> > +    fn is_test_result_ok(&self) -> bool {
> > +        true
> > +    }
> > +}
> > +
> > +impl<T, E> TestResult for Result<T, E> {
> > +    fn is_test_result_ok(&self) -> bool {
> > +        self.is_ok()
> > +    }
> > +}
> > +
> > +/// Returns whether a test result is to be considered OK.
> > +///
> > +/// This will be `assert!`ed from the generated tests.
> > +#[doc(hidden)]
> > +#[expect(private_bounds)]
> > +pub fn is_test_result_ok(t: impl TestResult) -> bool {
> > +    t.is_test_result_ok()
> > +}
> > +
> >  /// Represents an individual test case.
> >  ///
> >  /// The [`kunit_unsafe_test_suite!`] macro expects a NULL-terminated list of valid test cases.
> > diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> > index eb4f2afdbe43..9681775d160a 100644
> > --- a/rust/macros/kunit.rs
> > +++ b/rust/macros/kunit.rs
> > @@ -105,8 +105,9 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
> >      let path = crate::helpers::file();
> >      for test in &tests {
> >          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
> > +        // An extra `use` is used here to reduce the length of the message.
> >          let kunit_wrapper = format!(
> > -            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
> > +            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ use kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({}())); }}",
> >              kunit_wrapper_fn_name, test
> >          );
> >          writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
> > --
> > 2.49.0
> >

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]

On Tue, 6 May 2025 at 03:34, Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, May 05, 2025 at 02:02:09PM +0800, David Gow wrote:
> > On Sat, 3 May 2025 at 05:51, Miguel Ojeda <ojeda@kernel.org> wrote:
> > >
> > > Currently, return values of KUnit `#[test]` functions are ignored.
> > >
> > > Thus introduce support for `-> Result` functions by checking their
> > > returned values.
> > >
> > > At the same time, require that test functions return `()` or `Result<T,
> > > E>`, which should avoid mistakes, especially with non-`#[must_use]`
> > > types. Other types can be supported in the future if needed.
> > >
> > > With this, a failing test like:
> > >
> > >     #[test]
> > >     fn my_test() -> Result {
> > >         f()?;
> > >         Ok(())
> > >     }
> > >
> > > will output:
> > >
> > >     [    3.744214]     KTAP version 1
> > >     [    3.744287]     # Subtest: my_test_suite
> > >     [    3.744378]     # speed: normal
> > >     [    3.744399]     1..1
> > >     [    3.745817]     # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321
> > >     [    3.745817]     Expected is_test_result_ok(my_test()) to be true, but is false
> > >     [    3.747152]     # my_test.speed: normal
> > >     [    3.747199]     not ok 1 my_test
> > >     [    3.747345] not ok 4 my_test_suite
> > >
> > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> > > ---
> >
> > This is a neat idea. I think a lot of tests will still want to go with
> > the () return value, but having both as an option seems like a good
> > idea to me.
> >
>
> Handling the return value of a test is definitely a good thing, but I'm
> not sure returning `Err` should be treated as assertion failure
> though. Considering:
>
>     #[test]
>     fn foo() -> Result {
>         let b = KBox::new(...)?; // need to allocate memory for the test
>         use_box(b);
>         assert!(...);
>     }
>
> if the test runs without enough memory, then KBox::new() would return a
> Err(ENOMEM), and technically, it's not a test failure rather the test
> has been skipped because of no enough resource.
>
> I would suggest we handle the `Err` returns specially (mark as "SKIPPED"
> maybe?).
>
> Thoughts?
>
> Regards,
> Boqun
>

FWIW, having out-of-memory situations trigger a test failure is
consistent with what other KUnit tests (written in C) do.

There's both advantages and disadvantages to this: on the one hand,
it's prone to false positives (as you mention), on the other, it
catches cases where the test is using an unusually large amount of
memory (which could indeed be a test issues).

My go-to rule is that tests should fail if small allocations (which,
in the normal course of events, _should_ succeed) fail, but if they
have unusual resource requirements (beyond "enough memory for the
system to run normally") these should be checked separately when the
test starts.

That being said, I definitely think that, by default, an `Err` return
should map to a FAILED test result, as not all Err Results are a
resource exhaustion issue, and we definitely don't want to mark a test
as skipped if there's a real error occurring. If test authors wish to
skip a test when an out-of-memory condition occurs, they probably
should handle that explicitly. (But I'd not be opposed to helpers to
make it easier.)

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

[Miguel Ojeda]

On Tue, May 6, 2025 at 8:33 AM David Gow <davidgow@google.com> wrote:
>
> FWIW, having out-of-memory situations trigger a test failure is
> consistent with what other KUnit tests (written in C) do.
>
> There's both advantages and disadvantages to this: on the one hand,
> it's prone to false positives (as you mention), on the other, it
> catches cases where the test is using an unusually large amount of
> memory (which could indeed be a test issues).
>
> My go-to rule is that tests should fail if small allocations (which,
> in the normal course of events, _should_ succeed) fail, but if they
> have unusual resource requirements (beyond "enough memory for the
> system to run normally") these should be checked separately when the
> test starts.
>
> That being said, I definitely think that, by default, an `Err` return
> should map to a FAILED test result, as not all Err Results are a
> resource exhaustion issue, and we definitely don't want to mark a test
> as skipped if there's a real error occurring. If test authors wish to
> skip a test when an out-of-memory condition occurs, they probably
> should handle that explicitly. (But I'd not be opposed to helpers to
> make it easier.)

Yeah, agreed.

There may be value in differentiating errors coming from `?` vs.
`assert!`s -- i.e. the discussion in the other thread. It could even
be a different error state if that was valuable -- though I think over
complicating is not great either.

By the way, before I forget to write this down: I talked to Alice
about this back when this discussion happened and she suggested having
a `Result` that carries extra information -- the location. I was
worried about the added cost of doing that in all cases (i.e. for
every `Result` in the kernel), so I suggested doing that but opt-in,
i.e. with a Kconfig option that would be in e.g. the debug menu. Then
people could typically run their debug kernels and things like KUnit
with it enabled, but production kernels without it. Then later on, if
it proves useful for other things, we could try to figure out ways to
do it without too much cost.

Cheers,
Miguel

[PATCH 3/7] rust: add `kunit_tests` to the prelude

[Miguel Ojeda]

It is convenient to have certain things in the `kernel` prelude, and
means kernel developers will find it even easier to start writing tests.

And, anyway, nobody should need to use this identifier for anything else.

Thus add it to the prelude.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/kunit.rs   | 3 +--
 rust/kernel/prelude.rs | 2 +-
 rust/macros/lib.rs     | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index f43e3ed460c2..053a7da147d5 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -6,6 +6,7 @@
 //!
 //! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>
 
+use crate::prelude::*;
 use core::{ffi::c_void, fmt};
 
 /// Prints a KUnit error-level message.
@@ -40,8 +41,6 @@ pub fn info(args: fmt::Arguments<'_>) {
     }
 }
 
-use macros::kunit_tests;
-
 /// Asserts that a boolean expression is `true` at runtime.
 ///
 /// Public but hidden since it should only be used from generated tests.
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index baa774a351ce..e5d61a83952f 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -17,7 +17,7 @@
 pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec};
 
 #[doc(no_inline)]
-pub use macros::{export, module, vtable};
+pub use macros::{export, kunit_tests, module, vtable};
 
 pub use pin_init::{init, pin_data, pin_init, pinned_drop, InPlaceWrite, Init, PinInit, Zeroable};
 
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 8bd7906276be..8b8d46e759d4 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -406,7 +406,7 @@ pub fn paste(input: TokenStream) -> TokenStream {
 /// # Examples
 ///
 /// ```ignore
-/// # use macros::kunit_tests;
+/// # use kernel::prelude::*;
 /// #[kunit_tests(kunit_test_suit_name)]
 /// mod tests {
 ///     #[test]
-- 
2.49.0

Re: [PATCH 3/7] rust: add `kunit_tests` to the prelude

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2191 bytes --]

On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> It is convenient to have certain things in the `kernel` prelude, and
> means kernel developers will find it even easier to start writing tests.
>
> And, anyway, nobody should need to use this identifier for anything else.
>
> Thus add it to the prelude.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

Definitely a fan of this here, thanks!

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

Cheers,
-- David


>  rust/kernel/kunit.rs   | 3 +--
>  rust/kernel/prelude.rs | 2 +-
>  rust/macros/lib.rs     | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index f43e3ed460c2..053a7da147d5 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -6,6 +6,7 @@
>  //!
>  //! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>
>
> +use crate::prelude::*;
>  use core::{ffi::c_void, fmt};
>
>  /// Prints a KUnit error-level message.
> @@ -40,8 +41,6 @@ pub fn info(args: fmt::Arguments<'_>) {
>      }
>  }
>
> -use macros::kunit_tests;
> -
>  /// Asserts that a boolean expression is `true` at runtime.
>  ///
>  /// Public but hidden since it should only be used from generated tests.
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index baa774a351ce..e5d61a83952f 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -17,7 +17,7 @@
>  pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec};
>
>  #[doc(no_inline)]
> -pub use macros::{export, module, vtable};
> +pub use macros::{export, kunit_tests, module, vtable};
>
>  pub use pin_init::{init, pin_data, pin_init, pinned_drop, InPlaceWrite, Init, PinInit, Zeroable};
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 8bd7906276be..8b8d46e759d4 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -406,7 +406,7 @@ pub fn paste(input: TokenStream) -> TokenStream {
>  /// # Examples
>  ///
>  /// ```ignore
> -/// # use macros::kunit_tests;
> +/// # use kernel::prelude::*;
>  /// #[kunit_tests(kunit_test_suit_name)]
>  /// mod tests {
>  ///     #[test]
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

[PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[Miguel Ojeda]

In general, we should aim to test as much as possible within the actual
kernel, and not in the build host.

Thus convert these `rusttest` tests into KUnit tests.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/str.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 878111cb77bc..cf2caa2db168 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -6,7 +6,7 @@
 use core::fmt::{self, Write};
 use core::ops::{self, Deref, DerefMut, Index};
 
-use crate::error::{code::*, Error};
+use crate::prelude::*;
 
 /// Byte string without UTF-8 validity guarantee.
 #[repr(transparent)]
@@ -572,8 +572,7 @@ macro_rules! c_str {
     }};
 }
 
-#[cfg(test)]
-#[expect(clippy::items_after_test_module)]
+#[kunit_tests(rust_kernel_str)]
 mod tests {
     use super::*;
 
@@ -622,11 +621,10 @@ fn test_cstr_to_str() {
     }
 
     #[test]
-    #[should_panic]
-    fn test_cstr_to_str_panic() {
+    fn test_cstr_to_str_invalid_utf8() {
         let bad_bytes = b"\xc3\x28\0";
         let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
-        checked_cstr.to_str().unwrap();
+        assert!(checked_cstr.to_str().is_err());
     }
 
     #[test]
-- 
2.49.0

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[Tamir Duberstein]

On Fri, May 2, 2025 at 5:53 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In general, we should aim to test as much as possible within the actual
> kernel, and not in the build host.

Is that true? The build host is often easier to work with. There's a
number of host tests on the C side that exist precisely for this
reason.

> Thus convert these `rusttest` tests into KUnit tests.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/str.rs | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 878111cb77bc..cf2caa2db168 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -6,7 +6,7 @@
>  use core::fmt::{self, Write};
>  use core::ops::{self, Deref, DerefMut, Index};
>
> -use crate::error::{code::*, Error};
> +use crate::prelude::*;
>
>  /// Byte string without UTF-8 validity guarantee.
>  #[repr(transparent)]
> @@ -572,8 +572,7 @@ macro_rules! c_str {
>      }};
>  }
>
> -#[cfg(test)]
> -#[expect(clippy::items_after_test_module)]
> +#[kunit_tests(rust_kernel_str)]
>  mod tests {
>      use super::*;
>
> @@ -622,11 +621,10 @@ fn test_cstr_to_str() {
>      }
>
>      #[test]
> -    #[should_panic]
> -    fn test_cstr_to_str_panic() {
> +    fn test_cstr_to_str_invalid_utf8() {
>          let bad_bytes = b"\xc3\x28\0";
>          let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> -        checked_cstr.to_str().unwrap();
> +        assert!(checked_cstr.to_str().is_err());
>      }
>
>      #[test]
> --
> 2.49.0
>
>

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[Miguel Ojeda]

On Sun, May 4, 2025 at 7:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Is that true? The build host is often easier to work with. There's a
> number of host tests on the C side that exist precisely for this
> reason.

Even for tests that could run in the host (pure functions), if you
test in the host, then you are not testing the actual kernel code, in
the sense of same compile flags, target, etc.

Moreover, you have UML, which gives you access to other APIs.

As for "easier to work with", I am not sure what you mean -- KUnit
does not really require anything special w.r.t. building the kernel
normally. In a way, these restricted host tests actually are an extra
hassle, in that you have to deal with yet another test environment and
special restrictions.

But which host tests are you referring to?

Thanks for reviewing!

Cheers,
Miguel

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[Tamir Duberstein]

On Sun, May 4, 2025 at 2:31 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 7:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Is that true? The build host is often easier to work with. There's a
> > number of host tests on the C side that exist precisely for this
> > reason.
>
> Even for tests that could run in the host (pure functions), if you
> test in the host, then you are not testing the actual kernel code, in
> the sense of same compile flags, target, etc.
>
> Moreover, you have UML, which gives you access to other APIs.
>
> As for "easier to work with", I am not sure what you mean -- KUnit
> does not really require anything special w.r.t. building the kernel
> normally. In a way, these restricted host tests actually are an extra
> hassle, in that you have to deal with yet another test environment and
> special restrictions.

All good points.

> But which host tests are you referring to?

One example is https://github.com/torvalds/linux/blob/59c9ab3e8cc7f56cd65608f6e938b5ae96eb9cd2/tools/testing/radix-tree/xarray.c.

It might be that these are necessary because the xarray tests don't
use kunit, and so are pretty inconvenient to run. As you might have
guessed, I discovered these host tests when my patch porting the
xarray tests to kunit broke the host-side build :(

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[Miguel Ojeda]

On Sun, May 4, 2025 at 8:39 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> One example is https://github.com/torvalds/linux/blob/59c9ab3e8cc7f56cd65608f6e938b5ae96eb9cd2/tools/testing/radix-tree/xarray.c.
>
> It might be that these are necessary because the xarray tests don't
> use kunit, and so are pretty inconvenient to run. As you might have
> guessed, I discovered these host tests when my patch porting the
> xarray tests to kunit broke the host-side build :(

It can be useful to have some tests as independent userspace things
(i.e. outside KUnit-UML) to use other tooling on it, but I think for
such cases we would want to have a way to use the tests from userspace
without having to remove them from being KUnit tests too, since we
definitely want to test them in the actual kernel too.

David et al. can probably tell us more context, e.g. I may be missing
some plans on their side here. For instance, for Rust, we wanted to
eventually have a way to tag stuff as kernel vs. host etc., but that
is longer term.

Cheers,
Miguel

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

On Mon, 5 May 2025 at 03:02, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 8:39 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > One example is https://github.com/torvalds/linux/blob/59c9ab3e8cc7f56cd65608f6e938b5ae96eb9cd2/tools/testing/radix-tree/xarray.c.
> >
> > It might be that these are necessary because the xarray tests don't
> > use kunit, and so are pretty inconvenient to run. As you might have
> > guessed, I discovered these host tests when my patch porting the
> > xarray tests to kunit broke the host-side build :(
>
> It can be useful to have some tests as independent userspace things
> (i.e. outside KUnit-UML) to use other tooling on it, but I think for
> such cases we would want to have a way to use the tests from userspace
> without having to remove them from being KUnit tests too, since we
> definitely want to test them in the actual kernel too.
>
> David et al. can probably tell us more context, e.g. I may be missing
> some plans on their side here. For instance, for Rust, we wanted to
> eventually have a way to tag stuff as kernel vs. host etc., but that
> is longer term.

Yeah, this ultimately boils down to a combination of which tradeoffs
are best for a given test, and personal preference.

KUnit definitely has the advantage of being more a more "accurate"
test in a kernel environment — particularly if you're cross-compiling
— but is a bit slower and more bloated (due to having the whole
kernel), and therefore a bit more difficult to debug.

In an ideal world, most tests would be able to be compiled either as a
host-side, standalone test or as a KUnit test. There are some (sadly
lower-priority) plans to support this more with C code by having a
standalone implementation of the KUnit API, and things like the
automatic conversion of, e.g., assert!() macros into their KUnit
equivalent could help if we wanted to try something similar on the
Rust side.

But, as you point out, that sort of tagging of tests is really a
longer-term goal.

In the meantime, my strategy has been to encourage KUnit for new
tests, or ports where it's not going to break existing workflows too
much, but ultimately to defer to the maintainer of the tests / code
being tested if they've got strong opinions. (Of course, I am biased
in KUnit's favour. :-))



-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In general, we should aim to test as much as possible within the actual
> kernel, and not in the build host.
>
> Thus convert these `rusttest` tests into KUnit tests.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

I'm obviously a fan of this. Assuming no-one working on the str code
strenuously objects, let's do it!

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

Cheers,
-- David


>  rust/kernel/str.rs | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 878111cb77bc..cf2caa2db168 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -6,7 +6,7 @@
>  use core::fmt::{self, Write};
>  use core::ops::{self, Deref, DerefMut, Index};
>
> -use crate::error::{code::*, Error};
> +use crate::prelude::*;
>
>  /// Byte string without UTF-8 validity guarantee.
>  #[repr(transparent)]
> @@ -572,8 +572,7 @@ macro_rules! c_str {
>      }};
>  }
>
> -#[cfg(test)]
> -#[expect(clippy::items_after_test_module)]
> +#[kunit_tests(rust_kernel_str)]
>  mod tests {
>      use super::*;
>
> @@ -622,11 +621,10 @@ fn test_cstr_to_str() {
>      }
>
>      #[test]
> -    #[should_panic]
> -    fn test_cstr_to_str_panic() {
> +    fn test_cstr_to_str_invalid_utf8() {
>          let bad_bytes = b"\xc3\x28\0";
>          let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> -        checked_cstr.to_str().unwrap();
> +        assert!(checked_cstr.to_str().is_err());
>      }
>
>      #[test]
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 4/7] rust: str: convert `rusttest` tests into KUnit

[John Hubbard]

On 5/2/25 2:51 PM, Miguel Ojeda wrote:
> In general, we should aim to test as much as possible within the actual
> kernel, and not in the build host.
> 
> Thus convert these `rusttest` tests into KUnit tests.

yes yes yes! :)

Like many, many kernel developers, I've been using separate development
and test machines, and so "make test" approaches are broken by
design.

In other works, launching tests from make(1) usually includes the
fatal flaw of leaving all the dependencies connected. And so when
you try to run on your test machine, it tries to rebuild, but the
kernel was build on the development machine...

It's just a constraint that should not be imposed on developers.

thanks,
-- 
John Hubbard

> 
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/str.rs | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 878111cb77bc..cf2caa2db168 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -6,7 +6,7 @@
>  use core::fmt::{self, Write};
>  use core::ops::{self, Deref, DerefMut, Index};
>  
> -use crate::error::{code::*, Error};
> +use crate::prelude::*;
>  
>  /// Byte string without UTF-8 validity guarantee.
>  #[repr(transparent)]
> @@ -572,8 +572,7 @@ macro_rules! c_str {
>      }};
>  }
>  
> -#[cfg(test)]
> -#[expect(clippy::items_after_test_module)]
> +#[kunit_tests(rust_kernel_str)]
>  mod tests {
>      use super::*;
>  
> @@ -622,11 +621,10 @@ fn test_cstr_to_str() {
>      }
>  
>      #[test]
> -    #[should_panic]
> -    fn test_cstr_to_str_panic() {
> +    fn test_cstr_to_str_invalid_utf8() {
>          let bad_bytes = b"\xc3\x28\0";
>          let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> -        checked_cstr.to_str().unwrap();
> +        assert!(checked_cstr.to_str().is_err());
>      }
>  
>      #[test]

[PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[Miguel Ojeda]

Since now we have support for returning `-> Result`s, we can convert some
of these tests to use the feature, and serve as a first user for it too.

Thus convert them.

This, in turn, simplifies them a fair bit.

We keep the actual assertions we want to make as explicit ones with
`assert*!`s.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/str.rs | 68 ++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index cf2caa2db168..8dcfb11013f2 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -576,25 +576,9 @@ macro_rules! c_str {
 mod tests {
     use super::*;
 
-    struct String(CString);
-
-    impl String {
-        fn from_fmt(args: fmt::Arguments<'_>) -> Self {
-            String(CString::try_from_fmt(args).unwrap())
-        }
-    }
-
-    impl Deref for String {
-        type Target = str;
-
-        fn deref(&self) -> &str {
-            self.0.to_str().unwrap()
-        }
-    }
-
     macro_rules! format {
         ($($f:tt)*) => ({
-            &*String::from_fmt(kernel::fmt!($($f)*))
+            CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()?
         })
     }
 
@@ -613,66 +597,72 @@ macro_rules! format {
         \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff";
 
     #[test]
-    fn test_cstr_to_str() {
+    fn test_cstr_to_str() -> Result {
         let good_bytes = b"\xf0\x9f\xa6\x80\0";
-        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
-        let checked_str = checked_cstr.to_str().unwrap();
+        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
+        let checked_str = checked_cstr.to_str()?;
         assert_eq!(checked_str, "🦀");
+        Ok(())
     }
 
     #[test]
-    fn test_cstr_to_str_invalid_utf8() {
+    fn test_cstr_to_str_invalid_utf8() -> Result {
         let bad_bytes = b"\xc3\x28\0";
-        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
+        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?;
         assert!(checked_cstr.to_str().is_err());
+        Ok(())
     }
 
     #[test]
-    fn test_cstr_as_str_unchecked() {
+    fn test_cstr_as_str_unchecked() -> Result {
         let good_bytes = b"\xf0\x9f\x90\xA7\0";
-        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
+        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
         // SAFETY: The contents come from a string literal which contains valid UTF-8.
         let unchecked_str = unsafe { checked_cstr.as_str_unchecked() };
         assert_eq!(unchecked_str, "🐧");
+        Ok(())
     }
 
     #[test]
-    fn test_cstr_display() {
-        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
+    fn test_cstr_display() -> Result {
+        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
         assert_eq!(format!("{}", hello_world), "hello, world!");
-        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
+        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
         assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
-        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
+        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
         assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
-        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
+        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
         assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+        Ok(())
     }
 
     #[test]
-    fn test_cstr_display_all_bytes() {
+    fn test_cstr_display_all_bytes() -> Result {
         let mut bytes: [u8; 256] = [0; 256];
         // fill `bytes` with [1..=255] + [0]
         for i in u8::MIN..=u8::MAX {
             bytes[i as usize] = i.wrapping_add(1);
         }
-        let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
+        let cstr = CStr::from_bytes_with_nul(&bytes)?;
         assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
+        Ok(())
     }
 
     #[test]
-    fn test_cstr_debug() {
-        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
+    fn test_cstr_debug() -> Result {
+        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
         assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
-        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
+        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
         assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\"");
-        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
+        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
         assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
-        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
+        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
         assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
+        Ok(())
     }
 
     #[test]
-    fn test_bstr_display() {
+    fn test_bstr_display() -> Result {
         let hello_world = BStr::from_bytes(b"hello, world!");
         assert_eq!(format!("{}", hello_world), "hello, world!");
         let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
@@ -683,10 +673,11 @@ fn test_bstr_display() {
         assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
         let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
         assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+        Ok(())
     }
 
     #[test]
-    fn test_bstr_debug() {
+    fn test_bstr_debug() -> Result {
         let hello_world = BStr::from_bytes(b"hello, world!");
         assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
         let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
@@ -697,6 +688,7 @@ fn test_bstr_debug() {
         assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
         let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
         assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
+        Ok(())
     }
 }
 
-- 
2.49.0

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[Tamir Duberstein]

On Fri, May 2, 2025 at 5:54 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Since now we have support for returning `-> Result`s, we can convert some
> of these tests to use the feature, and serve as a first user for it too.
>
> Thus convert them.
>
> This, in turn, simplifies them a fair bit.
>
> We keep the actual assertions we want to make as explicit ones with
> `assert*!`s.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Alice pointed this out in another thread but: one of the downsides of
returning Result is that in the event of failure the line number where
the error occurred is no longer contained in the test output. I'm 👎
on this change for that reason.

> ---
>  rust/kernel/str.rs | 68 ++++++++++++++++++++--------------------------
>  1 file changed, 30 insertions(+), 38 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index cf2caa2db168..8dcfb11013f2 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -576,25 +576,9 @@ macro_rules! c_str {
>  mod tests {
>      use super::*;
>
> -    struct String(CString);
> -
> -    impl String {
> -        fn from_fmt(args: fmt::Arguments<'_>) -> Self {
> -            String(CString::try_from_fmt(args).unwrap())
> -        }
> -    }
> -
> -    impl Deref for String {
> -        type Target = str;
> -
> -        fn deref(&self) -> &str {
> -            self.0.to_str().unwrap()
> -        }
> -    }

These changes don't depend on returning `Result` from the tests
AFAICT. Can they be in a separate patch?

> -
>      macro_rules! format {
>          ($($f:tt)*) => ({
> -            &*String::from_fmt(kernel::fmt!($($f)*))
> +            CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()?
>          })
>      }
>
> @@ -613,66 +597,72 @@ macro_rules! format {
>          \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff";
>
>      #[test]
> -    fn test_cstr_to_str() {
> +    fn test_cstr_to_str() -> Result {
>          let good_bytes = b"\xf0\x9f\xa6\x80\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> -        let checked_str = checked_cstr.to_str().unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
> +        let checked_str = checked_cstr.to_str()?;
>          assert_eq!(checked_str, "🦀");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_to_str_invalid_utf8() {
> +    fn test_cstr_to_str_invalid_utf8() -> Result {
>          let bad_bytes = b"\xc3\x28\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?;
>          assert!(checked_cstr.to_str().is_err());
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_as_str_unchecked() {
> +    fn test_cstr_as_str_unchecked() -> Result {
>          let good_bytes = b"\xf0\x9f\x90\xA7\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
>          // SAFETY: The contents come from a string literal which contains valid UTF-8.
>          let unchecked_str = unsafe { checked_cstr.as_str_unchecked() };
>          assert_eq!(unchecked_str, "🐧");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_display() {
> -        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> +    fn test_cstr_display() -> Result {
> +        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
>          assert_eq!(format!("{}", hello_world), "hello, world!");
> -        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> +        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
>          assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
> -        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> +        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
>          assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
> -        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> +        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
>          assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_display_all_bytes() {
> +    fn test_cstr_display_all_bytes() -> Result {
>          let mut bytes: [u8; 256] = [0; 256];
>          // fill `bytes` with [1..=255] + [0]
>          for i in u8::MIN..=u8::MAX {
>              bytes[i as usize] = i.wrapping_add(1);
>          }
> -        let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
> +        let cstr = CStr::from_bytes_with_nul(&bytes)?;
>          assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_debug() {
> -        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> +    fn test_cstr_debug() -> Result {
> +        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
>          assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
> -        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> +        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
>          assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\"");
> -        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> +        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
>          assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
> -        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> +        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
>          assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_bstr_display() {
> +    fn test_bstr_display() -> Result {
>          let hello_world = BStr::from_bytes(b"hello, world!");
>          assert_eq!(format!("{}", hello_world), "hello, world!");
>          let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
> @@ -683,10 +673,11 @@ fn test_bstr_display() {
>          assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
>          let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
>          assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_bstr_debug() {
> +    fn test_bstr_debug() -> Result {
>          let hello_world = BStr::from_bytes(b"hello, world!");
>          assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
>          let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
> @@ -697,6 +688,7 @@ fn test_bstr_debug() {
>          assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
>          let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
>          assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
> +        Ok(())
>      }
>  }
>
> --
> 2.49.0
>
>

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[Miguel Ojeda]

On Sun, May 4, 2025 at 7:30 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Alice pointed this out in another thread but: one of the downsides of
> returning Result is that in the event of failure the line number where
> the error occurred is no longer contained in the test output. I'm 👎
> on this change for that reason.

We could perhaps customize `?` to help here, e.g. printing a trace or
panic, with the `Try` trait or similar.

Related to this: I thought about saying in the guidelines that `?` in
tests is intended for things that you would normally use `?` in
similar kernel code, i.e. things that the test is not "testing",
rather than things that you would want to assert explicitly. Thus the
actual code under test should still have `assert!`s in the right
places. I did that in the sample. That way, having `?` would still
simplify a lot of test code and yet allow us to differentiate between
code under test vs. other code failing.

> These changes don't depend on returning `Result` from the tests
> AFAICT. Can they be in a separate patch?

Not sure what you mean. The change below uses `?`, which is what
allows this to be removed.

Thanks!

Cheers,
Miguel

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[Tamir Duberstein]

On Sun, May 4, 2025 at 2:15 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 7:30 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Alice pointed this out in another thread but: one of the downsides of
> > returning Result is that in the event of failure the line number where
> > the error occurred is no longer contained in the test output. I'm 👎
> > on this change for that reason.
>
> We could perhaps customize `?` to help here, e.g. printing a trace or
> panic, with the `Try` trait or similar.
>
> Related to this: I thought about saying in the guidelines that `?` in
> tests is intended for things that you would normally use `?` in
> similar kernel code, i.e. things that the test is not "testing",
> rather than things that you would want to assert explicitly. Thus the
> actual code under test should still have `assert!`s in the right
> places. I did that in the sample. That way, having `?` would still
> simplify a lot of test code and yet allow us to differentiate between
> code under test vs. other code failing.

I see. Up to you, obviously, but ISTM that this degree of freedom is
unnecessary, but perhaps there's a benefit I'm underappreciating?

>
> > These changes don't depend on returning `Result` from the tests
> > AFAICT. Can they be in a separate patch?
>
> Not sure what you mean. The change below uses `?`, which is what
> allows this to be removed.

Even without this change, couldn't you apply

     macro_rules! format {
         ($($f:tt)*) => ({
-            &*String::from_fmt(kernel::fmt!($($f)*))
+            CString::try_from_fmt(kernel::fmt!($($f)*)).unwrap().to_str().unwrap()
         })
     }

and achieve roughly the same reduction in line count in the test module?

Cheers.
Tamir

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[Miguel Ojeda]

On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> I see. Up to you, obviously, but ISTM that this degree of freedom is
> unnecessary, but perhaps there's a benefit I'm underappreciating?

Well, having this allows one to write code like the rest of the kernel
code, instead of, say, `.unwrap()`ing or `assert!`ing everywhere.

So easier to read, easier to copy-paste from normal code, and people
(especially those learning) wouldn't get accustomed to seeing
`.unwrap()`s etc. everywhere.

Having said that, C KUnit uses the macros for things that require
stopping the test, even if "unrelated" to the actual test, and it does
not look like normal code, of course. They do have `->init()` which
can return a failure, but not the test cases themselves.

David perhaps has some advice here. Perhaps test functions being
fallible (like returning `int`) were considered (or asserts for
"unrelated" things) for C at some point and discarded.

The custom `?` is quite tempting, to get the best of both worlds,
assuming we could make it work well.

> Even without this change, couldn't you apply
>
>      macro_rules! format {
>          ($($f:tt)*) => ({
> -            &*String::from_fmt(kernel::fmt!($($f)*))
> +            CString::try_from_fmt(kernel::fmt!($($f)*)).unwrap().to_str().unwrap()
>          })
>      }
>
> and achieve roughly the same reduction in line count in the test module?

Sure, the line would need to change again later, but that is fine too,
we can do a separate commit.

Cheers,
Miguel

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

On Mon, 5 May 2025 at 05:54, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > I see. Up to you, obviously, but ISTM that this degree of freedom is
> > unnecessary, but perhaps there's a benefit I'm underappreciating?
>
> Well, having this allows one to write code like the rest of the kernel
> code, instead of, say, `.unwrap()`ing or `assert!`ing everywhere.
>
> So easier to read, easier to copy-paste from normal code, and people
> (especially those learning) wouldn't get accustomed to seeing
> `.unwrap()`s etc. everywhere.
>
> Having said that, C KUnit uses the macros for things that require
> stopping the test, even if "unrelated" to the actual test, and it does
> not look like normal code, of course. They do have `->init()` which
> can return a failure, but not the test cases themselves.
>
> David perhaps has some advice here. Perhaps test functions being
> fallible (like returning `int`) were considered (or asserts for
> "unrelated" things) for C at some point and discarded.

The decision to not have a return value for tests predates me (if
Brendan's around, maybe he recalls the original reason here), but I
suspect it was mostly in order to encourage explicit assertions, which
could then contain the line number of the failure.

We use the KUnit assertions for "unrelated" failures as well mainly as
that's the only way to end a test early if it's not entirely contained
in one function. But it's not _wrong_ for a test to mark itself as
failed (or skipped) and then return early if that makes more sense.

> The custom `?` is quite tempting, to get the best of both worlds,
> assuming we could make it work well.

I'm definitely a fan of using '?' over unwrap() here, if only because
it won't trigger an actual panic. That being said, if it turns out to
be easier to have a variant of unwrap(), that'd work, too.

The fact that not all Result Errs implement Debug makes having a nice
way of printing it a bit more fun, too.


-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 7163 bytes --]

On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Since now we have support for returning `-> Result`s, we can convert some
> of these tests to use the feature, and serve as a first user for it too.
>
> Thus convert them.
>
> This, in turn, simplifies them a fair bit.
>
> We keep the actual assertions we want to make as explicit ones with
> `assert*!`s.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

I prefer this, even though I actually don't mind the .unwrap(), just
because we'll funnel failures to KUnit properly, rather than having a
full panic from the unwrap.

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

Cheers,
-- David


>  rust/kernel/str.rs | 68 ++++++++++++++++++++--------------------------
>  1 file changed, 30 insertions(+), 38 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index cf2caa2db168..8dcfb11013f2 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -576,25 +576,9 @@ macro_rules! c_str {
>  mod tests {
>      use super::*;
>
> -    struct String(CString);
> -
> -    impl String {
> -        fn from_fmt(args: fmt::Arguments<'_>) -> Self {
> -            String(CString::try_from_fmt(args).unwrap())
> -        }
> -    }
> -
> -    impl Deref for String {
> -        type Target = str;
> -
> -        fn deref(&self) -> &str {
> -            self.0.to_str().unwrap()
> -        }
> -    }
> -
>      macro_rules! format {
>          ($($f:tt)*) => ({
> -            &*String::from_fmt(kernel::fmt!($($f)*))
> +            CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()?
>          })
>      }
>
> @@ -613,66 +597,72 @@ macro_rules! format {
>          \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff";
>
>      #[test]
> -    fn test_cstr_to_str() {
> +    fn test_cstr_to_str() -> Result {
>          let good_bytes = b"\xf0\x9f\xa6\x80\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> -        let checked_str = checked_cstr.to_str().unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
> +        let checked_str = checked_cstr.to_str()?;
>          assert_eq!(checked_str, "🦀");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_to_str_invalid_utf8() {
> +    fn test_cstr_to_str_invalid_utf8() -> Result {
>          let bad_bytes = b"\xc3\x28\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?;
>          assert!(checked_cstr.to_str().is_err());
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_as_str_unchecked() {
> +    fn test_cstr_as_str_unchecked() -> Result {
>          let good_bytes = b"\xf0\x9f\x90\xA7\0";
> -        let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> +        let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
>          // SAFETY: The contents come from a string literal which contains valid UTF-8.
>          let unchecked_str = unsafe { checked_cstr.as_str_unchecked() };
>          assert_eq!(unchecked_str, "🐧");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_display() {
> -        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> +    fn test_cstr_display() -> Result {
> +        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
>          assert_eq!(format!("{}", hello_world), "hello, world!");
> -        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> +        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
>          assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
> -        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> +        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
>          assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
> -        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> +        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
>          assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_display_all_bytes() {
> +    fn test_cstr_display_all_bytes() -> Result {
>          let mut bytes: [u8; 256] = [0; 256];
>          // fill `bytes` with [1..=255] + [0]
>          for i in u8::MIN..=u8::MAX {
>              bytes[i as usize] = i.wrapping_add(1);
>          }
> -        let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
> +        let cstr = CStr::from_bytes_with_nul(&bytes)?;
>          assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_cstr_debug() {
> -        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> +    fn test_cstr_debug() -> Result {
> +        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
>          assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
> -        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> +        let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
>          assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\"");
> -        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> +        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
>          assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
> -        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> +        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
>          assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_bstr_display() {
> +    fn test_bstr_display() -> Result {
>          let hello_world = BStr::from_bytes(b"hello, world!");
>          assert_eq!(format!("{}", hello_world), "hello, world!");
>          let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
> @@ -683,10 +673,11 @@ fn test_bstr_display() {
>          assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
>          let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
>          assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
> +        Ok(())
>      }
>
>      #[test]
> -    fn test_bstr_debug() {
> +    fn test_bstr_debug() -> Result {
>          let hello_world = BStr::from_bytes(b"hello, world!");
>          assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
>          let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
> @@ -697,6 +688,7 @@ fn test_bstr_debug() {
>          assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
>          let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
>          assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
> +        Ok(())
>      }
>  }
>
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

[PATCH 6/7] Documentation: rust: rename `#[test]`s to "`rusttest` host tests"

[Miguel Ojeda]

Now that `rusttest`s are not really used much, clarify the section of
the documentation that describes them.

In addition, free the section name for the KUnit-based `#[test]`s that
will be added afterwards. To do so, rename the section into `rusttest`
host tests.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/testing.rst | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst
index f692494f7b74..6337b83815ab 100644
--- a/Documentation/rust/testing.rst
+++ b/Documentation/rust/testing.rst
@@ -130,16 +130,17 @@ please see:
 
 	https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
 
-The ``#[test]`` tests
----------------------
+The ``rusttest`` host tests
+---------------------------
 
-Additionally, there are the ``#[test]`` tests. These can be run using the
-``rusttest`` Make target::
+These are userspace tests that can be built and run in the host (i.e. the one
+that performs the kernel build) using the ``rusttest`` Make target::
 
 	make LLVM=1 rusttest
 
-This requires the kernel ``.config``. It runs the ``#[test]`` tests on the host
-(currently) and thus is fairly limited in what these tests can test.
+This requires the kernel ``.config``.
+
+Currently, they are mostly used for testing the ``macros`` crate's examples.
 
 The Kselftests
 --------------
-- 
2.49.0

Re: [PATCH 6/7] Documentation: rust: rename `#[test]`s to "`rusttest` host tests"

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Now that `rusttest`s are not really used much, clarify the section of
> the documentation that describes them.
>
> In addition, free the section name for the KUnit-based `#[test]`s that
> will be added afterwards. To do so, rename the section into `rusttest`
> host tests.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

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

Cheers,
-- David


>  Documentation/rust/testing.rst | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst
> index f692494f7b74..6337b83815ab 100644
> --- a/Documentation/rust/testing.rst
> +++ b/Documentation/rust/testing.rst
> @@ -130,16 +130,17 @@ please see:
>
>         https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
>
> -The ``#[test]`` tests
> ----------------------
> +The ``rusttest`` host tests
> +---------------------------
>
> -Additionally, there are the ``#[test]`` tests. These can be run using the
> -``rusttest`` Make target::
> +These are userspace tests that can be built and run in the host (i.e. the one
> +that performs the kernel build) using the ``rusttest`` Make target::
>
>         make LLVM=1 rusttest
>
> -This requires the kernel ``.config``. It runs the ``#[test]`` tests on the host
> -(currently) and thus is fairly limited in what these tests can test.
> +This requires the kernel ``.config``.
> +
> +Currently, they are mostly used for testing the ``macros`` crate's examples.
>
>  The Kselftests
>  --------------
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

[PATCH 7/7] Documentation: rust: testing: add docs on the new KUnit `#[test]` tests

[Miguel Ojeda]

There was no documentation yet on the KUnit-based `#[test]`s.

Thus add it now.

It includes an explanation about the `assert*!` macros being mapped to
KUnit and the support for `-> Result` introduced in these series.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/testing.rst | 71 ++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst
index 6337b83815ab..f43cb77bcc69 100644
--- a/Documentation/rust/testing.rst
+++ b/Documentation/rust/testing.rst
@@ -130,6 +130,77 @@ please see:
 
 	https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
 
+The ``#[test]`` tests
+---------------------
+
+Additionally, there are the ``#[test]`` tests. Like for documentation tests,
+these are also fairly similar to what you would expect from userspace, and they
+are also mapped to KUnit.
+
+These tests are introduced by the ``kunit_tests`` procedural macro, which takes
+the name of the test suite as an argument.
+
+For instance, assume we want to test the function ``f`` from the documentation
+tests section. We could write, in the same file where we have our function:
+
+.. code-block:: rust
+
+	#[kunit_tests(rust_kernel_mymod)]
+	mod tests {
+	    use super::*;
+
+	    #[test]
+	    fn test_f() {
+	        assert_eq!(f(10, 20), 30);
+	    }
+	}
+
+And if we run it, the kernel log would look like::
+
+	    KTAP version 1
+	    # Subtest: rust_kernel_mymod
+	    # speed: normal
+	    1..1
+	    # test_f.speed: normal
+	    ok 1 test_f
+	ok 1 rust_kernel_mymod
+
+Like documentation tests, the ``assert!`` and ``assert_eq!`` macros are mapped
+back to KUnit and do not panic. Similarly, the
+`? <https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator>`_
+operator is supported, i.e. the test functions may return either nothing (i.e.
+the unit type ``()``) or ``Result`` (i.e. any ``Result<T, E>``). For instance:
+
+.. code-block:: rust
+
+	#[kunit_tests(rust_kernel_mymod)]
+	mod tests {
+	    use super::*;
+
+	    #[test]
+	    fn test_g() -> Result {
+	        let x = g()?;
+	        assert_eq!(x, 30);
+	        Ok(())
+	    }
+	}
+
+If we run the test and the call to ``g`` fails, then the kernel log would show::
+
+	    KTAP version 1
+	    # Subtest: rust_kernel_mymod
+	    # speed: normal
+	    1..1
+	    # test_g: ASSERTION FAILED at rust/kernel/lib.rs:335
+	    Expected is_test_result_ok(test_g()) to be true, but is false
+	    # test_g.speed: normal
+	    not ok 1 test_g
+	not ok 1 rust_kernel_mymod
+
+If a ``#[test]`` test could be useful as an example for the user, then please
+use a documentation test instead. Even edge cases of an API, e.g. error or
+boundary cases, can be interesting to show in examples.
+
 The ``rusttest`` host tests
 ---------------------------
 
-- 
2.49.0

Re: [PATCH 7/7] Documentation: rust: testing: add docs on the new KUnit `#[test]` tests

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3812 bytes --]

On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote:
>
> There was no documentation yet on the KUnit-based `#[test]`s.
>
> Thus add it now.
>
> It includes an explanation about the `assert*!` macros being mapped to
> KUnit and the support for `-> Result` introduced in these series.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

Assuming all of the other changes go through, this looks good to me.

It _may_ be useful to add some notes about when to choose KUnit tests
versus rusttest host tests: particularly around cross-compiling and/or
the need to call kernel APIs / access global kernel state. But some of
that is covered in the general kernel testing / KUnit documentation in
Documentation/dev-tools, anyway.

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

Cheers,
-- David



>  Documentation/rust/testing.rst | 71 ++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>
> diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst
> index 6337b83815ab..f43cb77bcc69 100644
> --- a/Documentation/rust/testing.rst
> +++ b/Documentation/rust/testing.rst
> @@ -130,6 +130,77 @@ please see:
>
>         https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
>
> +The ``#[test]`` tests
> +---------------------
> +
> +Additionally, there are the ``#[test]`` tests. Like for documentation tests,
> +these are also fairly similar to what you would expect from userspace, and they
> +are also mapped to KUnit.
> +
> +These tests are introduced by the ``kunit_tests`` procedural macro, which takes
> +the name of the test suite as an argument.
> +
> +For instance, assume we want to test the function ``f`` from the documentation
> +tests section. We could write, in the same file where we have our function:
> +
> +.. code-block:: rust
> +
> +       #[kunit_tests(rust_kernel_mymod)]
> +       mod tests {
> +           use super::*;
> +
> +           #[test]
> +           fn test_f() {
> +               assert_eq!(f(10, 20), 30);
> +           }
> +       }
> +
> +And if we run it, the kernel log would look like::
> +
> +           KTAP version 1
> +           # Subtest: rust_kernel_mymod
> +           # speed: normal
> +           1..1
> +           # test_f.speed: normal
> +           ok 1 test_f
> +       ok 1 rust_kernel_mymod
> +
> +Like documentation tests, the ``assert!`` and ``assert_eq!`` macros are mapped
> +back to KUnit and do not panic. Similarly, the
> +`? <https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator>`_
> +operator is supported, i.e. the test functions may return either nothing (i.e.
> +the unit type ``()``) or ``Result`` (i.e. any ``Result<T, E>``). For instance:
> +
> +.. code-block:: rust
> +
> +       #[kunit_tests(rust_kernel_mymod)]
> +       mod tests {
> +           use super::*;
> +
> +           #[test]
> +           fn test_g() -> Result {
> +               let x = g()?;
> +               assert_eq!(x, 30);
> +               Ok(())
> +           }
> +       }
> +
> +If we run the test and the call to ``g`` fails, then the kernel log would show::
> +
> +           KTAP version 1
> +           # Subtest: rust_kernel_mymod
> +           # speed: normal
> +           1..1
> +           # test_g: ASSERTION FAILED at rust/kernel/lib.rs:335
> +           Expected is_test_result_ok(test_g()) to be true, but is false
> +           # test_g.speed: normal
> +           not ok 1 test_g
> +       not ok 1 rust_kernel_mymod
> +
> +If a ``#[test]`` test could be useful as an example for the user, then please
> +use a documentation test instead. Even edge cases of an API, e.g. error or
> +boundary cases, can be interesting to show in examples.
> +
>  The ``rusttest`` host tests
>  ---------------------------
>
> --
> 2.49.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 0/7] Rust KUnit `#[test]` support improvements

[Danilo Krummrich]

On Fri, May 02, 2025 at 11:51:25PM +0200, Miguel Ojeda wrote:
> Improvements that build on top of the very basic `#[test]` support merged in
> v6.15.
> 
> They are fairly minimal changes, but they allow us to map `assert*!`s back to
> KUnit, plus to add support for test functions that return `Result`s.
> 
> In essence, they get our `#[test]`s essentially on par with the documentation
> tests.
> 
> I also took the chance to convert some host `#[test]`s we had to KUnit in order
> to showcase the feature.
> 
> Finally, I added documentation that was lacking from the original submission.
> 
> I hope this helps.

It does -- thanks for this series!

	Acked-by: Danilo Krummrich <dakr@kernel.org>

>   rust: str: convert `rusttest` tests into KUnit

With that, do we still expose `alloc` primitives to userspace tests?

Re: [PATCH 0/7] Rust KUnit `#[test]` support improvements

[Miguel Ojeda]

On Mon, May 5, 2025 at 6:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> With that, do we still expose `alloc` primitives to userspace tests?

I considered removing a bunch of stuff (even the build support for
non-`macros` `rusttest`, to be honest) -- you are referring to the
`any(test, testlib)` bits, right?

I think we can wait to see if we need it, or we can also just remove
it and re-introduce later if needed.

Thanks for taking a look!

Cheers,
Miguel

Re: [PATCH 0/7] Rust KUnit `#[test]` support improvements

[Miguel Ojeda]

On Fri, May 2, 2025 at 11:51 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Improvements that build on top of the very basic `#[test]` support merged in
> v6.15.
>
> They are fairly minimal changes, but they allow us to map `assert*!`s back to
> KUnit, plus to add support for test functions that return `Result`s.
>
> In essence, they get our `#[test]`s essentially on par with the documentation
> tests.
>
> I also took the chance to convert some host `#[test]`s we had to KUnit in order
> to showcase the feature.
>
> Finally, I added documentation that was lacking from the original submission.
>
> I hope this helps.

Applied to `rust-next` -- thanks everyone!

    [ Used the `cfg_attr` from the TODO comment and clarified its comment
      now that the stabilization is in beta and thus quite likely stable
      in Rust 1.88.0. Simplified the `new_body` code by introducing a new
      variable. Added `#[allow(clippy::incompatible_msrv)]`. - Miguel ]

    [ Used `::kernel` for paths. - Miguel ]

    [ Split from the next commit as suggested by Tamir. - Miguel ]

    [ Split the `CString` simplification into a new commit. - Miguel ]

Cheers,
Miguel

Re: [PATCH 0/7] Rust KUnit `#[test]` support improvements

[Miguel Ojeda]

On Tue, May 27, 2025 at 2:10 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
>     [ Split from the next commit as suggested by Tamir. - Miguel ]
>
>     [ Split the `CString` simplification into a new commit. - Miguel ]

By the way, I kept the tags from David and Danilo in that new commit,
since it was really a pure split, but of course let me know if someone
doesn't want that.

Cheers,
Miguel

Re: [PATCH 0/7] Rust KUnit `#[test]` support improvements

[Miguel Ojeda]

On Tue, May 27, 2025 at 2:10 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
>     [ Used the `cfg_attr` from the TODO comment and clarified its comment
>       now that the stabilization is in beta and thus quite likely stable
>       in Rust 1.88.0. Simplified the `new_body` code by introducing a new
>       variable. Added `#[allow(clippy::incompatible_msrv)]`. - Miguel ]

One addition:

    Required `KUNIT=y` like for doctests.

The build error was reported by Stephen when merging -- thanks!

Cheers,
Miguel