On Mon, 5 May 2025 at 05:54, Miguel Ojeda <miguel.ojeda.sando...@gmail.com> wrote: > > On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tam...@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
smime.p7s
Description: S/MIME Cryptographic Signature
