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

feat: add teardown fn for defmt-test #917

Merged
merged 5 commits into from
Jan 10, 2025
Merged

Conversation

mhatzl
Copy link

@mhatzl mhatzl commented Jan 8, 2025

This PR adds the possibility to specify a teardown function in a defmt-test module.

Similar to the before_each and after_each attributes, the teardown function can only be specified once and may take the state from the init function as parameter.

In contrast to the *_each functions, the teardown function is called only once at the end of the test run directly before exiting.

This function is needed, for example to handle cleanup code that must only be run at the end of a test run.

Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

Hi Manuel 👋🏾 thank you for your contribution. Looks very good to me, only a few minor nits.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
firmware/defmt-test/Cargo.toml Outdated Show resolved Hide resolved
firmware/defmt-test/Cargo.toml Outdated Show resolved Hide resolved
firmware/defmt-test/macros/Cargo.toml Outdated Show resolved Hide resolved
firmware/defmt-test/README.md Outdated Show resolved Hide resolved
firmware/defmt-test/macros/src/lib.rs Show resolved Hide resolved
@mhatzl
Copy link
Author

mhatzl commented Jan 9, 2025

Hi Johann,
thanks for the review.
I will revert the version and changelog changes.

for the error msg I wait on how you decide

Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

Looks amazing. One question around the macro, if you don't like it I will merge.

Comment on lines +18 to +26
macro_rules! fn_state_signature_msg {
($name:literal) => {
concat!(
"`#[",
$name,
"]` function must have signature `fn()` or `fn(state: &mut T)`"
)
};
}
Copy link
Member

Choose a reason for hiding this comment

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

Could this also be a function? If not, it is okay as well. I just think functions are usually easier to understand.

Suggested change
macro_rules! fn_state_signature_msg {
($name:literal) => {
concat!(
"`#[",
$name,
"]` function must have signature `fn()` or `fn(state: &mut T)`"
)
};
}
fn fn_state_signature_msg(name: &str) -> String {
format!("`#[{name}]` function must have signature `fn()` or `fn(state: &mut T)`")
}

Copy link
Author

Choose a reason for hiding this comment

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

not sure how Rust optimizes format!() in this case, because String is heap allocated while the macro is compile time (so constant).

Thats why I went with a macro.

For defmt-test to be usable in no_std, it is not possible to use String.

Copy link
Member

Choose a reason for hiding this comment

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

For defmt-test to be usable in no_std, it is not possible to use String.

True, makes sense

@Urhengulas Urhengulas added this pull request to the merge queue Jan 10, 2025
Merged via the queue into knurling-rs:main with commit d003488 Jan 10, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants