On Sun, May 4, 2025 at 2:15 PM Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sun, May 4, 2025 at 7:30 PM Tamir Duberstein tamird@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