On Sun, May 4, 2025 at 2:15 PM Miguel Ojeda
<miguel.ojeda.sando...@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 7:30 PM Tamir Duberstein <tam...@gmail.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.
I see. Up to you, obviously, but ISTM that this degree of freedom is
unnecessary, but perhaps there's a benefit I'm underappreciating?
>
> > 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.
Even without this change, couldn't you apply
macro_rules! format {
($($f:tt)*) => ({
- &*String::from_fmt(kernel::fmt!($($f)*))
+
CString::try_from_fmt(kernel::fmt!($($f)*)).unwrap().to_str().unwrap()
})
}
and achieve roughly the same reduction in line count in the test module?
Cheers.
Tamir
