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

David Gow
2025-05-05T06:03:25Z

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

On Mon, 5 May 2025 at 05:54, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > I see. Up to you, obviously, but ISTM that this degree of freedom is
> > unnecessary, but perhaps there's a benefit I'm underappreciating?
>
> Well, having this allows one to write code like the rest of the kernel
> code, instead of, say, `.unwrap()`ing or `assert!`ing everywhere.
>
> So easier to read, easier to copy-paste from normal code, and people
> (especially those learning) wouldn't get accustomed to seeing
> `.unwrap()`s etc. everywhere.
>
> Having said that, C KUnit uses the macros for things that require
> stopping the test, even if "unrelated" to the actual test, and it does
> not look like normal code, of course. They do have `->init()` which
> can return a failure, but not the test cases themselves.
>
> David perhaps has some advice here. Perhaps test functions being
> fallible (like returning `int`) were considered (or asserts for
> "unrelated" things) for C at some point and discarded.

The decision to not have a return value for tests predates me (if
Brendan's around, maybe he recalls the original reason here), but I
suspect it was mostly in order to encourage explicit assertions, which
could then contain the line number of the failure.

We use the KUnit assertions for "unrelated" failures as well mainly as
that's the only way to end a test early if it's not entirely contained
in one function. But it's not _wrong_ for a test to mark itself as
failed (or skipped) and then return early if that makes more sense.

> The custom `?` is quite tempting, to get the best of both worlds,
> assuming we could make it work well.

I'm definitely a fan of using '?' over unwrap() here, if only because
it won't trigger an actual panic. That being said, if it turns out to
be easier to have a variant of unwrap(), that'd work, too.

The fact that not all Result Errs implement Debug makes having a nice
way of printing it a bit more fun, too.

-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]