Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support #[ignore] attribute in defmt_test #618

Merged
merged 15 commits into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -143,6 +155,7 @@ jobs:
- mdbook
- qemu-snapshot
- backcompat
- ui
runs-on: ubuntu-20.04
steps:
- name: CI succeeded
Expand Down
3 changes: 3 additions & 0 deletions firmware/defmt-test/macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
27 changes: 23 additions & 4 deletions firmware/defmt-test/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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
}
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -130,6 +141,7 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result<TokenStrea
func: f,
input,
should_error,
ignore,
})
}
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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;));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will still call the test function if it has #[ignore] on it, leading to the issue reported in #731

} else {
unit_test_calls.push(quote!(
#krate::export::check_outcome(#call, #should_error);
));
}
}

let test_functions = tests.iter().map(|test| &test.func);
Expand All @@ -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! {
Expand Down Expand Up @@ -263,6 +281,7 @@ struct Test {
cfgs: Vec<Attribute>,
input: Option<Input>,
should_error: bool,
ignore: bool,
}

struct Input {
Expand Down
5 changes: 5 additions & 0 deletions firmware/defmt-test/macros/tests/ui.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[test]
fn ui() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/ui/*.rs");
}
10 changes: 10 additions & 0 deletions firmware/defmt-test/macros/tests/ui/init-duplicate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[init]
fn first() {}

#[init]
fn second() {}
}
5 changes: 5 additions & 0 deletions firmware/defmt-test/macros/tests/ui/init-duplicate.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: only a single `#[init]` function can be defined
--> tests/ui/init-duplicate.rs:9:8
|
9 | fn second() {}
| ^^^^^^
8 changes: 8 additions & 0 deletions firmware/defmt-test/macros/tests/ui/init-has-ignore-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[init]
#[ignore]
fn init() {}
}
Original file line number Diff line number Diff line change
@@ -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() {}
| ^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[init]
fn hello(a: i32, b: i32) -> i32 {
a + b
}
}
Original file line number Diff line number Diff line change
@@ -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 {
| ^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[init]
#[should_error]
fn init() {}
}
Original file line number Diff line number Diff line change
@@ -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() {}
| ^^^^
4 changes: 4 additions & 0 deletions firmware/defmt-test/macros/tests/ui/mod-must-be-inline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests;
13 changes: 13 additions & 0 deletions firmware/defmt-test/macros/tests/ui/mod-must-be-inline.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0658]: non-inline modules in proc macro input are unstable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(interesting that you also get an error from rustc about stability)

--> 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;
| ^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[test]
fn say(name: &str) {
justahero marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!("name", name);
}
}
Original file line number Diff line number Diff line change
@@ -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) {
| ^^^^
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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) {
| ^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[test]
fn hello(a: i32, b: i32) -> i32 {
a + b
}
}
Original file line number Diff line number Diff line change
@@ -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 {
| ^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
#[init]
fn init() {
// empty
}

#[test]
fn test(arg: &mut u8) {
assert!(true);
}
}
Original file line number Diff line number Diff line change
@@ -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) {
| ^^^^
5 changes: 5 additions & 0 deletions firmware/defmt-test/macros/tests/ui/tests-has-argument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn main() {}

#[defmt_test_macros::tests(tests)]
mod tests {
}
7 changes: 7 additions & 0 deletions firmware/defmt-test/macros/tests/ui/tests-has-argument.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error: `#[test]` attribute takes no arguments
justahero marked this conversation as resolved.
Show resolved Hide resolved
--> 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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {}

#[defmt_test_macros::tests]
mod tests {
fn some_function() {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: function requires `#[init]` or `#[test]` attribute
--> tests/ui/tests-without-annotated-function.rs:5:5
|
5 | fn some_function() {
| ^^
15 changes: 8 additions & 7 deletions firmware/qemu/tests/defmt-test.out
Original file line number Diff line number Diff line change
@@ -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`)'
6 changes: 6 additions & 0 deletions firmware/qemu/tests/defmt-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ mod tests {
Err(())
}

#[test]
#[ignore]
fn ignored() -> Result<(), ()> {
Err(())
}

#[test]
#[should_error]
fn fail() -> Result<&'static str, ()> {
Expand Down
12 changes: 11 additions & 1 deletion xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 => {
Expand Down Expand Up @@ -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,
&[],
)
Expand Down Expand Up @@ -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",
);
}