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: Replace assetTrue with assetEquals in tests #861

Open
Simulant87 opened this issue Feb 22, 2024 · 2 comments
Open

Enhancement: Replace assetTrue with assetEquals in tests #861

Simulant87 opened this issue Feb 22, 2024 · 2 comments

Comments

@Simulant87
Copy link
Contributor

There are a lot of tests with an Aaron pattern like

assertTrue("message", expected.equals(actual))

If such an fails, it only returns the message as indicator of the failure, but the difference in expected to actual value is not visible.

The assertions should be changed to:

assertEquals("message", actual, expected)

This way a failing assertion would also output the difference, which would make it easier to find the reason for it.

I would like to work on this issue and provide a PR to improve the assertions.

@stleary
Copy link
Owner

stleary commented Feb 25, 2024

@Simulant87 Thanks for raising this issue. It would be better to get a 'go-ahead' before proceeding with the PR. assertEquals may be marginally better than assertTrue, and it would be reasonable to use it going forward, but I don't see a compelling reason for changing existing tests in a non-functional way.

@rikkarth
Copy link
Contributor

I believe it will become more relevant if and when it is decided to move forward with the JUnit 5 upgrade.

Maybe then, if possible we could correct these declarations to more conventional ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants