Re: [PATCH v2 1/3] rust: kunit: add #[should_panic] support

[David Gow <david@davidgow.net>]

Le 03/11/2026 à 05:25, Miguel Ojeda a écrit :
> On Mon, Nov 2, 2026 at 9:53 PM Nicolás Antinori
> <nico.antinori.7@gmail.com> wrote:
>>
>> I first thought of disabling `#[should_panic]` tests by directly
>> checking the conditions in the macro code. I ended up creating a new
>> config because I thought it would be cleaner codewise, if a new
>> constraint arises in the future, we just have to add it to the
>> `depends on` clause instead of modifying the macro code.
> 
> It depends on what we want: if it is meant to be configurable, then it
> should be a visible Kconfig symbol; if not, then it should remain an
> implementation detail (which could perhaps be a non-visible Kconfig
> symbol).
> 

The reason KUNIT_FAULT_TEST was user-configurable is that some test 
harnesses / CI systems would automatically mark a test failed if they 
saw a panic/WARN stacktrace in the kernel output.

>> Yes! they are pretty similar, for `KUNIT_FAULT_TEST`:
> 
> Yeah, in that case, I think a new visible Kconfig symbol may be
> justifiable -- please see below.
> 
>> The only one that is different in `RUST_KUNIT_TESTS` is:
>>
>> #[test]
>> #[should_panic]
>> fn rust_test_assert_false_success_with_should_panic() {
>>      assert!(false)
>> }
> 
> I guess it is reasonable to expect this test to pass, since `assert!`
> doesn't return and actually panics in normal Rust.
> 
> Having said that, I would expect our `should_panic` tests to be
> generally about non-`assert*!` panics.
> 

Yeah, I really don't like the way this effectively reverses what 
assert!() does. I'm sort-of okay with it in indirectly-called code (I 
want to check that this function panics if I pass an invalid input, and 
don't care whether it calls assert!() or panic!()), but it's really 
confusing from within the test function.

Plus there's a whole bunch of special-cased code to make kunit_assert!() 
behave this way in this patch, and if we could get rid of that, the 
whole thing would be much cleaner.

(Of course, that introduces an inconsistency depending on where the 
assert!() is, which isn't particularly nice, though I'd be prepared to 
tolerate it to make everything else behave as expected.)

>> On top of the reasons I gave above, I tried to follow the same as
>> `CONFIG_RUST_BUG_KUNIT_TEST` or `CONFIG_KUNIT_FAULT_TEST` are doing, but
>> in this case in particular, the config is not for enabling a test suite
>> but for enabling a feature that can affect multiple suites (in this
>> patch series the ones behind `CONFIG_RUST_KUNIT_TESTS` and
>> `CONFIG_RUST_BITMAP_KUNIT_TESTS`).
> 
> Yeah, it is a "mode" -- that is why originally I thought we would try
> to just do the right thing automatically, i.e. enable them when
> possible, but then when I saw the description of
> `CONFIG_KUNIT_FAULT_TEST`:
> 
>      Enables fault handling tests for the KUnit framework. These tests may
>      trigger a kernel BUG(), and the associated stack trace, even when they
>      pass. If this conflicts with your test infrastrcture (or is confusing
>      or annoying), they can be disabled by setting this to N.
> 
> and I thought it may be fair to have the Kconfig symbol, i.e. it could
> also be annoying to see the traces or similar, and thus it may be that
> someone needs to disable this.
> 
> Having said that, if nobody has asked, we could keep it simple for now
> as well -- that may be fine too.
> 
> David: what do you think? Is it something that was discussed back then
> for that one in C?
I've always thought of CONFIG_KUNIT_FAULT_TEST as being a more general 
switch for tests which can cause faults, but the name and description to 
imply it's more of a switch for just those tests. I'm happy to keep 
CONFIG_KUNIT_FAULT_TEST as the generic implementation here, but if we 
really want another one, they both probably should chain off a single 
CONFIG_HAVE_KUNIT_FAULT_HANDLER or similar.

That being said, I'd prefer we go back to the panic_handler 
implementation for should_panic, as I think we're asking for trouble 
with Rust tests which deliberately panic in C code. Ideally, Rust tests 
will only trigger Rust panics deliberately. The panic_handler-based 
imeplementation works fine on UML.


Cheers,
-- David