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

✅ Fix stump_config tests #331

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Conversation

JMicheli
Copy link
Collaborator

So I was investigating the failing tests as mentioned in the Discord earlier and I happened on a solution that seems sensible.

The crate temp-env provides a way to set environment variables, run a test, then unset those variables. Embedding the tests involving environment variables in this crate's wrapper both makes it more ergonomic and also fixes the inconsistent passing of tests that I was experiencing (I interpret this as confirming that the previous tests were causing some sort of race condition where the environment variables for each were fighting each other).

Also, I set the telegram and discord tests to be ignored since they rely on local variables being set. It's probably a good idea to fix those in the future, but this will also open the way for automated benchmarking since that will need to rely on running tests and gauging their performance anyway.

Copy link
Collaborator

@aaronleopold aaronleopold left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@aaronleopold aaronleopold changed the title Fix those stump_config tests ✅ Fix stump_config tests Apr 29, 2024
@aaronleopold aaronleopold merged commit a98f86f into stumpapp:develop Apr 29, 2024
3 checks passed
@JMicheli JMicheli deleted the fix-config-tests branch April 30, 2024 14:50
@aaronleopold aaronleopold mentioned this pull request May 14, 2024
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.

2 participants