All of lore.kernel.org
 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: 24+ 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
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
2026-10-29 16:59   ` 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-06  5:31       ` David Gow
2026-11-04 17:58     ` Nicolás Antinori
2026-11-06  5:31       ` David Gow
2026-10-29 16:59 ` [PATCH v2 2/3] rust: kunit: allow same test name with different #[cfg(...)] Nicolás Antinori
2026-10-29 16:59   ` 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
2026-10-29 16:59   ` 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]
2026-11-06  5:31     ` David Gow

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.