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