Re: [PATCH 2/7] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s

Miguel Ojeda
2025-05-27T15:22:39Z

On Tue, May 6, 2025 at 8:33 AM David Gow <davidgow@google.com> wrote:
>
> FWIW, having out-of-memory situations trigger a test failure is
> consistent with what other KUnit tests (written in C) do.
>
> There's both advantages and disadvantages to this: on the one hand,
> it's prone to false positives (as you mention), on the other, it
> catches cases where the test is using an unusually large amount of
> memory (which could indeed be a test issues).
>
> My go-to rule is that tests should fail if small allocations (which,
> in the normal course of events, _should_ succeed) fail, but if they
> have unusual resource requirements (beyond "enough memory for the
> system to run normally") these should be checked separately when the
> test starts.
>
> That being said, I definitely think that, by default, an `Err` return
> should map to a FAILED test result, as not all Err Results are a
> resource exhaustion issue, and we definitely don't want to mark a test
> as skipped if there's a real error occurring. If test authors wish to
> skip a test when an out-of-memory condition occurs, they probably
> should handle that explicitly. (But I'd not be opposed to helpers to
> make it easier.)

Yeah, agreed.

There may be value in differentiating errors coming from `?` vs.
`assert!`s -- i.e. the discussion in the other thread. It could even
be a different error state if that was valuable -- though I think over
complicating is not great either.

By the way, before I forget to write this down: I talked to Alice
about this back when this discussion happened and she suggested having
a `Result` that carries extra information -- the location. I was
worried about the added cost of doing that in all cases (i.e. for
every `Result` in the kernel), so I suggested doing that but opt-in,
i.e. with a Kconfig option that would be in e.g. the debug menu. Then
people could typically run their debug kernels and things like KUnit
with it enabled, but production kernels without it. Then later on, if
it proves useful for other things, we could try to figure out ways to
do it without too much cost.

Cheers,
Miguel