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

[David Gow <david@davidgow.net>]

Le 04/11/2026 à 12:32 AM, Miguel Ojeda a écrit :
> On Tue, Nov 3, 2026 at 3:46 PM David Gow <david@davidgow.net> wrote:
>>
>> I'd rather this not pass, personally. I think that an assert!() within a
>> test should work normally (i.e., it's possible to assert that the test
>> state is normal, and a failing assert will fail the test).
>>
>> I could be talked out of this if it's really unpleasantly different from
>> non-kernel Rust, but in that case I think we'd want to make sure that
>> kunit_assert!() within a test behaves like a normal assert even in a
>> should_panic test.
> 
> Yeah, it all boils down to what we mean by `assert!()`.
> 
> When I implemented it back then, I intended to allow people to write
> the normal asserts they would write (which could help in case we were
> to e.g. import code from elsewhere, like a vendored library), as if
> would have "Rust-panicked", but not "kernel-panicked".
> 
> Viewed in that light, `BUG()` would not necessarily need to make
> `should_panic` succeed or not [*], but an `assert!(false)` would need
> to make the test succeed. And then we could have a
> `should_kernel_panic` and maybe expose a public `kunit_assert!` etc.
> to distinguish them and so on.

I'm pretty convinced that #[should_panic] should fail for anything other
than a deliberate panic!() (including panic!() implemented via an
assertion). Another failure (be it BUG(), an invalid memory access,
etc.) feels like a separate thing. The example I'd give is a test which
is deliberately trying to trigger a panic!() or assert!() to verify
that, e.g., an invalid input/state is caught: if the code tried to
handle the invalid state and crashed, that shouldn't be a pass.


For assert!(false), while I personally don't like that this will cause a
test to pass -- it feels confusing to me -- I'm okay with having this
work for compatibility with upstream Rust.

How about:
- If #[should_panic] is set, we don't override assert!()/assert_eq!():
instead of getting compiled down to a KUnit assertion, they use the core
implementation which will panic!() on failure, and get caught by the
panic_handler. This should match upstream Rust behaviour (because it is
upstream Rust behaviour), and avoid having to plumb a bunch of
should_panic things through the C assert macro implementations.
- If a test really wants a KUnit assertion, the option of using
kernel::kunit_assert!() is there, which will behave like the C
assertions: a failed assertion will cause the test to fail. Of course, I
don't think we need to implement this until someone actually needs it.

> Having said that, I am not sure how many tests would actually rely on
> such differences... I think the core requirement is that, as you say,
> we are able to test for Rust panics in functions we call within the
> test (and ideally also other Rust panics within the test body too,
> when viewed in the light above). But if something simplifies
> implementation significantly, then we can consider other things.

Yeah, I'm hesitant to do something which involves either a lot of
plumbing in C, or a significant difference in behaviour between tests
written in C and Rust unless we've got a real use-case here.

Disabling the assert{,_eq}!() overrides for tests with #[should_panic]
seems to be the appropriate compromise to me, though.

> [*] I asked an LLM to test what happens in userspace Rust when one
> aborts/traps/etc., and it differs in stable between a normal test and
> a doctest (with the default `panic=unwind`): essentially, the former
> dies, the latter succeeds -- it doesn't seem to be well-defined yet.
> There is an issue for the doctest side of it:
> 
>   https://github.com/rust-lang/rust/issues/143009
> 
> and there is a bunch of discussion on the different linked PRs.
> 
Thanks,
-- David