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

add default timeout environment variable RSTEST_TIMEOUT #191

Merged
merged 2 commits into from Mar 24, 2023

Conversation

aviramha
Copy link
Contributor

@aviramha aviramha commented Mar 22, 2023

closes #190
I didn't add tests because I wasn't sure how to test it - would love guidance.

@la10736 la10736 self-requested a review March 22, 2023 09:34
@la10736
Copy link
Owner

la10736 commented Mar 22, 2023

The implementation it's OK. Can you also update changelog.md and the docs in

/// ### Test `#[timeout()]`
? I 've this duplication to handle by now.... I'll switch to cargo-readme soon.

For integration test look at

fn timeout() {
but you need to modify the framework in rstest_test also to inject environment variable.
There are no unit tests about timeout rendering but in this case what you would to test is the environment injection logic (like a singleton) so the integration test should be enough.

@aviramha
Copy link
Contributor Author

thank you, added tests.

@la10736
Copy link
Owner

la10736 commented Mar 24, 2023

Ok, fine to me. I'll do the bureaucracy 😄

@la10736 la10736 merged commit dea3bea into la10736:master Mar 24, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "global" timeout
2 participants