linux-kernel - Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANiq72nEHy5wvOyPzW4DLu9aE_MxTx1gq7AJ-f_Ny4E6bbOJ5g@mail.gmail.com>
Date: Sun, 4 May 2025 20:15:21 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Tamir Duberstein <tamird@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Brendan Higgins <brendan.higgins@...ux.dev>, 
	David Gow <davidgow@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, Rae Moar <rmoar@...gle.com>, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support
 in KUnit `#[test]`'s

On Sun, May 4, 2025 at 7:30 PM Tamir Duberstein <tamird@...il.com> wrote:
>
> Alice pointed this out in another thread but: one of the downsides of
> returning Result is that in the event of failure the line number where
> the error occurred is no longer contained in the test output. I'm 👎
> on this change for that reason.

We could perhaps customize `?` to help here, e.g. printing a trace or
panic, with the `Try` trait or similar.

Related to this: I thought about saying in the guidelines that `?` in
tests is intended for things that you would normally use `?` in
similar kernel code, i.e. things that the test is not "testing",
rather than things that you would want to assert explicitly. Thus the
actual code under test should still have `assert!`s in the right
places. I did that in the sample. That way, having `?` would still
simplify a lot of test code and yet allow us to differentiate between
code under test vs. other code failing.

> These changes don't depend on returning `Result` from the tests
> AFAICT. Can they be in a separate patch?

Not sure what you mean. The change below uses `?`, which is what
allows this to be removed.

Thanks!

Cheers,
Miguel