[PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support

Nicolás Antinori posted 3 patches 1 week, 2 days ago
include/kunit/test.h      |  1 +
include/kunit/try-catch.h |  1 +
lib/kunit/test.c          | 14 +++++++
lib/kunit/try-catch.c     |  7 ++++
rust/kernel/bitmap.rs     | 40 +++++++++-----------
rust/kernel/kunit.rs      | 24 ++++++++++++
rust/kernel/lib.rs        | 46 +++++++++++++++++++++--
rust/macros/kunit.rs      | 78 ++++++++++++++++++++++++++++++++++++---
8 files changed, 181 insertions(+), 30 deletions(-)
[PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by Nicolás Antinori 1 week, 2 days ago
This patch series intends to implement two features for KUnit tests
written in Rust. The work is based on a TODO comment made in the
`bitmap.rs` module [1].

1. Supporting `#[should_panic]` [2]:

KUnit tests in Rust follow the user-space syntax, but at the moment
`#[should_panic]` is not supported. The first patch of this series adds
support for the attribute (only in its basic form, `#[should_panic =
"message"]` is not supported, and I don't know if it makes sense to
support it)

The way it is supported is by having a separate `#[panic_handler]` when
`CONFIG_KUNIT` is enabled. When a test is marked with `#[should_panic]`,
a static value (KUNIT_SHOULD_PANIC = 0xDEAD7357) is assigned to the
kunit's `priv` field, since it is meant for saving arbitrary user data
[3]. At the moment, I did not find any place where Rust tests use that
field, so it should be safe to write it.

When the test panics, the `#[panic_handler]` function is called, obtains
the kunit current test and checks if the `priv` field is not null and
contains the value `KUNIT_SHOULD_PANIC`.

If those conditions are true, it marks the test as successful (since it
panicked as expected) and calls `__kunit_abort_expecting_error`, a new
function that exits the testing thread but fills `try_catch->try_result`
with a 0 so the test runner does not mistake it as a failed test.

If those conditions are not true:
 - If `priv` is null, the test panics as it would have before having
   this feature, priv = null means that the test was not expected to
   panic.
 - If `priv` is not null but its value is not `KUNIT_SHOULD_PANIC`, the
   test panics with an error message informing that the code found in
   `priv` was invalid.

If the test does not panic, the `#[panic_handler]` is not triggered. The
test is marked as failed (since it was expected to panic).

Regarding this feature:
 - Do this approach make sense?
 - Is it ok to mark the `#[should_panic]` tests with a static constant?
   Is another mechanism better to check in the `#[panic_handler]` that
   the test was supposed to panic?

2. Allow same test name with different #[cfg(...)]:

When testing `#[should_panic]` in `bitmap.rs` (check the last patch of
the series) I found that the test that was supposed to panic had the
same name as another one, but they were run on different configurations.
This caused the following compilation error:

ERROR:root:error[E0428]: the name `kunit_rust_wrapper_owned_bitmap_out_of_bounds` is defined multiple times
   --> ../rust/kernel/bitmap.rs:503:1
    |
503 | #[macros::kunit_tests(rust_kernel_bitmap)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `kunit_rust_wrapper_owned_bitmap_out_of_bounds` redefined here
    |
    = note: `kunit_rust_wrapper_owned_bitmap_out_of_bounds` must be defined only once in the value namespace of this module
    = note: this error originates in the attribute macro `macros::kunit_tests` (in Nightly builds, run with -Z macro-backtrace for more info)


To fix this problem, I appended to the `kunit_rust_wrapper_*`
identifiers an 'stringified' version of the test's #[cfg(...)]
arguments. The purpose of this is that, if we have a test with the same
name and configuration, it would fail.

The configuration string was also appended to the tests names. This was
done to have a better test run report:

...
[SKIPPED] owned_bitmap_out_of_bounds_cfg_not_config_rust_bitmap_hardened
[PASSED] owned_bitmap_out_of_bounds_cfg_config_rust_bitmap_hardened
...

Otherwise we would have something like the following:
...
[SKIPPED] owned_bitmap_out_of_bounds
[PASSED] owned_bitmap_out_of_bounds
...

Regarding this:
- Does it makes sense to allow same test names with different cfgs?
- Is it ok to 'stringify' the configuration so it can be distinguished
  in the report? Would you prefer something like `_case_1` `_case_2` ..
  instead?

This is the first RFC patch I send to the LKML, if there's something not
right with it please let me know.

Kind Regards,
Nicolás

[1] https://github.com/Rust-for-Linux/linux/blob/fd73f4a6659897191fa0d40695fe370925dd3780/rust/kernel/bitmap.rs#L592-L600
[2] https://doc.rust-lang.org/rust-by-example/testing/unit_testing.html#testing-panics
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/kunit/test.h?id=f6e7b42bf05b2427fb8a7a1d1c387a86638bb413#n314

Nicolás Antinori (3):
  rust: kunit: add #[should_panic] support
  rust: kunit: allow same test name with different #[cfg(...)]
  rust: bitmap: kunit: uncomment owned_bitmap_out_of_bounds panic case

 include/kunit/test.h      |  1 +
 include/kunit/try-catch.h |  1 +
 lib/kunit/test.c          | 14 +++++++
 lib/kunit/try-catch.c     |  7 ++++
 rust/kernel/bitmap.rs     | 40 +++++++++-----------
 rust/kernel/kunit.rs      | 24 ++++++++++++
 rust/kernel/lib.rs        | 46 +++++++++++++++++++++--
 rust/macros/kunit.rs      | 78 ++++++++++++++++++++++++++++++++++++---
 8 files changed, 181 insertions(+), 30 deletions(-)

--
2.47.3

Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by David Gow 2 days, 22 hours ago
Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
> This patch series intends to implement two features for KUnit tests
> written in Rust. The work is based on a TODO comment made in the
> `bitmap.rs` module [1].
> 

Thanks very much for this series! It works fine here, but I think there 
are a few other options for how this could be implemented, and it's 
probably worth our at least considering them.

In particular, we've already got code for suppressing warnings, and I'm 
not sure whether it makes sense to unify all of the different attempts 
to intercept panics / bugs / warnings of various kinds.

That being said, Rust has unwinding and panic handlers as a core part of 
the language, and the C side of the kernel doesn't. Combine that with 
the fact that #[should_panic] is already standardised in Rust, and the 
argument for a separate implementation is not totally silly either.

Do you think that #[should_panic] should only trigger on a rust 
panic!(), or on any kernel panic? I'm leaning towards the former, but if 
the latter then we'd need to implement it in C and provide a C interface 
to it.

> 1. Supporting `#[should_panic]` [2]:
> 
> KUnit tests in Rust follow the user-space syntax, but at the moment
> `#[should_panic]` is not supported. The first patch of this series adds
> support for the attribute (only in its basic form, `#[should_panic =
> "message"]` is not supported, and I don't know if it makes sense to
> support it)
> 
> The way it is supported is by having a separate `#[panic_handler]` when
> `CONFIG_KUNIT` is enabled. When a test is marked with `#[should_panic]`,
> a static value (KUNIT_SHOULD_PANIC = 0xDEAD7357) is assigned to the
> kunit's `priv` field, since it is meant for saving arbitrary user data
> [3]. At the moment, I did not find any place where Rust tests use that
> field, so it should be safe to write it.

I don't think the `priv` field is the optimal place to put this. I don't 
think it's strictly a _problem_, particularly since Rust tests aren't 
using it, but nominally `priv` is for test use, and I'd rather not use 
it here (there may be future tests which want to use priv for something 
else, particularly as a quick way of passing test state between C and Rust).

For most of these sorts of things, I'd recommend using a KUnit 'named 
resource', but alas, there aren't any Rust binding for these. That being 
said, we've used named resources in C because they're setup at runtime 
(which is how we've handled this in the past). That's useful if we want 
to note that a particular line in the test wants to panic, but if we're 
only concerned with whether a test as a whole panics, then this could be 
static.

In that case, how about adding a new `rust_should_panic` field to 
`struct kunit_attributes`. If you only care about whether a panic 
occurs, this could just be a boolean, but it also could be a place to 
store, for example, a string to support #[should_panic = "message"] if 
you wish.

As an attribute, you could also then add it to lib/kunit/attributes.c 
(probably with PRINT_NEVER, as I don't think we need it included in KTAP 
output), which would, for example, allow us to filter tests by whether 
or not they expect to panic.

> 
> When the test panics, the `#[panic_handler]` function is called, obtains
> the kunit current test and checks if the `priv` field is not null and
> contains the value `KUNIT_SHOULD_PANIC`.
> 
> If those conditions are true, it marks the test as successful (since it
> panicked as expected) and calls `__kunit_abort_expecting_error`, a new
> function that exits the testing thread but fills `try_catch->try_result`
> with a 0 so the test runner does not mistake it as a failed test.
> 
> If those conditions are not true:
>   - If `priv` is null, the test panics as it would have before having
>     this feature, priv = null means that the test was not expected to
>     panic.
>   - If `priv` is not null but its value is not `KUNIT_SHOULD_PANIC`, the
>     test panics with an error message informing that the code found in
>     `priv` was invalid.
> 
> If the test does not panic, the `#[panic_handler]` is not triggered. The
> test is marked as failed (since it was expected to panic).
> 
> Regarding this feature:
>   - Do this approach make sense?

Yes, I think this approach makes sense. While I think a less 
rust-specific way of trapping panics could be useful (à la the 
suppressed warning system), I am erring on the side of implementing it 
this way given it (a) doesn't involve

>   - Is it ok to mark the `#[should_panic]` tests with a static constant?
>     Is another mechanism better to check in the `#[panic_handler]` that
>     the test was supposed to panic?

I think that we do want to base this off the struct kunit, though a 
special constant in 'priv' is not optimal. I'd go with either a named 
kunit resource (alas, which don't have Rust bindings) if we'd want to 
support extending this to specify a specific line / block panicking; or 
a new field in struct kunit_attributes.

> 2. Allow same test name with different #[cfg(...)]:
> 
> When testing `#[should_panic]` in `bitmap.rs` (check the last patch of
> the series) I found that the test that was supposed to panic had the
> same name as another one, but they were run on different configurations.
> This caused the following compilation error:
> 
> ERROR:root:error[E0428]: the name `kunit_rust_wrapper_owned_bitmap_out_of_bounds` is defined multiple times
>     --> ../rust/kernel/bitmap.rs:503:1
>      |
> 503 | #[macros::kunit_tests(rust_kernel_bitmap)]
>      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `kunit_rust_wrapper_owned_bitmap_out_of_bounds` redefined here
>      |
>      = note: `kunit_rust_wrapper_owned_bitmap_out_of_bounds` must be defined only once in the value namespace of this module
>      = note: this error originates in the attribute macro `macros::kunit_tests` (in Nightly builds, run with -Z macro-backtrace for more info)
> 
> 
> To fix this problem, I appended to the `kunit_rust_wrapper_*`
> identifiers an 'stringified' version of the test's #[cfg(...)]
> arguments. The purpose of this is that, if we have a test with the same
> name and configuration, it would fail.
> 
> The configuration string was also appended to the tests names. This was
> done to have a better test run report:
> 
> ...
> [SKIPPED] owned_bitmap_out_of_bounds_cfg_not_config_rust_bitmap_hardened
> [PASSED] owned_bitmap_out_of_bounds_cfg_config_rust_bitmap_hardened
> ...
> 
> Otherwise we would have something like the following:
> ...
> [SKIPPED] owned_bitmap_out_of_bounds
> [PASSED] owned_bitmap_out_of_bounds
> ...
> 
> Regarding this:
> - Does it makes sense to allow same test names with different cfgs?

Yes-ish. I think it definitely makes sense for the same test to be 
redefined with different cfgs, but I'd rather only one of those tests 
then actually be compiled in (see below).

> - Is it ok to 'stringify' the configuration so it can be distinguished
>    in the report? Would you prefer something like `_case_1` `_case_2` ..
>    instead?

I don't _like_ this: my preference would be for us to keep the same 
name, and just not emit a test_case for anything which should be 
compiled out with cfg. Unfortunately, implementing that is a bit harder 
than would be ideal: we need a way of evaluating the cfg() arguments in 
a proc macro, I think. (Ultimately, because otherwise there's no way of 
statically determining the length of the TEST_CASES array?)

Unless you've got a good idea how to fix this, though, I'm happy to put 
up with adding the configs to the name for now. Though if there's a nice 
way to make the names shorter 
(rust_test_kunit_parse_cfg_in_kunit_test_cfg_config_rust_kunit_selftest_equals_n 
is definitely too long a test name, for instance), that'd be best.

> This is the first RFC patch I send to the LKML, if there's something not
> right with it please let me know.
> 
> Kind Regards,
> Nicolás
> 
> [1] https://github.com/Rust-for-Linux/linux/blob/fd73f4a6659897191fa0d40695fe370925dd3780/rust/kernel/bitmap.rs#L592-L600
> [2] https://doc.rust-lang.org/rust-by-example/testing/unit_testing.html#testing-panics
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/kunit/test.h?id=f6e7b42bf05b2427fb8a7a1d1c387a86638bb413#n314
> 
> Nicolás Antinori (3):
>    rust: kunit: add #[should_panic] support
>    rust: kunit: allow same test name with different #[cfg(...)]
>    rust: bitmap: kunit: uncomment owned_bitmap_out_of_bounds panic case
> 
>   include/kunit/test.h      |  1 +
>   include/kunit/try-catch.h |  1 +
>   lib/kunit/test.c          | 14 +++++++
>   lib/kunit/try-catch.c     |  7 ++++
>   rust/kernel/bitmap.rs     | 40 +++++++++-----------
>   rust/kernel/kunit.rs      | 24 ++++++++++++
>   rust/kernel/lib.rs        | 46 +++++++++++++++++++++--
>   rust/macros/kunit.rs      | 78 ++++++++++++++++++++++++++++++++++++---
>   8 files changed, 181 insertions(+), 30 deletions(-)
> 
> --
> 2.47.3
> 

Cheers,
-- David

Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by Nicolás Antinori 1 day, 15 hours ago
Thank you for the feedback!

On Tue Sep 22, 2026 at 4:56 AM -03, David Gow wrote:
> Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
>> This patch series intends to implement two features for KUnit tests
>> written in Rust. The work is based on a TODO comment made in the
>> `bitmap.rs` module [1].
>> 
>
> Thanks very much for this series! It works fine here, but I think there 
> are a few other options for how this could be implemented, and it's 
> probably worth our at least considering them.
>
> In particular, we've already got code for suppressing warnings, and I'm 
> not sure whether it makes sense to unify all of the different attempts 
> to intercept panics / bugs / warnings of various kinds.
>
> That being said, Rust has unwinding and panic handlers as a core part of 
> the language, and the C side of the kernel doesn't. Combine that with 
> the fact that #[should_panic] is already standardised in Rust, and the 
> argument for a separate implementation is not totally silly either.
>
> Do you think that #[should_panic] should only trigger on a rust 
> panic!(), or on any kernel panic? I'm leaning towards the former, but if 
> the latter then we'd need to implement it in C and provide a C interface 
> to it.

When I sent the series I leaned towards the former too. But thinking
about it I believe there are situations where a kernel panic can be
originated from C code called by Rust, for example, this test case:

#[test]
#[should_panic]
fn rust_test_kunit_panic_in_kunit_test_bug() {
    unsafe { bindings::BUG() };
}

This kernel panic is not caught by the Rust's panic handler. In the
current version of my code, I catch that in
lib/kunit/test.c::kunit_run_case_catch_errors function. With that
modification there's no need of a Rust side panic handler (as it catches
Rust's panics too, since the panic hanlder executes a bindings::BUG()).

>
>> 1. Supporting `#[should_panic]` [2]:
>> 
>> KUnit tests in Rust follow the user-space syntax, but at the moment
>> `#[should_panic]` is not supported. The first patch of this series adds
>> support for the attribute (only in its basic form, `#[should_panic =
>> "message"]` is not supported, and I don't know if it makes sense to
>> support it)
>> 
>> The way it is supported is by having a separate `#[panic_handler]` when
>> `CONFIG_KUNIT` is enabled. When a test is marked with `#[should_panic]`,
>> a static value (KUNIT_SHOULD_PANIC = 0xDEAD7357) is assigned to the
>> kunit's `priv` field, since it is meant for saving arbitrary user data
>> [3]. At the moment, I did not find any place where Rust tests use that
>> field, so it should be safe to write it.
>
> I don't think the `priv` field is the optimal place to put this. I don't 
> think it's strictly a _problem_, particularly since Rust tests aren't 
> using it, but nominally `priv` is for test use, and I'd rather not use 
> it here (there may be future tests which want to use priv for something 
> else, particularly as a quick way of passing test state between C and Rust).
>
> For most of these sorts of things, I'd recommend using a KUnit 'named 
> resource', but alas, there aren't any Rust binding for these. That being 
> said, we've used named resources in C because they're setup at runtime 
> (which is how we've handled this in the past). That's useful if we want 
> to note that a particular line in the test wants to panic, but if we're 
> only concerned with whether a test as a whole panics, then this could be 
> static.

I did not know that you could test particular lines for panics! That
said, I believe the #[should_panic] attribute is meant to check if the
test panics as a whole.

If I had to test a particular line for panic that I'd write a new test,
but that's just how I'd do it :P.

>
> In that case, how about adding a new `rust_should_panic` field to 
> `struct kunit_attributes`. If you only care about whether a panic 
> occurs, this could just be a boolean, but it also could be a place to 
> store, for example, a string to support #[should_panic = "message"] if 
> you wish.

I could not find a way to retrieve the kunit_case struct from Rust. I
believe this is needed for implementing the check because the actual
panic message can only be retrieved from the PanicInfo [1] struct. 

I am sure this can be implemented (the first things that comes to mind
is having a C api that retrieves the current kunit_case struct, but I am
not sure if the kunit_case meant to be leaked outside the runner) but
I'd do it in another iteration if we find that it is useful.

>
> As an attribute, you could also then add it to lib/kunit/attributes.c 
> (probably with PRINT_NEVER, as I don't think we need it included in KTAP 
> output), which would, for example, allow us to filter tests by whether 
> or not they expect to panic.

Excellent! I'll do that!

>> ...
>> 2. Allow same test name with different #[cfg(...)]:
>> 
>> When testing `#[should_panic]` in `bitmap.rs` (check the last patch of
>> the series) I found that the test that was supposed to panic had the
>> same name as another one, but they were run on different configurations.
>> This caused the following compilation error:
>> 
>> ERROR:root:error[E0428]: the name `kunit_rust_wrapper_owned_bitmap_out_of_bounds` is defined multiple times
>>     --> ../rust/kernel/bitmap.rs:503:1
>>      |
>> 503 | #[macros::kunit_tests(rust_kernel_bitmap)]
>>      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `kunit_rust_wrapper_owned_bitmap_out_of_bounds` redefined here
>>      |
>>      = note: `kunit_rust_wrapper_owned_bitmap_out_of_bounds` must be defined only once in the value namespace of this module
>>      = note: this error originates in the attribute macro `macros::kunit_tests` (in Nightly builds, run with -Z macro-backtrace for more info)
>> 
>> 
>> To fix this problem, I appended to the `kunit_rust_wrapper_*`
>> identifiers an 'stringified' version of the test's #[cfg(...)]
>> arguments. The purpose of this is that, if we have a test with the same
>> name and configuration, it would fail.
>> 
>> The configuration string was also appended to the tests names. This was
>> done to have a better test run report:
>> 
>> ...
>> [SKIPPED] owned_bitmap_out_of_bounds_cfg_not_config_rust_bitmap_hardened
>> [PASSED] owned_bitmap_out_of_bounds_cfg_config_rust_bitmap_hardened
>> ...
>> 
>> Otherwise we would have something like the following:
>> ...
>> [SKIPPED] owned_bitmap_out_of_bounds
>> [PASSED] owned_bitmap_out_of_bounds
>> ...
>> 
>> Regarding this:
>> - Does it makes sense to allow same test names with different cfgs?
>
> Yes-ish. I think it definitely makes sense for the same test to be 
> redefined with different cfgs, but I'd rather only one of those tests 
> then actually be compiled in (see below).
>
>> - Is it ok to 'stringify' the configuration so it can be distinguished
>>    in the report? Would you prefer something like `_case_1` `_case_2` ..
>>    instead?
>
> I don't _like_ this: my preference would be for us to keep the same 
> name, and just not emit a test_case for anything which should be 
> compiled out with cfg. Unfortunately, implementing that is a bit harder 
> than would be ideal: we need a way of evaluating the cfg() arguments in 
> a proc macro, I think. (Ultimately, because otherwise there's no way of 
> statically determining the length of the TEST_CASES array?)
>
> Unless you've got a good idea how to fix this, though, I'm happy to put 
> up with adding the configs to the name for now. Though if there's a nice 
> way to make the names shorter 
> (rust_test_kunit_parse_cfg_in_kunit_test_cfg_config_rust_kunit_selftest_equals_n 
> is definitely too long a test name, for instance), that'd be best.

I did not like it either but I could not find a way of evaluating the
correspondig cfgs and not including the ones that were not active in the
TEST_CASES array. I implemented the Gary's solution [2] (very neat
trick!) and it worked really well!

Again, thank you both for the feedback. I'll be sending a patch soon.

Best regards,
Nicolás

[1] https://github.com/Rust-for-Linux/linux/blob/93f51579e7df248780214094418f205253383cc5/rust/kernel/lib.rs#L179
[2] https://lore.kernel.org/rust-for-linux/DLLYGLVEA0R3.3D1733XFFTFPV@garyguo.net/
Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by Gary Guo 2 days, 16 hours ago
On Tue Sep 22, 2026 at 8:56 AM BST, David Gow wrote:
> Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
>> - Is it ok to 'stringify' the configuration so it can be distinguished
>>    in the report? Would you prefer something like `_case_1` `_case_2` ..
>>    instead?

I also don't like the stringifcation of cfgs.

>
> I don't _like_ this: my preference would be for us to keep the same 
> name, and just not emit a test_case for anything which should be 
> compiled out with cfg. Unfortunately, implementing that is a bit harder 
> than would be ideal: we need a way of evaluating the cfg() arguments in 
> a proc macro, I think. (Ultimately, because otherwise there's no way of 
> statically determining the length of the TEST_CASES array?)

This is possible with a trick. In pin-init we have a similar need, so what I do
is for

    #[macro]
    struct Foo {
        #[cfg(a)]
        bar: u32,
    }

to be expanded to

    #[cfg(a)]
    #[macro]
    struct Foo {
        bar: u32
    }

    #[cfg(not(a))]
    #[macro]
    struct Foo {
    }

However, for kunit I don't think that's needed. Deduplicating the names
should be sufficient?

Best,
Gary

>
> Unless you've got a good idea how to fix this, though, I'm happy to put 
> up with adding the configs to the name for now. Though if there's a nice 
> way to make the names shorter 
> (rust_test_kunit_parse_cfg_in_kunit_test_cfg_config_rust_kunit_selftest_equals_n 
> is definitely too long a test name, for instance), that'd be best.
Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by David Gow 2 days, 14 hours ago
Le 22/09/2026 à 21:35, Gary Guo a écrit :
> On Tue Sep 22, 2026 at 8:56 AM BST, David Gow wrote:
>> Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
>>> - Is it ok to 'stringify' the configuration so it can be distinguished
>>>     in the report? Would you prefer something like `_case_1` `_case_2` ..
>>>     instead?
> 
> I also don't like the stringifcation of cfgs.
> 
>>
>> I don't _like_ this: my preference would be for us to keep the same
>> name, and just not emit a test_case for anything which should be
>> compiled out with cfg. Unfortunately, implementing that is a bit harder
>> than would be ideal: we need a way of evaluating the cfg() arguments in
>> a proc macro, I think. (Ultimately, because otherwise there's no way of
>> statically determining the length of the TEST_CASES array?)
> 
> This is possible with a trick. In pin-init we have a similar need, so what I do
> is for
> 
>      #[macro]
>      struct Foo {
>          #[cfg(a)]
>          bar: u32,
>      }
> 
> to be expanded to
> 
>      #[cfg(a)]
>      #[macro]
>      struct Foo {
>          bar: u32
>      }
> 
>      #[cfg(not(a))]
>      #[macro]
>      struct Foo {
>      }
> 
> However, for kunit I don't think that's needed. Deduplicating the names
> should be sufficient?
> 
The problem with (at least my naive implementation of) duplication is 
that -- while it works great for switching between implementations -- it 
doesn't handle the case where _no_ implementation is active.

(The current implementation just compiles to a skipped test if the 
#[cfg(...)] isn't active, which sidesteps the problem until we have 
multiple implementations...)

Even the expansion above could be problematic, as we really are trying 
to add entries to a static array, so I don't know what we could put in 
the not(a) case (particularly since there'd be potentially lots of them).

Maybe the trick is to generate the array size by using a big series of 
something like:
static mut TEST_CASES: [...,
	#[cfg(a)] 1
	#[cfg(not(a))]0
	+
	#[cfg(b)] 1
	#[cfg(not(b))]0
	+
	…] = {
	#[cfg(a)] case1,
	#[cfg(b)] case1,
	…
	}
}

Then, as long as there's only one or zero active configurations, the 
array size should match, and any duplicates will be caught by having 
multiple definitions of case1.

I assume the compiler would be able to reduce that down to a 
compile-time integer, even if it is extremely ugly...

-- David
Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by Gary Guo 2 days, 14 hours ago
On Tue Sep 22, 2026 at 4:27 PM BST, David Gow wrote:
> Le 22/09/2026 à 21:35, Gary Guo a écrit :
>> On Tue Sep 22, 2026 at 8:56 AM BST, David Gow wrote:
>>> Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
>>>> - Is it ok to 'stringify' the configuration so it can be distinguished
>>>>     in the report? Would you prefer something like `_case_1` `_case_2` ..
>>>>     instead?
>> 
>> I also don't like the stringifcation of cfgs.
>> 
>>>
>>> I don't _like_ this: my preference would be for us to keep the same
>>> name, and just not emit a test_case for anything which should be
>>> compiled out with cfg. Unfortunately, implementing that is a bit harder
>>> than would be ideal: we need a way of evaluating the cfg() arguments in
>>> a proc macro, I think. (Ultimately, because otherwise there's no way of
>>> statically determining the length of the TEST_CASES array?)
>> 
>> This is possible with a trick. In pin-init we have a similar need, so what I do
>> is for
>> 
>>      #[macro]
>>      struct Foo {
>>          #[cfg(a)]
>>          bar: u32,
>>      }
>> 
>> to be expanded to
>> 
>>      #[cfg(a)]
>>      #[macro]
>>      struct Foo {
>>          bar: u32
>>      }
>> 
>>      #[cfg(not(a))]
>>      #[macro]
>>      struct Foo {
>>      }
>> 
>> However, for kunit I don't think that's needed. Deduplicating the names
>> should be sufficient?
>> 
> The problem with (at least my naive implementation of) duplication is 
> that -- while it works great for switching between implementations -- it 
> doesn't handle the case where _no_ implementation is active.
>
> (The current implementation just compiles to a skipped test if the 
> #[cfg(...)] isn't active, which sidesteps the problem until we have 
> multiple implementations...)
>
> Even the expansion above could be problematic, as we really are trying 
> to add entries to a static array, so I don't know what we could put in 
> the not(a) case (particularly since there'd be potentially lots of them).
>
> Maybe the trick is to generate the array size by using a big series of 
> something like:
> static mut TEST_CASES: [...,
> 	#[cfg(a)] 1
> 	#[cfg(not(a))]0
> 	+
> 	#[cfg(b)] 1
> 	#[cfg(not(b))]0
> 	+
> 	…] = {
> 	#[cfg(a)] case1,
> 	#[cfg(b)] case1,
> 	…
> 	}
> }
>

For array sizes, you have the option of building a slice first.

Some thing like:

    const TEST_CASES_UNIT: &[()] = [
        #[cfg(a)] (),
        #[cfg(b)] (),
    ];

    static mut TEST_CASES: [...; TEST_CASES_SLICE.len()] = [...];

You could also just build everything as a const slice of `&'static
[kunit_cases]`, if there is no need to make it `mut`. But I suppose it needs to
be `static mut` for some reason?

Best,
Gary


> Then, as long as there's only one or zero active configurations, the 
> array size should match, and any duplicates will be caught by having 
> multiple definitions of case1.
>
> I assume the compiler would be able to reduce that down to a 
> compile-time integer, even if it is extremely ugly...
>
> -- David
Re: [PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support
Posted by David Gow 2 days, 14 hours ago
Le 22/09/2026 à 23:37, Gary Guo a écrit :
> On Tue Sep 22, 2026 at 4:27 PM BST, David Gow wrote:
>> Le 22/09/2026 à 21:35, Gary Guo a écrit :
>>> On Tue Sep 22, 2026 at 8:56 AM BST, David Gow wrote:
>>>> Le 16/09/2026 à 03:33, Nicolás Antinori a écrit :
>>>>> - Is it ok to 'stringify' the configuration so it can be distinguished
>>>>>      in the report? Would you prefer something like `_case_1` `_case_2` ..
>>>>>      instead?
>>>
>>> I also don't like the stringifcation of cfgs.
>>>
>>>>
>>>> I don't _like_ this: my preference would be for us to keep the same
>>>> name, and just not emit a test_case for anything which should be
>>>> compiled out with cfg. Unfortunately, implementing that is a bit harder
>>>> than would be ideal: we need a way of evaluating the cfg() arguments in
>>>> a proc macro, I think. (Ultimately, because otherwise there's no way of
>>>> statically determining the length of the TEST_CASES array?)
>>>
>>> This is possible with a trick. In pin-init we have a similar need, so what I do
>>> is for
>>>
>>>       #[macro]
>>>       struct Foo {
>>>           #[cfg(a)]
>>>           bar: u32,
>>>       }
>>>
>>> to be expanded to
>>>
>>>       #[cfg(a)]
>>>       #[macro]
>>>       struct Foo {
>>>           bar: u32
>>>       }
>>>
>>>       #[cfg(not(a))]
>>>       #[macro]
>>>       struct Foo {
>>>       }
>>>
>>> However, for kunit I don't think that's needed. Deduplicating the names
>>> should be sufficient?
>>>
>> The problem with (at least my naive implementation of) duplication is
>> that -- while it works great for switching between implementations -- it
>> doesn't handle the case where _no_ implementation is active.
>>
>> (The current implementation just compiles to a skipped test if the
>> #[cfg(...)] isn't active, which sidesteps the problem until we have
>> multiple implementations...)
>>
>> Even the expansion above could be problematic, as we really are trying
>> to add entries to a static array, so I don't know what we could put in
>> the not(a) case (particularly since there'd be potentially lots of them).
>>
>> Maybe the trick is to generate the array size by using a big series of
>> something like:
>> static mut TEST_CASES: [...,
>> 	#[cfg(a)] 1
>> 	#[cfg(not(a))]0
>> 	+
>> 	#[cfg(b)] 1
>> 	#[cfg(not(b))]0
>> 	+
>> 	…] = {
>> 	#[cfg(a)] case1,
>> 	#[cfg(b)] case1,
>> 	…
>> 	}
>> }
>>
> 
> For array sizes, you have the option of building a slice first.
> 
> Some thing like:
> 
>      const TEST_CASES_UNIT: &[()] = [
>          #[cfg(a)] (),
>          #[cfg(b)] (),
>      ];
> 
>      static mut TEST_CASES: [...; TEST_CASES_SLICE.len()] = [...];
> 

Neat: I hadn't thought of that, and it seems to work great.


> You could also just build everything as a const slice of `&'static
> [kunit_cases]`, if there is no need to make it `mut`. But I suppose it needs to
> be `static mut` for some reason?
> 
Yeah, the kunit_cases are modified at runtime to store the result, so 
this really does need to be `static mut`. And while these writes are all 
done behind the scenes from C, so they should _appear_ constant from 
Rust (modulo a couple of writes to status we can get rid of once we fix 
the cfg() stuff here), we still need to ensure they can't end up in 
read-only memory.

Cheers,
-- David