| Message ID | 20250502215133.1923676-6-ojeda@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Rust KUnit `#[test]` support improvements | expand |
On Fri, May 2, 2025 at 5:54 PM Miguel Ojeda <ojeda@kernel.org> wrote: > > Since now we have support for returning `-> Result`s, we can convert some > of these tests to use the feature, and serve as a first user for it too. > > Thus convert them. > > This, in turn, simplifies them a fair bit. > > We keep the actual assertions we want to make as explicit ones with > `assert*!`s. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> 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. > --- > rust/kernel/str.rs | 68 ++++++++++++++++++++-------------------------- > 1 file changed, 30 insertions(+), 38 deletions(-) > > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs > index cf2caa2db168..8dcfb11013f2 100644 > --- a/rust/kernel/str.rs > +++ b/rust/kernel/str.rs > @@ -576,25 +576,9 @@ macro_rules! c_str { > mod tests { > use super::*; > > - struct String(CString); > - > - impl String { > - fn from_fmt(args: fmt::Arguments<'_>) -> Self { > - String(CString::try_from_fmt(args).unwrap()) > - } > - } > - > - impl Deref for String { > - type Target = str; > - > - fn deref(&self) -> &str { > - self.0.to_str().unwrap() > - } > - } These changes don't depend on returning `Result` from the tests AFAICT. Can they be in a separate patch? > - > macro_rules! format { > ($($f:tt)*) => ({ > - &*String::from_fmt(kernel::fmt!($($f)*)) > + CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()? > }) > } > > @@ -613,66 +597,72 @@ macro_rules! format { > \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff"; > > #[test] > - fn test_cstr_to_str() { > + fn test_cstr_to_str() -> Result { > let good_bytes = b"\xf0\x9f\xa6\x80\0"; > - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); > - let checked_str = checked_cstr.to_str().unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; > + let checked_str = checked_cstr.to_str()?; > assert_eq!(checked_str, "🦀"); > + Ok(()) > } > > #[test] > - fn test_cstr_to_str_invalid_utf8() { > + fn test_cstr_to_str_invalid_utf8() -> Result { > let bad_bytes = b"\xc3\x28\0"; > - let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?; > assert!(checked_cstr.to_str().is_err()); > + Ok(()) > } > > #[test] > - fn test_cstr_as_str_unchecked() { > + fn test_cstr_as_str_unchecked() -> Result { > let good_bytes = b"\xf0\x9f\x90\xA7\0"; > - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; > // SAFETY: The contents come from a string literal which contains valid UTF-8. > let unchecked_str = unsafe { checked_cstr.as_str_unchecked() }; > assert_eq!(unchecked_str, "🐧"); > + Ok(()) > } > > #[test] > - fn test_cstr_display() { > - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); > + fn test_cstr_display() -> Result { > + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; > assert_eq!(format!("{}", hello_world), "hello, world!"); > - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); > + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; > assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a"); > - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); > + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; > assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); > - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); > + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; > assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); > + Ok(()) > } > > #[test] > - fn test_cstr_display_all_bytes() { > + fn test_cstr_display_all_bytes() -> Result { > let mut bytes: [u8; 256] = [0; 256]; > // fill `bytes` with [1..=255] + [0] > for i in u8::MIN..=u8::MAX { > bytes[i as usize] = i.wrapping_add(1); > } > - let cstr = CStr::from_bytes_with_nul(&bytes).unwrap(); > + let cstr = CStr::from_bytes_with_nul(&bytes)?; > assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS); > + Ok(()) > } > > #[test] > - fn test_cstr_debug() { > - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); > + fn test_cstr_debug() -> Result { > + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; > assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); > - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); > + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; > assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\""); > - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); > + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; > assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); > - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); > + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; > assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); > + Ok(()) > } > > #[test] > - fn test_bstr_display() { > + fn test_bstr_display() -> Result { > let hello_world = BStr::from_bytes(b"hello, world!"); > assert_eq!(format!("{}", hello_world), "hello, world!"); > let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); > @@ -683,10 +673,11 @@ fn test_bstr_display() { > assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); > let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); > assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); > + Ok(()) > } > > #[test] > - fn test_bstr_debug() { > + fn test_bstr_debug() -> Result { > let hello_world = BStr::from_bytes(b"hello, world!"); > assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); > let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); > @@ -697,6 +688,7 @@ fn test_bstr_debug() { > assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); > let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); > assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); > + Ok(()) > } > } > > -- > 2.49.0 > >
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. > 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. Thanks! Cheers, Miguel
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
On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tamird@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 custom `?` is quite tempting, to get the best of both worlds, assuming we could make it work well. > 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? Sure, the line would need to change again later, but that is fine too, we can do a separate commit. Cheers, Miguel
On Sat, 3 May 2025 at 05:52, Miguel Ojeda <ojeda@kernel.org> wrote: > > Since now we have support for returning `-> Result`s, we can convert some > of these tests to use the feature, and serve as a first user for it too. > > Thus convert them. > > This, in turn, simplifies them a fair bit. > > We keep the actual assertions we want to make as explicit ones with > `assert*!`s. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > --- I prefer this, even though I actually don't mind the .unwrap(), just because we'll funnel failures to KUnit properly, rather than having a full panic from the unwrap. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > rust/kernel/str.rs | 68 ++++++++++++++++++++-------------------------- > 1 file changed, 30 insertions(+), 38 deletions(-) > > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs > index cf2caa2db168..8dcfb11013f2 100644 > --- a/rust/kernel/str.rs > +++ b/rust/kernel/str.rs > @@ -576,25 +576,9 @@ macro_rules! c_str { > mod tests { > use super::*; > > - struct String(CString); > - > - impl String { > - fn from_fmt(args: fmt::Arguments<'_>) -> Self { > - String(CString::try_from_fmt(args).unwrap()) > - } > - } > - > - impl Deref for String { > - type Target = str; > - > - fn deref(&self) -> &str { > - self.0.to_str().unwrap() > - } > - } > - > macro_rules! format { > ($($f:tt)*) => ({ > - &*String::from_fmt(kernel::fmt!($($f)*)) > + CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()? > }) > } > > @@ -613,66 +597,72 @@ macro_rules! format { > \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff"; > > #[test] > - fn test_cstr_to_str() { > + fn test_cstr_to_str() -> Result { > let good_bytes = b"\xf0\x9f\xa6\x80\0"; > - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); > - let checked_str = checked_cstr.to_str().unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; > + let checked_str = checked_cstr.to_str()?; > assert_eq!(checked_str, "🦀"); > + Ok(()) > } > > #[test] > - fn test_cstr_to_str_invalid_utf8() { > + fn test_cstr_to_str_invalid_utf8() -> Result { > let bad_bytes = b"\xc3\x28\0"; > - let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?; > assert!(checked_cstr.to_str().is_err()); > + Ok(()) > } > > #[test] > - fn test_cstr_as_str_unchecked() { > + fn test_cstr_as_str_unchecked() -> Result { > let good_bytes = b"\xf0\x9f\x90\xA7\0"; > - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); > + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; > // SAFETY: The contents come from a string literal which contains valid UTF-8. > let unchecked_str = unsafe { checked_cstr.as_str_unchecked() }; > assert_eq!(unchecked_str, "🐧"); > + Ok(()) > } > > #[test] > - fn test_cstr_display() { > - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); > + fn test_cstr_display() -> Result { > + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; > assert_eq!(format!("{}", hello_world), "hello, world!"); > - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); > + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; > assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a"); > - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); > + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; > assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); > - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); > + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; > assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); > + Ok(()) > } > > #[test] > - fn test_cstr_display_all_bytes() { > + fn test_cstr_display_all_bytes() -> Result { > let mut bytes: [u8; 256] = [0; 256]; > // fill `bytes` with [1..=255] + [0] > for i in u8::MIN..=u8::MAX { > bytes[i as usize] = i.wrapping_add(1); > } > - let cstr = CStr::from_bytes_with_nul(&bytes).unwrap(); > + let cstr = CStr::from_bytes_with_nul(&bytes)?; > assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS); > + Ok(()) > } > > #[test] > - fn test_cstr_debug() { > - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); > + fn test_cstr_debug() -> Result { > + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; > assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); > - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); > + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; > assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\""); > - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); > + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; > assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); > - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); > + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; > assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); > + Ok(()) > } > > #[test] > - fn test_bstr_display() { > + fn test_bstr_display() -> Result { > let hello_world = BStr::from_bytes(b"hello, world!"); > assert_eq!(format!("{}", hello_world), "hello, world!"); > let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); > @@ -683,10 +673,11 @@ fn test_bstr_display() { > assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); > let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); > assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); > + Ok(()) > } > > #[test] > - fn test_bstr_debug() { > + fn test_bstr_debug() -> Result { > let hello_world = BStr::from_bytes(b"hello, world!"); > assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); > let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); > @@ -697,6 +688,7 @@ fn test_bstr_debug() { > assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); > let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); > assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); > + Ok(()) > } > } > > -- > 2.49.0 >
On Mon, 5 May 2025 at 05:54, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sun, May 4, 2025 at 8:23 PM Tamir Duberstein <tamird@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
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index cf2caa2db168..8dcfb11013f2 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -576,25 +576,9 @@ macro_rules! c_str { mod tests { use super::*; - struct String(CString); - - impl String { - fn from_fmt(args: fmt::Arguments<'_>) -> Self { - String(CString::try_from_fmt(args).unwrap()) - } - } - - impl Deref for String { - type Target = str; - - fn deref(&self) -> &str { - self.0.to_str().unwrap() - } - } - macro_rules! format { ($($f:tt)*) => ({ - &*String::from_fmt(kernel::fmt!($($f)*)) + CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()? }) } @@ -613,66 +597,72 @@ macro_rules! format { \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff"; #[test] - fn test_cstr_to_str() { + fn test_cstr_to_str() -> Result { let good_bytes = b"\xf0\x9f\xa6\x80\0"; - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); - let checked_str = checked_cstr.to_str().unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; + let checked_str = checked_cstr.to_str()?; assert_eq!(checked_str, "🦀"); + Ok(()) } #[test] - fn test_cstr_to_str_invalid_utf8() { + fn test_cstr_to_str_invalid_utf8() -> Result { let bad_bytes = b"\xc3\x28\0"; - let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?; assert!(checked_cstr.to_str().is_err()); + Ok(()) } #[test] - fn test_cstr_as_str_unchecked() { + fn test_cstr_as_str_unchecked() -> Result { let good_bytes = b"\xf0\x9f\x90\xA7\0"; - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; // SAFETY: The contents come from a string literal which contains valid UTF-8. let unchecked_str = unsafe { checked_cstr.as_str_unchecked() }; assert_eq!(unchecked_str, "🐧"); + Ok(()) } #[test] - fn test_cstr_display() { - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + fn test_cstr_display() -> Result { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; assert_eq!(format!("{}", hello_world), "hello, world!"); - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a"); - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); + Ok(()) } #[test] - fn test_cstr_display_all_bytes() { + fn test_cstr_display_all_bytes() -> Result { let mut bytes: [u8; 256] = [0; 256]; // fill `bytes` with [1..=255] + [0] for i in u8::MIN..=u8::MAX { bytes[i as usize] = i.wrapping_add(1); } - let cstr = CStr::from_bytes_with_nul(&bytes).unwrap(); + let cstr = CStr::from_bytes_with_nul(&bytes)?; assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS); + Ok(()) } #[test] - fn test_cstr_debug() { - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + fn test_cstr_debug() -> Result { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\""); - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); + Ok(()) } #[test] - fn test_bstr_display() { + fn test_bstr_display() -> Result { let hello_world = BStr::from_bytes(b"hello, world!"); assert_eq!(format!("{}", hello_world), "hello, world!"); let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); @@ -683,10 +673,11 @@ fn test_bstr_display() { assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); + Ok(()) } #[test] - fn test_bstr_debug() { + fn test_bstr_debug() -> Result { let hello_world = BStr::from_bytes(b"hello, world!"); assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); @@ -697,6 +688,7 @@ fn test_bstr_debug() { assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); + Ok(()) } }
Since now we have support for returning `-> Result`s, we can convert some of these tests to use the feature, and serve as a first user for it too. Thus convert them. This, in turn, simplifies them a fair bit. We keep the actual assertions we want to make as explicit ones with `assert*!`s. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> --- rust/kernel/str.rs | 68 ++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 38 deletions(-)