Support for expected messages on should_panic.

Anders429 · Dec 27, 2024 · 01af280 · 01af280
1 parent 54c16a7
commit 01af280
Show file tree

Hide file tree

Showing 31 changed files with 270 additions and 30 deletions.
diff --git a/.github/workflows/mgba-rom-test.yml b/.github/workflows/mgba-rom-test.yml
@@ -151,6 +151,46 @@ jobs:
         rom-path: should_panic_fail.gba
         success-code: 1  # Fail
 
+  should_panic_message_pass:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: dtolnay/rust-toolchain@nightly
+      with:
+        components: rust-src
+    - run: sudo apt-get install binutils-arm-none-eabi
+    - run: cd tests/should_panic_message_pass && cargo test --no-run --message-format=json | tee results.json
+    - run: echo "ROM_PATH=$(cd tests/parse_executable && cargo run ../should_panic_message_pass/results.json)" >> $GITHUB_ENV
+    - run: arm-none-eabi-objcopy -O binary ${{ env.ROM_PATH }} should_panic_message_pass.gba
+    - run: cargo install gbafix
+    - run: gbafix should_panic_message_pass.gba
+    - uses: felixjones/github-mgba-rom-test@v1
+      with:
+        swi-call: 0x27
+        read-register: 'r0'
+        rom-path: should_panic_message_pass.gba
+        success-code: 0  # Pass
+
+  should_panic_message_fail:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: dtolnay/rust-toolchain@nightly
+      with:
+        components: rust-src
+    - run: sudo apt-get install binutils-arm-none-eabi
+    - run: cd tests/should_panic_message_fail && cargo test --no-run --message-format=json | tee results.json
+    - run: echo "ROM_PATH=$(cd tests/parse_executable && cargo run ../should_panic_message_fail/results.json)" >> $GITHUB_ENV
+    - run: arm-none-eabi-objcopy -O binary ${{ env.ROM_PATH }} should_panic_message_fail.gba
+    - run: cargo install gbafix
+    - run: gbafix should_panic_message_fail.gba
+    - uses: felixjones/github-mgba-rom-test@v1
+      with:
+        swi-call: 0x27
+        read-register: 'r0'
+        rom-path: should_panic_message_fail.gba
+        success-code: 1  # Fail
+
   allocate:
     runs-on: ubuntu-latest
     steps:

diff --git a/README.md b/README.md
@@ -26,7 +26,7 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
 ```
 
 ### Adding `gba_test` to your list of dependencies

diff --git a/gba_test/.cargo/config.toml b/gba_test/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/gba_test/CHANGELOG.md b/gba_test/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 ### Added
 - Support for returning `Result<T, E>` from tests, as well as other custom return types.
+- `ShouldPanic::YesWithMessage(message)` to indicate that a panic message should contain the given substring.
 ### Fixed
 - Scrolling through multiple failed tests no longer breaks due to incorrect pointer alignment arithmetic.
 - Filtered test scrolling will no longer panic due to underflows.

diff --git a/gba_test/src/lib.rs b/gba_test/src/lib.rs
@@ -78,6 +78,7 @@
 #![cfg_attr(doc_cfg, feature(doc_cfg))]
 #![allow(clippy::needless_doctest_main, static_mut_refs)]
 
+extern crate alloc;
 #[cfg(test)]
 extern crate self as gba_test;
 

diff --git a/gba_test/src/runner.rs b/gba_test/src/runner.rs
@@ -8,6 +8,7 @@ use crate::{
     allocator, allocator::Allocator, log, test_case::Ignore, ui, Outcome, ShouldPanic, TestCase,
     Tests,
 };
+use alloc::format;
 use core::{arch::asm, fmt::Display, mem::MaybeUninit, panic::PanicInfo, ptr::addr_of};
 
 // TODO: Make these more type-safe.
@@ -104,6 +105,16 @@ fn panic(info: &PanicInfo) -> ! {
                     log::info!("test passed");
                     store_outcome(Outcome::<&str>::Passed);
                 }
+                ShouldPanic::YesWithMessage(message) => {
+                    let panic_message = format!("{}", info);
+                    if panic_message.contains(message) {
+                        log::info!("test passed");
+                        store_outcome(Outcome::<&str>::Passed);
+                    } else {
+                        log::info!("test failed");
+                        store_outcome(Outcome::Failed(format_args!("panic did not contain expected string\n     panic message: `{panic_message}`\nexpected substring: `{message}`")))
+                    }
+                }
             }
 
             // Soft resetting the system allows us to recover from the panicked state and continue testing.
@@ -187,7 +198,7 @@ pub fn runner(tests: &'static [&'static dyn TestCase]) -> ! {
                         log::info!("test passed");
                         store_outcome(Outcome::<&str>::Passed);
                     }
-                    ShouldPanic::Yes => {
+                    ShouldPanic::Yes | ShouldPanic::YesWithMessage(_) => {
                         log::info!("test failed");
                         store_outcome(Outcome::Failed("note: test did not panic as expected"))
                     }

diff --git a/gba_test/src/test_case.rs b/gba_test/src/test_case.rs
@@ -60,6 +60,8 @@ pub enum ShouldPanic {
     No,
     /// The test is expected to panic during execution.
     Yes,
+    /// The test is expected to panic with the given substring present in the panic message.
+    YesWithMessage(&'static str),
 }
 
 /// Defines a test case executable by the test runner.
@@ -217,7 +219,7 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expectd = "assertion failed: false")]
+    #[should_panic(expected = "assertion failed: false")]
     fn test_run_panic() {
         let test = Test {
             name: "",

diff --git a/gba_test_macros/CHANGELOG.md b/gba_test_macros/CHANGELOG.md
@@ -3,6 +3,9 @@
 ## Unreleased
 ### Added
 - Support for returning `Result<T, E>` from tests, as well as other custom return types.
+- Expected messages for `#[should_panic]` attributes.
+### Fixed
+- `#[ignore]` no longer accepts incorrect arguments.
 
 ## 0.2.0 - 2024-12-24
 ### Fixed

diff --git a/gba_test_macros/src/lib.rs b/gba_test_macros/src/lib.rs
@@ -32,14 +32,16 @@ use proc_macro::TokenStream;
 use proc_macro2::Span;
 use quote::quote;
 use syn::{
-    parse, parse_str, token, Attribute, Error, ExprParen, Ident, ItemFn, Meta, ReturnType, Type,
+    parse, parse2, parse_str, token, Attribute, Error, ExprParen, Ident, ItemFn, Meta, ReturnType,
+    Type,
 };
 
 /// Structured representation of the configuration attributes provided for a test.
 struct Attributes {
     ignore: Ident,
     ignore_message: Option<ExprParen>,
     should_panic: Ident,
+    should_panic_message: Option<ExprParen>,
 }
 
 impl Attributes {
@@ -49,31 +51,68 @@ impl Attributes {
             ignore: Ident::new("No", Span::call_site()),
             ignore_message: None,
             should_panic: Ident::new("No", Span::call_site()),
+            should_panic_message: None,
         }
     }
 }
 
-impl From<&Vec<Attribute>> for Attributes {
-    fn from(attributes: &Vec<Attribute>) -> Self {
+impl TryFrom<&Vec<Attribute>> for Attributes {
+    type Error = Error;
+
+    fn try_from(attributes: &Vec<Attribute>) -> Result<Self, Self::Error> {
         let mut result = Attributes::new();
 
         for attribute in attributes {
             if let Some(ident) = attribute.path().get_ident() {
                 match ident.to_string().as_str() {
                     "ignore" => {
-                        if let Meta::NameValue(name_value) = &attribute.meta {
-                            result.ignore = Ident::new("YesWithMessage", Span::call_site());
-                            result.ignore_message = Some(ExprParen {
-                                attrs: Vec::new(),
-                                paren_token: token::Paren::default(),
-                                expr: Box::new(name_value.value.clone()),
-                            });
-                        } else {
-                            result.ignore = Ident::new("Yes", Span::call_site());
+                        match &attribute.meta {
+                            Meta::NameValue(name_value) => {
+                                result.ignore = Ident::new("YesWithMessage", Span::call_site());
+                                result.ignore_message = Some(ExprParen {
+                                    attrs: Vec::new(),
+                                    paren_token: token::Paren::default(),
+                                    expr: Box::new(name_value.value.clone()),
+                                });
+                            }
+                            Meta::List(_) => return Err(Error::new_spanned(attribute, "valid forms for the attribute are `#[ignore]` and `#[ignore = \"reason\"]`")),
+                            Meta::Path(_) => result.ignore = Ident::new("Yes", Span::call_site()),
                         }
                     }
                     "should_panic" => {
-                        result.should_panic = Ident::new("Yes", Span::call_site());
+                        match &attribute.meta {
+                            Meta::List(meta_list) => {
+                                if let Ok(Meta::NameValue(name_value)) =
+                                    parse2(meta_list.tokens.clone())
+                                {
+                                    if name_value.path == parse_str("expected").unwrap() {
+                                        result.should_panic =
+                                            Ident::new("YesWithMessage", Span::call_site());
+                                        result.should_panic_message = Some(ExprParen {
+                                            attrs: Vec::new(),
+                                            paren_token: token::Paren::default(),
+                                            expr: Box::new(name_value.value),
+                                        });
+                                    } else {
+                                        return Err(Error::new_spanned(attribute, "argument must be of the form: `expected = \"error message\"`"));
+                                    }
+                                } else {
+                                    return Err(Error::new_spanned(attribute, "argument must be of the form: `expected = \"error message\"`"));
+                                }
+                            }
+                            Meta::NameValue(name_value) => {
+                                result.should_panic =
+                                    Ident::new("YesWithMessage", Span::call_site());
+                                result.should_panic_message = Some(ExprParen {
+                                    attrs: Vec::new(),
+                                    paren_token: token::Paren::default(),
+                                    expr: Box::new(name_value.value.clone()),
+                                });
+                            }
+                            Meta::Path(_) => {
+                                result.should_panic = Ident::new("Yes", Span::call_site());
+                            }
+                        }
                     }
                     _ => {
                         // Not supported.
@@ -82,7 +121,7 @@ impl From<&Vec<Attribute>> for Attributes {
             }
         }
 
-        result
+        Ok(result)
     }
 }
 
@@ -128,10 +167,14 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
         ReturnType::Default => parse_str::<Type>("()").unwrap(),
         ReturnType::Type(_, return_type) => *return_type.clone(),
     };
-    let attributes = Attributes::from(&function.attrs);
+    let attributes = match Attributes::try_from(&function.attrs) {
+        Ok(attributes) => attributes,
+        Err(error) => return error.into_compile_error().into(),
+    };
     let ignore = attributes.ignore;
     let ignore_message = attributes.ignore_message;
     let should_panic = attributes.should_panic;
+    let should_panic_message = attributes.should_panic_message;
     if return_type != parse_str::<Type>("()").unwrap()
         && should_panic != Ident::new("No", Span::call_site())
     {
@@ -154,7 +197,7 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream {
             module: module_path!(),
             test: #name,
             ignore: ::gba_test::Ignore::#ignore #ignore_message,
-            should_panic: ::gba_test::ShouldPanic::#should_panic,
+            should_panic: ::gba_test::ShouldPanic::#should_panic #should_panic_message,
         };
     })
 }
diff --git a/gba_test_macros/tests/trybuild/ignore_invalid_arguments.rs b/gba_test_macros/tests/trybuild/ignore_invalid_arguments.rs
@@ -0,0 +1,10 @@
+#![feature(custom_test_frameworks)]
+
+use gba_test_macros::test;
+
+#[test]
+#[ignore("foo")]
+fn foo() {}
+
+
+fn main() {}
diff --git a/gba_test_macros/tests/trybuild/ignore_invalid_arguments.stderr b/gba_test_macros/tests/trybuild/ignore_invalid_arguments.stderr
@@ -0,0 +1,5 @@
+error: valid forms for the attribute are `#[ignore]` and `#[ignore = "reason"]`
+ --> tests/trybuild/ignore_invalid_arguments.rs:6:1
+  |
+6 | #[ignore("foo")]
+  | ^^^^^^^^^^^^^^^^
diff --git a/gba_test_macros/tests/trybuild/should_panic_arguments_not_name_value.rs b/gba_test_macros/tests/trybuild/should_panic_arguments_not_name_value.rs
@@ -0,0 +1,10 @@
+#![feature(custom_test_frameworks)]
+
+use gba_test_macros::test;
+
+#[test]
+#[should_panic("foo")]
+fn foo() {}
+
+
+fn main() {}
diff --git a/gba_test_macros/tests/trybuild/should_panic_arguments_not_name_value.stderr b/gba_test_macros/tests/trybuild/should_panic_arguments_not_name_value.stderr
@@ -0,0 +1,5 @@
+error: argument must be of the form: `expected = "error message"`
+ --> tests/trybuild/should_panic_arguments_not_name_value.rs:6:1
+  |
+6 | #[should_panic("foo")]
+  | ^^^^^^^^^^^^^^^^^^^^^^
diff --git a/gba_test_macros/tests/trybuild/should_panic_incorrect_argument_name.rs b/gba_test_macros/tests/trybuild/should_panic_incorrect_argument_name.rs
@@ -0,0 +1,10 @@
+#![feature(custom_test_frameworks)]
+
+use gba_test_macros::test;
+
+#[test]
+#[should_panic(expectd = "foo")]
+fn foo() {}
+
+
+fn main() {}
diff --git a/gba_test_macros/tests/trybuild/should_panic_incorrect_argument_name.stderr b/gba_test_macros/tests/trybuild/should_panic_incorrect_argument_name.stderr
@@ -0,0 +1,5 @@
+error: argument must be of the form: `expected = "error message"`
+ --> tests/trybuild/should_panic_incorrect_argument_name.rs:6:1
+  |
+6 | #[should_panic(expectd = "foo")]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/fail/.cargo/config.toml b/tests/fail/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/ignore/.cargo/config.toml b/tests/ignore/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/ignore_with_message/.cargo/config.toml b/tests/ignore_with_message/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/multiple/.cargo/config.toml b/tests/multiple/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/multiple_ignore/.cargo/config.toml b/tests/multiple_ignore/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/pass/.cargo/config.toml b/tests/pass/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/result_fail/.cargo/config.toml b/tests/result_fail/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/result_pass/.cargo/config.toml b/tests/result_pass/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/should_panic_fail/.cargo/config.toml b/tests/should_panic_fail/.cargo/config.toml
@@ -6,4 +6,4 @@ runner = "mgba"
 rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
 
 [unstable]
-build-std = ["core"]
+build-std = ["alloc", "core"]
diff --git a/tests/should_panic_message_fail/.cargo/config.toml b/tests/should_panic_message_fail/.cargo/config.toml
@@ -0,0 +1,9 @@
+[build]
+target = "thumbv4t-none-eabi"
+
+[target.thumbv4t-none-eabi]
+runner = "mgba"
+rustflags = ["-Clinker=arm-none-eabi-ld", "-Clink-arg=-Tgba.ld", "-Ztrap-unreachable=no"]
+
+[unstable]
+build-std = ["alloc", "core"]