mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Nicolás Antinori" <nico.antinori.7@gmail.com>
To: "David Gow" <david@davidgow.net>,
	"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>,
	"Gary Guo" <gary@garyguo.net>, "Miguel Ojeda" <ojeda@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>
Cc: "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 0/3] rust: kunit: support #[should_panic] and same test name with different #[cfg(...)]
Date: Thu, 05 Nov 2026 10:46:34 -0300	[thread overview]
Message-ID: <DMNBNKKPHI0H.EZQ7YA2CSMEA@gmail.com> (raw)
In-Reply-To: <35284b5b-bf0c-4585-ad3e-a76bae4be7df@davidgow.net>

On Tue Nov 3, 2026 at 11:45 AM -03, David Gow wrote:
> Thanks very much, Nicolás.
>
> I've had a closer look at the new version of this, and have a few 
> comments. Ultimately, I think a mix of this version and the original RFC 
> is probably best, and in particular, using a panic_handler instead of 
> modifying kunit_try_catch, and removing the special-case assert!() 
> handling would be ideal.
>
> Le 30/10/2026 à 00:59, Nicolás Antinori via B4 Relay 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].
>> 
>> Changelog:
>> 
>> v2:
>> - Added more tests in `rust/kernel/kunit.rs` testing Sashiko findings
>>    and changes.
>> - Solved Sashiko: "[Low] kunit: spurious "try faulted" error logs for
>>    successful #[should_panic] tests": Solved by adding
>>    `rust_should_panic` in `kunit_try_catch` C struct to check if the test
>>    is expected to panic and act accordingly. Dismissed what I first talked
>>    about in [3].
>
> I think if we only handle Rust panics, this should go away, right?

Yes, it should. Since we won't `BUG()` anymore in case a test should
panic, the test thread won't exit with `-EINTR`. Because of this we
won't need to shield the feature behind a Kconfig symbol either, since
we won't have error logs in successful tests.

Here's the raw output from the panic handler version:

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

for test:

#[test]
#[should_panic]
fn rust_test_kunit_panic_in_kunit_test() {
    panic!("This test should panic and pass");
}

>> - Now the tests that have an inactive #[cfg(..)] do not appear un the
>>    KUnit execution summary as ignored, they just do not run.
>> Link to v1: https://lore.kernel.org/all/20260930-rust_kunit_v4-v1-0-e3b0d45311dd@gmail.com/
>> 
>> - RFC patch: https://lore.kernel.org/rust-for-linux/cover.1789500084.git.nico.antinori.7@gmail.com/
>> 
>> 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 yet).
>> 
>> The way it is supported is by adding a new boolean property called
>> `rust_should_panic` to `kunit_attributes` struct in kunit/test.h and in
>> `kunit_attributes` in kunit/try-catch.h and check for the
>> `try_catch->try_result` code and the named flag to detect if there was a
>> panic in `kunit_try_catch_run` function.
>
> I think rust_should_panic as an attribute is good, but it should just be 
> checked from a panic_handler. If it supports more than just rust panics, 
> it probably shouldn't be called *rust_*should_panic (and again, should 
> probably be based on the warning suppression instead).

If we consider failed `assert!` from outside a #[test] function panics,
then I'll leave it like this, otherwise I am thinking of
`rust_should_fail`.

Thank you!
Nicolás

      reply	other threads:[~2026-11-05 13:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 16:59 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
2026-11-02 21:25         ` Miguel Ojeda
2026-11-03 15:05           ` David Gow
2026-11-03 15:00     ` David Gow
2026-11-03 14:46   ` David Gow
2026-11-03 16:32     ` Miguel Ojeda
2026-11-04 17:58     ` Nicolás Antinori
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-11-03 14:45   ` David Gow
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
2026-11-03 14:46   ` David Gow
2026-11-03 14:45 ` [PATCH v2 0/3] rust: kunit: support #[should_panic] and same test name with different #[cfg(...)] David Gow
2026-11-05 13:46   ` Nicolás Antinori [this message]

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=DMNBNKKPHI0H.EZQ7YA2CSMEA@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=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®