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

[Enhancement]: use testify's require instead of assert. #2808

Open
mdelapenya opened this issue Oct 2, 2024 · 4 comments
Open

[Enhancement]: use testify's require instead of assert. #2808

mdelapenya opened this issue Oct 2, 2024 · 4 comments
Labels
enhancement New feature or request good first issue Want to contribute to testcontainers? Start from here hacktoberfest Pull Requests accepted for Hacktoberfest. low hanging fruit Issues that are good for newcomers.

Comments

@mdelapenya
Copy link
Member

Proposal

TL;DR: use require when a failure should stop the test completely, ensuring that subsequent steps are not executed with invalid assumptions. Use assert when we want to evaluate all assertions, even if some fail, to provide more comprehensive feedback on test results.

Why?

By using require for crucial checks, we can avoid misleading error messages from failed assertions that come later in the test after an essential condition has already failed. This makes the root cause of the failure easier to diagnose, as the test execution halts as soon as the condition fails, preventing further unnecessary execution of test logic that depends on the success of the failed condition. This can save time, especially in long or complex tests, where subsequent steps may not be relevant after a failure.

assert, on the other hand, might allow tests to continue, resulting in multiple failures and potentially obscuring the real cause.

@mdelapenya mdelapenya added hacktoberfest Pull Requests accepted for Hacktoberfest. good first issue Want to contribute to testcontainers? Start from here enhancement New feature or request low hanging fruit Issues that are good for newcomers. labels Oct 2, 2024
@AshutoshKD
Copy link
Contributor

Hey! I'd like to work on this

@AshutoshKD
Copy link
Contributor

Should the proposed change to be done on all the test files?

@mdelapenya
Copy link
Member Author

@AshutoshKD thanks for stepping up! Yes, you can start with a draft PR with the files you consider important, and we can iterate on it

@JoelLau
Copy link
Contributor

JoelLau commented Oct 5, 2024

hi @mdelapenya & @AshutoshKD , i found a few more places that could benefit from this change. let me know what you think!

#2814

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Want to contribute to testcontainers? Start from here hacktoberfest Pull Requests accepted for Hacktoberfest. low hanging fruit Issues that are good for newcomers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants