| Message ID | 20250502215133.1923676-1-ojeda@kernel.org |
|---|---|
| Headers | show |
| Series | Rust KUnit `#[test]` support improvements | expand |
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: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
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 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
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
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?
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
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