Merge #618

618: Support #[ignore] attribute in defmt_test r=japaric a=justahero

This PR adds support for the `#[ignore]` attribute in `defmt_test` to solve #390. Similar to the [`ignore`](https://doc.rust-lang.org/reference/attributes/testing.html#the-ignore-attribute) attribute, tests can be annotated with the attribute to ignore particular tests.

* use [`trybuild`](https://github.com/dtolnay/trybuild) crate to check multiple conditions with the proc macro

**Note** the message is different from the suggestion in #390 and should be revised accordingly if needed

Co-authored-by: Sebastian Ziebell <sebastian.ziebell@asquera.de>

.github/workflows/ci.yml

@@ -71,6 +71,18 @@ jobs:
       - name: Run rustfmt & clippy
         run: cargo xtask test-lint
 
+  ui:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
+          override: true
+      - name: Install Rust stable, run all UI tests on the host
+        run: cargo xtask test-ui
+
   mdbook:
     strategy:
       matrix:
@@ -143,6 +155,7 @@ jobs:
       - mdbook
       - qemu-snapshot
       - backcompat
+      - ui
     runs-on: ubuntu-20.04
     steps:
       - name: CI succeeded

firmware/defmt-test/macros/Cargo.toml

@@ -16,3 +16,6 @@ proc-macro = true
 proc-macro2 = "1.0.27"
 quote = "1.0.9"
 syn = { version = "1.0.72", features = ["extra-traits", "full"] }
+
+[dev-dependencies]
+trybuild = "1"

firmware/defmt-test/macros/src/lib.rs

@@ -42,6 +42,7 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
             Item::Fn(mut f) => {
                 let mut test_kind = None;
                 let mut should_error = false;
+                let mut ignore = false;
 
                 f.attrs.retain(|attr| {
                     if attr.path.is_ident("init") {
@@ -53,6 +54,9 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
                     } else if attr.path.is_ident("should_error") {
                         should_error = true;
                         false
+                    } else if attr.path.is_ident("ignore") {
+                        ignore = true;
+                        false
                     } else {
                         true
                     }
@@ -84,6 +88,13 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
                             ));
                         }
 
+                        if ignore {
+                            return Err(parse::Error::new(
+                                f.sig.ident.span(),
+                                "`#[ignore]` is not allowed on the `#[init]` function",
+                            ));
+                        }
+
                         if check_fn_sig(&f.sig).is_err() || !f.sig.inputs.is_empty() {
                             return Err(parse::Error::new(
                                 f.sig.ident.span(),
@@ -130,6 +141,7 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
                             func: f,
                             input,
                             should_error,
+                            ignore,
                         })
                     }
                 }
@@ -160,6 +172,7 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
     let mut unit_test_calls = vec![];
     for test in &tests {
         let should_error = test.should_error;
+        let ignore = test.ignore;
         let ident = &test.func.sig.ident;
         let span = test.func.sig.ident.span();
         let call = if let Some(input) = test.input.as_ref() {
@@ -181,9 +194,13 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
         } else {
             quote!(#ident())
         };
-        unit_test_calls.push(quote!(
-            #krate::export::check_outcome(#call, #should_error);
-        ));
+        if ignore {
+            unit_test_calls.push(quote!(let _ = #call;));
+        } else {
+            unit_test_calls.push(quote!(
+                #krate::export::check_outcome(#call, #should_error);
+            ));
+        }
     }
 
     let test_functions = tests.iter().map(|test| &test.func);
@@ -206,7 +223,8 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
         .iter()
         .map(|test| {
             let message = format!(
-                "({{=usize}}/{{=usize}}) running `{}`...",
+                "({{=usize}}/{{=usize}}) {} `{}`...",
+                if test.ignore { "ignoring" } else { "running" },
                 test.func.sig.ident
             );
             quote_spanned! {
@@ -263,6 +281,7 @@ struct Test {
     cfgs: Vec<Attribute>,
     input: Option<Input>,
     should_error: bool,
+    ignore: bool,
 }
 
 struct Input {

firmware/defmt-test/macros/tests/ui.rs

@@ -0,0 +1,5 @@
+#[test]
+fn ui() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/ui/*.rs");
+}

firmware/defmt-test/macros/tests/ui/init-duplicate.rs

@@ -0,0 +1,10 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    fn first() {}
+
+    #[init]
+    fn second() {}
+}

firmware/defmt-test/macros/tests/ui/init-duplicate.stderr

@@ -0,0 +1,5 @@
+error: only a single `#[init]` function can be defined
+ --> tests/ui/init-duplicate.rs:9:8
+  |
+9 |     fn second() {}
+  |        ^^^^^^

firmware/defmt-test/macros/tests/ui/init-has-ignore-macro.rs

@@ -0,0 +1,8 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    #[ignore]
+    fn init() {}
+}

firmware/defmt-test/macros/tests/ui/init-has-ignore-macro.stderr

@@ -0,0 +1,5 @@
+error: `#[ignore]` is not allowed on the `#[init]` function
+ --> tests/ui/init-has-ignore-macro.rs:7:8
+  |
+7 |     fn init() {}
+  |        ^^^^

firmware/defmt-test/macros/tests/ui/init-has-invalid-function-signaturen.rs

@@ -0,0 +1,9 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    fn hello(a: i32, b: i32) -> i32 {
+        a + b
+    }
+}

firmware/defmt-test/macros/tests/ui/init-has-invalid-function-signaturen.stderr

@@ -0,0 +1,5 @@
+error: `#[init]` function must have signature `fn() [-> Type]` (the return type is optional)
+ --> tests/ui/init-has-invalid-function-signaturen.rs:6:8
+  |
+6 |     fn hello(a: i32, b: i32) -> i32 {
+  |        ^^^^^

firmware/defmt-test/macros/tests/ui/init-has-should-error-macro.rs

@@ -0,0 +1,8 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    #[should_error]
+    fn init() {}
+}

firmware/defmt-test/macros/tests/ui/init-has-should-error-macro.stderr

@@ -0,0 +1,5 @@
+error: `#[should_error]` is not allowed on the `#[init]` function
+ --> tests/ui/init-has-should-error-macro.rs:7:8
+  |
+7 |     fn init() {}
+  |        ^^^^

firmware/defmt-test/macros/tests/ui/mod-must-be-inline.rs

@@ -0,0 +1,4 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests;

firmware/defmt-test/macros/tests/ui/mod-must-be-inline.stderr

@@ -0,0 +1,13 @@
+error[E0658]: non-inline modules in proc macro input are unstable
+ --> tests/ui/mod-must-be-inline.rs:4:1
+  |
+4 | mod tests;
+  | ^^^^^^^^^^
+  |
+  = note: see issue #54727 <https://github.com/rust-lang/rust/issues/54727> for more information
+
+error: module must be inline (e.g. `mod foo {}`)
+ --> tests/ui/mod-must-be-inline.rs:4:1
+  |
+4 | mod tests;
+  | ^^^

firmware/defmt-test/macros/tests/ui/test-has-immutable-param.rs

@@ -0,0 +1,9 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[test]
+    fn say(name: &str) {
+        assert_eq!("name", name);
+    }
+}

firmware/defmt-test/macros/tests/ui/test-has-immutable-param.stderr

@@ -0,0 +1,5 @@
+error: parameter must be a mutable reference (`&mut $Type`)
+ --> tests/ui/test-has-immutable-param.rs:6:12
+  |
+6 |     fn say(name: &str) {
+  |            ^^^^

firmware/defmt-test/macros/tests/ui/test-has-incompatible-init-signature.rs

@@ -0,0 +1,14 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    fn init() -> u32 {
+        0_u32
+    }
+
+    #[test]
+    fn say(value: &mut u16) {
+        assert!(true);
+    }
+}

firmware/defmt-test/macros/tests/ui/test-has-incompatible-init-signature.stderr

@@ -0,0 +1,5 @@
+error: this type must match `#[init]`s return type
+  --> tests/ui/test-has-incompatible-init-signature.rs:11:24
+   |
+11 |     fn say(value: &mut u16) {
+   |                        ^^^

firmware/defmt-test/macros/tests/ui/test-has-invalid-function-signature.rs

@@ -0,0 +1,9 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[test]
+    fn hello(a: i32, b: i32) -> i32 {
+        a + b
+    }
+}

firmware/defmt-test/macros/tests/ui/test-has-invalid-function-signature.stderr

@@ -0,0 +1,5 @@
+error: `#[test]` function must have signature `fn([&mut Type])` (parameter is optional)
+ --> tests/ui/test-has-invalid-function-signature.rs:6:8
+  |
+6 |     fn hello(a: i32, b: i32) -> i32 {
+  |        ^^^^^

firmware/defmt-test/macros/tests/ui/test-has-non-empty-signature.rs

@@ -0,0 +1,14 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    #[init]
+    fn init() {
+        // empty
+    }
+
+    #[test]
+    fn test(arg: &mut u8) {
+        assert!(true);
+    }
+}

firmware/defmt-test/macros/tests/ui/test-has-non-empty-signature.stderr

@@ -0,0 +1,5 @@
+error: no state was initialized by `#[init]`; signature must be `fn()`
+  --> tests/ui/test-has-non-empty-signature.rs:11:8
+   |
+11 |     fn test(arg: &mut u8) {
+   |        ^^^^

firmware/defmt-test/macros/tests/ui/tests-has-argument.rs

@@ -0,0 +1,5 @@
+fn main() {}
+
+#[defmt_test_macros::tests(tests)]
+mod tests {
+}

firmware/defmt-test/macros/tests/ui/tests-has-argument.stderr

@@ -0,0 +1,7 @@
+error: `#[test]` attribute takes no arguments
+ --> tests/ui/tests-has-argument.rs:3:1
+  |
+3 | #[defmt_test_macros::tests(tests)]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: this error originates in the attribute macro `defmt_test_macros::tests` (in Nightly builds, run with -Z macro-backtrace for more info)

firmware/defmt-test/macros/tests/ui/tests-without-annotated-function.rs

@@ -0,0 +1,7 @@
+fn main() {}
+
+#[defmt_test_macros::tests]
+mod tests {
+    fn some_function() {
+    }
+}

firmware/defmt-test/macros/tests/ui/tests-without-annotated-function.stderr

@@ -0,0 +1,5 @@
+error: function requires `#[init]` or `#[test]` attribute
+ --> tests/ui/tests-without-annotated-function.rs:5:5
+  |
+5 |     fn some_function() {
+  |     ^^

firmware/qemu/tests/defmt-test.out

@@ -1,8 +1,9 @@
-(1/7) running `change_init_struct`...
-(2/7) running `test_for_changed_init_struct`...
-(3/7) running `assert_true`...
-(4/7) running `assert_imported_max`...
-(5/7) running `result`...
-(6/7) running `should_error`...
-(7/7) running `fail`...
+(1/8) running `change_init_struct`...
+(2/8) running `test_for_changed_init_struct`...
+(3/8) running `assert_true`...
+(4/8) running `assert_imported_max`...
+(5/8) running `result`...
+(6/8) running `should_error`...
+(7/8) ignoring `ignored`...
+(8/8) running `fail`...
 ERROR panicked at '`#[should_error]` test failed with outcome: Ok(this should have returned `Err`)'

firmware/qemu/tests/defmt-test.rs

@@ -69,6 +69,12 @@ mod tests {
         Err(())
     }
 
+    #[test]
+    #[ignore]
+    fn ignored() -> Result<(), ()> {
+        Err(())
+    }
+
     #[test]
     #[should_error]
     fn fail() -> Result<&'static str, ()> {

xtask/src/main.rs

@@ -77,6 +77,7 @@ enum TestCommand {
     TestCross,
     TestHost,
     TestLint,
+    TestUi,
     /// Run snapshot tests or optionally overwrite the expected output
     TestSnapshot {
         /// Overwrite the expected output instead of comparing it.
@@ -97,6 +98,7 @@ fn main() -> anyhow::Result<()> {
         TestCommand::TestBackcompat => backcompat::test(),
         TestCommand::TestHost => test_host(opt.deny_warnings),
         TestCommand::TestLint => test_lint(),
+        TestCommand::TestUi => test_ui(),
 
         // following tests need to install additional targets
         cmd => {
@@ -192,7 +194,7 @@ fn test_host(deny_warnings: bool) {
         || {
             run_command(
                 "cargo",
-                &["test", "--workspace", "--features", "unstable-test"],
+                &["test", "--workspace", "--features", "unstable-test,alloc"],
                 None,
                 &[],
             )
@@ -426,3 +428,11 @@ fn test_lint() {
         "lint",
     );
 }
+
+fn test_ui() {
+    println!("🧪 lint");
+    do_test(
+        || run_command("cargo", &["test"], Some("firmware/defmt-test/macros"), &[]),
+        "ui",
+    );
+}