Message-ID: <CABVgOS=BPZYrJ8NLbVHj4S5CBpPd3xovBFCn8rhZmSuYq0rWSQ@mail.gmail.com>
Date: Tue, 29 Jul 2025 14:27:53 +0800
From: David Gow <davidgow@...gle.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: Alex Gaynor <alex.gaynor@...il.com>, Danilo Krummrich <dakr@...nel.org>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, Masahiro Yamada <masahiroy@...nel.org>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org, 
	Rae Moar <rmoar@...gle.com>, linux-kselftest@...r.kernel.org, 
	kunit-dev@...glegroups.com, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH] rust: kernel: remove support for unused host `#[test]`s

On Sun, 27 Jul 2025 at 02:08, Miguel Ojeda <ojeda@...nel.org> wrote:
>
> Since commit 028df914e546 ("rust: str: convert `rusttest` tests into
> KUnit"), we do not have anymore host `#[test]`s that run in the host.
>
> Moreover, we do not plan to add any new ones -- tests should generally
> run within KUnit, since there they are built the same way the kernel
> does. While we may want to have some way to define tests that can also
> be run outside the kernel, we still want to test within the kernel too
> [1], and thus would likely use a custom syntax anyway to define them.
>
> Thus simplify the `rusttest` target by removing support for host
> `#[test]`s for the `kernel` crate.
>
> This still maintains the support for the `macros` crate, even though we
> do not have any such tests there.
>
> Link: https://lore.kernel.org/rust-for-linux/CABVgOS=AKHSfifp0S68K3jgNZAkALBr=7iFb=niryG5WDxjSrg@mail.gmail.com/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---

With my (biased) KUnit hat on, this looks good to me.

_Maybe_ it's worth updating Documentation/rust/testing.rst, though it
already says these are "mostly used for testing the macros crate's
examples".

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

Cheers,
-- David

>  rust/Makefile        | 9 +--------
>  rust/kernel/alloc.rs | 6 +++---
>  rust/kernel/error.rs | 4 ++--
>  rust/kernel/lib.rs   | 2 +-
>  4 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 115b63b7d1e3..5290b37868dd 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -235,7 +235,7 @@ quiet_cmd_rustc_test = $(RUSTC_OR_CLIPPY_QUIET) T  $<
>         $(objtree)/$(obj)/test/$(subst rusttest-,,$@) $(rust_test_quiet) \
>                 $(rustc_test_run_flags)
>
> -rusttest: rusttest-macros rusttest-kernel
> +rusttest: rusttest-macros
>
>  rusttest-macros: private rustc_target_flags = --extern proc_macro \
>         --extern macros --extern kernel --extern pin_init
> @@ -245,13 +245,6 @@ rusttest-macros: $(src)/macros/lib.rs \
>         +$(call if_changed,rustc_test)
>         +$(call if_changed,rustdoc_test)
>
> -rusttest-kernel: private rustc_target_flags = --extern ffi --extern pin_init \
> -    --extern build_error --extern macros --extern bindings --extern uapi
> -rusttest-kernel: $(src)/kernel/lib.rs rusttestlib-ffi rusttestlib-kernel \
> -    rusttestlib-build_error rusttestlib-macros rusttestlib-bindings \
> -    rusttestlib-uapi rusttestlib-pin_init FORCE
> -       +$(call if_changed,rustc_test)
> -
>  ifdef CONFIG_CC_IS_CLANG
>  bindgen_c_flags = $(c_flags)
>  else
> diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> index a2c49e5494d3..335ae3271fa8 100644
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -2,16 +2,16 @@
>
>  //! Implementation of the kernel's memory allocation infrastructure.
>
> -#[cfg(not(any(test, testlib)))]
> +#[cfg(not(testlib))]
>  pub mod allocator;
>  pub mod kbox;
>  pub mod kvec;
>  pub mod layout;
>
> -#[cfg(any(test, testlib))]
> +#[cfg(testlib)]
>  pub mod allocator_test;
>
> -#[cfg(any(test, testlib))]
> +#[cfg(testlib)]
>  pub use self::allocator_test as allocator;
>
>  pub use self::kbox::Box;
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 3dee3139fcd4..7812aca1b6ef 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -157,7 +157,7 @@ pub fn to_ptr<T>(self) -> *mut T {
>      }
>
>      /// Returns a string representing the error, if one exists.
> -    #[cfg(not(any(test, testlib)))]
> +    #[cfg(not(testlib))]
>      pub fn name(&self) -> Option<&'static CStr> {
>          // SAFETY: Just an FFI call, there are no extra safety requirements.
>          let ptr = unsafe { bindings::errname(-self.0.get()) };
> @@ -174,7 +174,7 @@ pub fn name(&self) -> Option<&'static CStr> {
>      /// When `testlib` is configured, this always returns `None` to avoid the dependency on a
>      /// kernel function so that tests that use this (e.g., by calling [`Result::unwrap`]) can still
>      /// run in userspace.
> -    #[cfg(any(test, testlib))]
> +    #[cfg(testlib)]
>      pub fn name(&self) -> Option<&'static CStr> {
>          None
>      }
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index e13d6ed88fa6..8a0153f61732 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -197,7 +197,7 @@ pub const fn as_ptr(&self) -> *mut bindings::module {
>      }
>  }
>
> -#[cfg(not(any(testlib, test)))]
> +#[cfg(not(testlib))]
>  #[panic_handler]
>  fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>      pr_emerg!("{}\n", info);
>
> base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
> --
> 2.50.1
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5281 bytes)

Powered by blists - more mailing lists