Support #[ignore] attribute in defmt_test #618
Conversation
justahero
changed the title
justahero
changed the title
justahero
changed the title
justahero
force-pushed
the
feature/ignore-proc-macro-attribute-defmt-test
branch
from
8d4ef1a
to
3f84476
Compare
firmware/defmt-test/macros/tests/ui/tests-without-annotated-function.stderr
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The implementation looks good to me. Thanks for adding the UI tests. Left some comments inline.
Note the message is different from the suggestion in #390 and should be revised accordingly if needed
I don't feel strongly about the wording that's used at the end. I think I wrote "ignored" (without ellipsis) in the issue description because skipping tests is instantaneous, whereas running tests can take a while if the target needs to e.g. sample temperature once per seconds for a few seconds to check some requirement so one may see "running .." (long pause) and then new test.
| @@ -0,0 +1,13 @@ | |||
| error[E0658]: non-inline modules in proc macro input are unstable | |||
There was a problem hiding this comment.
(interesting that you also get an error from rustc about stability)
firmware/qemu/tests/defmt-test.rs
Outdated
| @@ -69,6 +69,13 @@ mod tests { | |||
| Err(()) | |||
| } | |||
|
|
|||
| #[test] | |||
| #[ignore] | |||
| #[allow(dead_code)] | |||
There was a problem hiding this comment.
sorry, to clarify: I meant putting the allow in the expansion (proc-macro code) not in the input code. the semantics here should be following (this can be UI tested)
#[test]
#[ignore]
fn f() { // <- warning about `f` being `dead_code` MUST NOT be emitted
let x = 42; // <- `dead_code` warning MUST be emitted here
}I think it would be good to also test that compiler errors in ignored tests are emitted.
The expect semantic is that ignored tests are compiled but not executed.
A UI test like this can test that:
#[test]
#[ignore]
fn f() {
let x: () = 42; //~ ERROR: type mismatch
}There was a problem hiding this comment.
The dead_code warning of an ignored function is handled by a recent change.
Unfortunately it's not easy to set up the suggested tests in the defmt-test crate using trybuild. The defmt-test crate is used as a library in a non-std environment, for example when set up from the app-template repo. I tried to add ui tests using trybuild but this works in std envs only.
* log ignored tests * put dummy statement to `unit_test_calls` to keep same vec size
This is to not cause a `dead_code` warning when an ignored test function is not run.
justahero
force-pushed
the
feature/ignore-proc-macro-attribute-defmt-test
branch
from
49b82a6
to
6914abd
Compare
firmware/defmt-test/tests/ui/test-uses-invalid-type-annotation.rs
Outdated
Show resolved
Hide resolved
justahero
force-pushed
the
feature/ignore-proc-macro-attribute-defmt-test
branch
from
abd28ff
to
918aa99
Compare
|
bors r+ |
|
Build succeeded: |
| #krate::export::check_outcome(#call, #should_error); | ||
| )); | ||
| if ignore { | ||
| unit_test_calls.push(quote!(let _ = #call;)); |
There was a problem hiding this comment.
This will still call the test function if it has #[ignore] on it, leading to the issue reported in #731
This PR adds support for the
#[ignore]attribute indefmt_testto solve #390. Similar to theignoreattribute, tests can be annotated with the attribute to ignore particular tests.trybuildcrate to check multiple conditions with the proc macroNote the message is different from the suggestion in #390 and should be revised accordingly if needed