Re: [PATCH v2 2/3] rust: kunit: allow same test name with different #[cfg(...)]

[David Gow <david@davidgow.net>]

Le 30/10/2026 à 00:59, Nicolás Antinori via B4 Relay a écrit :
> From: Nicolás Antinori <nico.antinori.7@gmail.com>
> 
> Sometimes it is necessary to test the same code paths under different
> configurations. The `#[cfg(...)]` macro can be used to check if specific
> configurations are enabled and run tests accordingly.
> 
> Currently, defining multiple tests with the same name under different
> `#[cfg(...)]` attributes results in a compilation error. This patch
> removes that restriction, allowing identical test names across different
> configurations.
> 
> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> ---

Thanks very much.

This is a great improvement over the old implementation, IMO.

Reviewed-by: David Gow <david@davidgow.net>

Cheers,
-- David

>   rust/kernel/kunit.rs | 22 ++++++++++++++++++++++
>   rust/macros/kunit.rs | 36 ++++++++++++++++++++----------------
>   2 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 5e688b743ee2152867e2f99c7f18b86e1b7fbe3d..ba3040d302b95af6d6e648a229ac8e9ea2c31a8c 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -367,6 +367,28 @@ fn rust_test_kunit_in_kunit_test() {
>           assert!(in_kunit_test());
>       }
>   
> +    // Tests with cfg have the same name on purpose because we are implicitly testing that
> +    // tests with the same name but different configs do not throw a compilation error
> +    #[test]
> +    #[cfg(CONFIG_RUST_KUNIT_SELFTEST = "y")]
> +    fn rust_test_kunit_run_only_active_cfg() {
> +        assert!(in_kunit_test());
> +    }
> +
> +    #[test]
> +    #[cfg(CONFIG_RUST_KUNIT_SELFTEST = "n")]
> +    fn rust_test_kunit_run_only_active_cfg() {
> +        // This test should never run because of the `cfg`.
> +        assert!(false)
> +    }
> +
> +    #[test]
> +    #[cfg(CONFIG_RUST_KUNIT_SELFTEST = "INVALID_VALUE")]
> +    fn rust_test_kunit_run_only_active_cfg() {
> +        // This test should never run because of the `cfg`.
> +        assert!(false)
> +    }
> +
>       #[test]
>       #[should_panic]
>       fn rust_test_kunit_panic_in_kunit_test() {
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index 288b53d21c4e7c8d9b5148d50c1a833f73e53015..173792563ca8b42e39ec0a53798f7b3bb75bf9cf 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -45,6 +45,8 @@ pub(crate) fn kunit_tests(test_suite: Ident, mut module: ItemMod) -> Result<Toke
>   
>       let mut processed_items = Vec::new();
>       let mut test_cases = Vec::new();
> +    let mut test_cases_cfgs = Vec::new();
> +    let mut test_case_number: u32 = 1;
>   
>       // Generate the test KUnit test suite and a test case for each `#[test]`.
>       //
> @@ -134,7 +136,7 @@ macro_rules! assert_eq {
>           // Add back the test item.
>           processed_items.push(Item::Fn(f));
>   
> -        let kunit_wrapper_fn_name = format_ident!("kunit_rust_wrapper_{test}");
> +        let kunit_wrapper_fn_name = format_ident!("kunit_rust_wrapper_{test}_{test_case_number}");
>           let test_cstr = LitCStr::new(
>               &CString::new(test_str.as_str()).expect("identifier cannot contain NUL"),
>               test.span(),
> @@ -154,18 +156,12 @@ macro_rules! assert_eq {
>               )
>           };
>           processed_items.push(parse_quote! {
> +            // Prepend any `cfg` attributes before the function declaration to activate it only in
> +            // cases the configs are on
> +            #(#cfg_attrs)*
>               unsafe extern "C" fn #kunit_wrapper_fn_name(_test: *mut ::kernel::bindings::kunit) {
> -                (*_test).status = ::kernel::bindings::kunit_status_KUNIT_SKIPPED;
> -
> -
> -                // Append any `cfg` attributes the user might have written on their tests so we
> -                // don't attempt to call them when they are `cfg`'d out. An extra `use` is used
> -                // here to reduce the length of the assert message.
> -                #(#cfg_attrs)*
> -                {
> -                    (*_test).status = ::kernel::bindings::kunit_status_KUNIT_SUCCESS;
> -                    #assertion
> -                }
> +                (*_test).status = ::kernel::bindings::kunit_status_KUNIT_SUCCESS;
> +                #assertion
>               }
>           });
>   
> @@ -174,15 +170,23 @@ macro_rules! assert_eq {
>               case.attr.rust_should_panic = #should_panic;
>               case
>           }));
> +        test_cases_cfgs.push(cfg_attrs);
> +        test_case_number += 1;
>       }
>   
> -    let num_tests_plus_1 = test_cases.len() + 1;
> +    // This dummy array is used to calculate the exact number of tests cases that are active
> +    // depending on their #[cfg(...)] attribute.
> +    processed_items.push(parse_quote!(
> +        const TEST_CASES_COUNT: &[()] = &[#(#(#test_cases_cfgs)* (),)* ()];
> +    ));
> +
>       processed_items.push(parse_quote! {
> -        static mut TEST_CASES: [::kernel::bindings::kunit_case; #num_tests_plus_1] = [
> -            #(#test_cases,)*
> -            ::pin_init::zeroed(),
> +        static mut TEST_CASES: [::kernel::bindings::kunit_case; TEST_CASES_COUNT.len()] = [
> +            #(#(#test_cases_cfgs)* { #test_cases },)*
> +            ::pin_init::zeroed()
>           ];
>       });
> +
>       processed_items.push(parse_quote! {
>           ::kernel::kunit_unsafe_test_suite!(#test_suite, TEST_CASES);
>       });
>