Re: [PATCH 2/2] rust: kunit: move test configuration gating into macro

[David Gow <david@davidgow.net>]

Le 18/08/2026 à 11:23 PM, Yury Norov a écrit :
> KUnit test modules currently need a separate cfg attribute in addition to
> the kunit_tests attribute. This makes every test suite repeat the same
> two-attribute pattern.
> 
> Declaring a KUnit test without a preceding `#[cfg]` attribute is also
> possible, causing it to be built whenever KUnit and its containing code are
> enabled.
> 
> Require the controlling Kconfig symbol as the second kunit_tests argument
> and have the macro emit the cfg attribute itself. Update all existing users
> to the new form.
> 
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---

Thanks.

I agree that every test needs to be behind a Kconfig option, though I'm
not totally convinced that this is the optimal solution, particularly
for a problem which should only affect test builds.

Personally, I lean towards keeping the #[cfg()] attribute separate here,
for mostly minor stylistic reasons. In particular, in C we already have
a number of existing tests where multiple suites share a Kconfig symbol.
While there's no reason we couldn't just pass the same symbol to several
different kunit_suite() attributes, it would be nicer to group them all
behind a single #[cfg()] where that makes sense (or even, going forward,
to support having them live in their own separate module/crate).

And personally, I find it a bit clearer: people already understand
#[cfg()], and having #[kunit_suite()] just accept the suite name better
matches suite registration in C.

That being said: I don't think there's anything fundamentally impossible
with implementing something like this (be it now, or later): worst-case
we'd end up with some redundant Kconfig entries and uglier chains of
attributes. So if this continues to be a problem (or enough people think
I'm wrong here), we can always change it later.

If we want to improve on the current situation, though, something like a
checkpatch.pl check or a lint would be a good way of making it less
likely for checks to slip through the cracks. And if we're particularly
worried about test size (or runtime), there are things we could look at
to improve things there. Moving tests to separate modules is probably a
bit harder to do in Rust than in C, but we could possibly have a
separate set of config options for large tests, and better support
filtering slow tests in Rust (which is on my to-do list).

(Finally, there are also a few minor issues with the patch below, which
would need to be fixed if there's a v2.)

Cheers,
-- David

>  Documentation/rust/testing.rst                | 11 ++++---
>  .../gpu/nova-core/gsp/cmdq/continuation.rs    |  3 +-
>  rust/kernel/alloc/allocator.rs                |  3 +-
>  rust/kernel/alloc/kvec.rs                     |  3 +-
>  rust/kernel/bitfield.rs                       |  3 +-
>  rust/kernel/bitmap.rs                         |  3 +-
>  rust/kernel/kunit.rs                          |  3 +-
>  rust/kernel/str.rs                            |  3 +-
>  rust/kernel/sync/atomic/predefine.rs          |  3 +-
>  rust/macros/kunit.rs                          | 30 +++++++++++++++++--
>  rust/macros/lib.rs                            |  6 ++--
>  11 files changed, 43 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst
> index e3943aceceb9..767b111afd41 100644
> --- a/Documentation/rust/testing.rst
> +++ b/Documentation/rust/testing.rst
> @@ -138,9 +138,10 @@ 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.
> +the name of the test suite and its controlling Kconfig option as arguments. The
> +Kconfig option is required, and the macro uses it to guard the test suite.
>  
> -Each test suite should be guarded by a Kconfig option in
> +Each test suite should have a Kconfig option, typically in
>  ``rust/kernel/Kconfig.test``.
>  
>  For instance, assume we want to test the function ``f`` from the documentation
> @@ -148,8 +149,7 @@ tests section. We could write, in the same file where we have our function:
>  
>  .. code-block:: rust
>  
> -	#[cfg(CONFIG_RUST_MYMOD_KUNIT_TEST)]
> -	#[kunit_tests(rust_kernel_mymod)]
> +	#[kunit_tests(rust_kernel_mymod, CONFIG_RUST_MYMOD_KUNIT_TEST)]
>  	mod tests {
>  	    use super::*;
>  
> @@ -177,8 +177,7 @@ the unit type ``()``) or ``Result`` (i.e. any ``Result<T, E>``). For instance:
>  
>  .. code-block:: rust
>  
> -	#[cfg(CONFIG_RUST_MYMOD_KUNIT_TEST)]
> -	#[kunit_tests(rust_kernel_mymod)]
> +	#[kunit_tests(rust_kernel_mymod, CONFIG_RUST_MYMOD_KUNIT_TEST)]
>  	mod tests {
>  	    use super::*;
>  
> diff --git a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> index c0aa16c8fbf4..2e4b21f2002f 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> @@ -167,8 +167,7 @@ fn init_variable_payload(
>      }
>  }
>  
> -#[cfg(CONFIG_NOVA_CORE_KUNIT_TEST)]
> -#[kunit_tests(nova_core_gsp_continuation)]
> +#[kunit_tests(nova_core_gsp_continuation, CONFIG_NOVA_CORE_KUNIT_TEST)]
>  mod tests {
>      use super::*;
>  
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index cd4203f27aed..b2c235801a3f 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -265,8 +265,7 @@ unsafe fn realloc(
>      }
>  }
>  
> -#[cfg(CONFIG_RUST_ALLOCATOR_KUNIT_TEST)]
> -#[macros::kunit_tests(rust_allocator)]
> +#[macros::kunit_tests(rust_allocator, CONFIG_RUST_ALLOCATOR_KUNIT_TEST)]
>  mod tests {
>      use super::*;
>      use core::mem::MaybeUninit;
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index f7af62835aa8..4b77425950d3 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -1508,8 +1508,7 @@ fn drop(&mut self) {
>      }
>  }
>  
> -#[cfg(CONFIG_RUST_KVEC_KUNIT_TEST)]
> -#[macros::kunit_tests(rust_kvec)]
> +#[macros::kunit_tests(rust_kvec, CONFIG_RUST_KVEC_KUNIT_TEST)]
>  mod tests {
>      use super::*;
>      use crate::prelude::*;
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index 35ede53f2b8e..8b329b6e4f8d 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -548,8 +548,7 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
>      };
>  }
>  
> -#[cfg(CONFIG_RUST_BITFIELD_KUNIT_TEST)]
> -#[::kernel::macros::kunit_tests(rust_kernel_bitfield)]
> +#[::kernel::macros::kunit_tests(rust_kernel_bitfield, CONFIG_RUST_BITFIELD_KUNIT_TEST)]
>  mod tests {
>      use core::convert::TryFrom;
>  
> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index b27e0ec80d64..c4195e00d895 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs
> @@ -499,8 +499,7 @@ pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
>      }
>  }
>  
> -#[cfg(CONFIG_RUST_BITMAP_KUNIT_TEST)]
> -#[macros::kunit_tests(rust_kernel_bitmap)]
> +#[macros::kunit_tests(rust_kernel_bitmap, CONFIG_RUST_BITMAP_KUNIT_TEST)]
>  mod tests {
>      use super::*;
>      use kernel::alloc::flags::GFP_KERNEL;
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 91eaff8c186a..52f2ccbfba49 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -330,8 +330,7 @@ pub fn in_kunit_test() -> bool {
>      !unsafe { bindings::kunit_get_current_test() }.is_null()
>  }
>  
> -#[cfg(CONFIG_RUST_KUNIT_SELFTEST)]
> -#[kunit_tests(rust_kernel_kunit)]
> +#[kunit_tests(rust_kernel_kunit, CONFIG_RUST_KUNIT_SELFTEST)]
>  mod tests {
>      use super::*;
>  
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index b3caa9a1c898..404418504c13 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -428,8 +428,7 @@ macro_rules! c_str {
>      }};
>  }
>  
> -#[cfg(CONFIG_RUST_STR_KUNIT_TEST)]
> -#[kunit_tests(rust_kernel_str)]
> +#[kunit_tests(rust_kernel_str, CONFIG_RUST_STR_KUNIT_TEST)]
>  mod tests {
>      use super::*;
>  
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 3d63f40791fa..e92194b37de8 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -152,8 +152,7 @@ fn rhs_into_delta(rhs: usize) -> isize_atomic_repr {
>      }
>  }
>  
> -#[cfg(CONFIG_RUST_ATOMICS_KUNIT_TEST)]
> -#[macros::kunit_tests(rust_atomics)]
> +#[macros::kunit_tests(rust_atomics, CONFIG_RUST_ATOMICS_KUNIT_TEST)]
>  mod tests {
>      use super::super::*;
>  
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index ae20ed6768f1..7c7ba30f58c0 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -13,6 +13,10 @@
>      ToTokens, //
>  };
>  use syn::{
> +    parse::{
> +        Parse,
> +        ParseStream, //
> +    },
>      parse_quote,
>      Error,
>      Ident,
> @@ -20,9 +24,28 @@
>      ItemMod,
>      LitCStr,
>      Result, //
> +    Token,
>  };
>  
> -pub(crate) fn kunit_tests(test_suite: Ident, mut module: ItemMod) -> Result<TokenStream> {
> +pub(crate) struct KunitTestArgs {
> +    test_suite: Ident,
> +    config: Ident,

#[cfg()] accepts more than just an Ident here: if we want to explicitly
gate things on either multiple config options, or on config options
being built-in/modules/etc, then we might want to allow something more
complex.

> +}
> +
> +impl Parse for KunitTestArgs {
> +    fn parse(input: ParseStream<'_>) -> Result<Self> {
> +        let test_suite = input.parse()?;
> +        input.parse::<Token![,]>()?;
> +        let config = input.parse()?;
> +
> +        Ok(Self { test_suite, config })
> +    }
> +}
> +
> +pub(crate) fn kunit_tests(
> +    KunitTestArgs { test_suite, config }: KunitTestArgs,
> +    mut module: ItemMod,
> +) -> Result<TokenStream> {
>      if test_suite.to_string().len() > 255 {
>          return Err(Error::new_spanned(
>              test_suite,
> @@ -34,7 +57,7 @@ pub(crate) fn kunit_tests(test_suite: Ident, mut module: ItemMod) -> Result<Toke
>      let Some((module_brace, module_items)) = module.content.take() else {
>          Err(Error::new_spanned(
>              module,
> -            "`#[kunit_tests(test_name)]` attribute should only be applied to inline modules",
> +            "`#[kunit_tests(test_name, CONFIG_KUNIT_TEST)]` attribute should only be applied to inline modules",

This is a much uglier message, and CONFIG_KUNIT_TEST is not an ideal
example to have here. Could we just have a placeholder, e.g.
#[kunit_tests(...)].

>          ))?
>      };
>  
> @@ -42,6 +65,7 @@ pub(crate) fn kunit_tests(test_suite: Ident, mut module: ItemMod) -> Result<Toke
>      module
>          .attrs
>          .insert(0, parse_quote!(#[cfg(CONFIG_KUNIT="y")]));
> +    module.attrs.insert(0, parse_quote!(#[cfg(#config)]));
>  
>      let mut processed_items = Vec::new();
>      let mut test_cases = Vec::new();
> @@ -51,7 +75,7 @@ pub(crate) fn kunit_tests(test_suite: Ident, mut module: ItemMod) -> Result<Toke
>      // The code generated for the following test module:
>      //
>      // ```
> -    // #[kunit_tests(kunit_test_suit_name)]
> +    // #[kunit_tests(kunit_test_suite_name, CONFIG_KUNIT_TEST)]

I really would rather these examples not state CONFIG_KUNIT_TEST here:
it's the config option for tests for KUnit itself, and I'd rather not
imply that people should add their tests to it.
CONFIG_KUNIT_EXAMPLE_TEST would be better.

>      // mod tests {
>      //     #[test]
>      //     fn foo() {
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 4a48fabbc268..a660296d5dbd 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -464,14 +464,14 @@ pub fn paste(input: TokenStream) -> TokenStream {
>  
>  /// Registers a KUnit test suite and its test cases using a user-space like syntax.
>  ///
> -/// This macro should be used on modules. If `CONFIG_KUNIT` (in `.config`) is `n`, the target module
> -/// is ignored.
> +/// This macro should be used on modules. The second argument is the Kconfig symbol that controls
> +/// the test suite. If it or `CONFIG_KUNIT` (in `.config`) is `n`, the target module is ignored.
>  ///
>  /// # Examples
>  ///
>  /// ```ignore
>  /// # use kernel::prelude::*;
> -/// #[kunit_tests(kunit_test_suit_name)]
> +/// #[kunit_tests(kunit_test_suite_name, CONFIG_KUNIT_TEST)]

As above, let's not use CONFIG_KUNIT_TEST in the documentation here.

>  /// mod tests {
>  ///     #[test]
>  ///     fn foo() {