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

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]

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
>

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
>
>

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

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 :(

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

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