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(-)
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
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
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/
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.
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
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
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
© 2016 - 2026 Red Hat, Inc.