mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Nicolás Antinori" <nico.antinori.7@gmail.com>
To: "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"David Gow" <david@davidgow.net>, "Gary Guo" <gary@garyguo.net>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Burak Emir" <burak.emir@gmail.com>,
	"Rae Moar" <raemoar63@gmail.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Yury Norov" <yury.norov@gmail.com>,
	kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v2 1/3] rust: kunit: add #[should_panic] support
Date: Mon, 02 Nov 2026 17:53:05 -0300	[thread overview]
Message-ID: <DML0UIHJNE2R.3JDG4HTEN6U0S@gmail.com> (raw)
In-Reply-To: <CANiq72n3YfMpWndR06B0s1Mj_PjAR8MJ+0nP4XXPkaFh3zX7pA@mail.gmail.com>

On Mon Nov 2, 2026 at 12:51 PM -03, Miguel Ojeda wrote:
> On Mon, Nov 2, 2026 at 3:58 PM Nicolás Antinori
> <nico.antinori.7@gmail.com> wrote:
>>
>> To solve this I created a new config "RUST_KUNIT_SHOULD_PANIC_ATTRIBUTE"
>> that depends on !UML && !PANIC_ON_OOPS && BUG. If the test is annotated
>> with `#[should_panic]` and CONFIG_RUST_KUNIT_SHOULD_PANIC_ATTRIBUTE=n,
>> the test won't run and won't show up in the summary.
>
> I was wondering if we needed the flexibility, i.e. if we could just
> simply disable them when the conditions are not satisfied, but I guess
> given `CONFIG_KUNIT_FAULT_TEST`'s existing description, it may make
> sense to have a new Kconfig symbol 

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.

> -- do the ones added here look
> similar in the kernel log to those fault ones?

Yes! they are pretty similar, for `KUNIT_FAULT_TEST`:

KTAP version 1
1..1
    KTAP version 1
    # Subtest: kunit_fault
    # module: kunit_test
    1..1
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 0 P4D 0
Oops: Oops: 0002 [#1] SMP NOPTI
CPU: 0 UID: 0 PID: 30 Comm: kunit_try_catch Tainted: G                 N   7.3.0-18137-ga2943ba718b4-dirty #357 PREEMPT(lazy)
Tainted: [N]=TEST
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.1 11/11/2019

... <Registers and stack trace> ...

note: kunit_try_catch[30] exited with irqs disabled
    # kunit_test_fault_null_dereference: try faulted: last line seen lib/kunit/kunit-test.c:100
    ok 1 kunit_test_fault_null_dereference
ok 1 kunit_fault

For `RUST_KUNIT_TESTS` (left only one that panics to shrink the log, but
they are all pretty much the same):

KTAP version 1
1..1
    KTAP version 1
    # Subtest: rust_kernel_kunit
    # speed: normal
    1..1
rust_kernel: panicked at ../rust/kernel/kunit.rs:399:9:
This test should panic and pass
------------[ cut here ]------------
kernel BUG at rust/helpers/bug.c:7!
Oops: invalid opcode: 0000 [#1] SMP NOPTI
CPU: 0 UID: 0 PID: 29 Comm: kunit_try_catch Tainted: G                 N   7.3.0-18137-ga2943ba718b4-dirty #357 PREEMPT(lazy)
Tainted: [N]=TEST
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.1 11/11/2019

... <Registers and stack trace> ...

    # rust_test_kunit_panic_in_kunit_test.speed: normal
    ok 1 rust_test_kunit_panic_in_kunit_test
ok 1 rust_kernel_kunit

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)
}

That logs:

KTAP version 1
1..1
    KTAP version 1
    # Subtest: rust_kernel_kunit
    # speed: normal
    1..1
    # rust_test_assert_false_success_with_should_panic.speed: normal
    ok 1 rust_test_assert_false_success_with_should_panic
ok 1 rust_kernel_kunit

Because a BUG() is never called, the failed assertion is catched in the
`kunit_assert!` macro and, if the test should panic, the KUnit thread is
aborted without throwing a failed asssertion.

>
> (And I was wondering because, in general, adding Kconfig symbols has a
> price: users may get confused etc.).

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`).

If this is prone to causing confusion, I think I am going to check the
contraints direclty in the code. 

Thank you for the feedback!

>
> Cheers,
> Miguel


  reply	other threads:[~2026-11-02 20:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 16:59 [PATCH v2 0/3] rust: kunit: support #[should_panic] and same test name with different #[cfg(...)] Nicolás Antinori via B4 Relay
2026-10-29 16:59 ` [PATCH v2 1/3] rust: kunit: add #[should_panic] support Nicolás Antinori via B4 Relay
2026-11-02 14:58   ` Nicolás Antinori
2026-11-02 15:51     ` Miguel Ojeda
2026-11-02 20:53       ` Nicolás Antinori [this message]
2026-11-02 21:25         ` Miguel Ojeda
2026-10-29 16:59 ` [PATCH v2 2/3] rust: kunit: allow same test name with different #[cfg(...)] Nicolás Antinori via B4 Relay
2026-10-29 16:59 ` [PATCH v2 3/3] rust: bitmap: kunit: uncomment owned_bitmap_out_of_bounds panic case Nicolás Antinori via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DML0UIHJNE2R.3JDG4HTEN6U0S@gmail.com \
    --to=nico.antinori.7@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=burak.emir@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=david@davidgow.net \
    --cc=gary@garyguo.net \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=raemoar63@gmail.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®