-
Notifications
You must be signed in to change notification settings - Fork 90
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 usage of asserts where order or condition is missleading #1700
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carstenartur
force-pushed
the
cleanuptestasserts
branch
2 times, most recently
from
October 6, 2024 08:39
bd2f3ff
to
8815ef7
Compare
carstenartur
force-pushed
the
cleanuptestasserts
branch
from
October 7, 2024 22:53
8815ef7
to
6cd0c8e
Compare
the build failed multiple times (build 4, 5) with
|
jukzi
force-pushed
the
cleanuptestasserts
branch
from
October 8, 2024 14:19
6cd0c8e
to
fc70250
Compare
jjohnstn
requested changes
Oct 8, 2024
...eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/templates/TemplateCompletionTests.java
Outdated
Show resolved
Hide resolved
This test has been failing randomly for me in PR runs lately before this patch. |
carstenartur
force-pushed
the
cleanuptestasserts
branch
4 times, most recently
from
October 15, 2024 19:16
cac9a88
to
a21fd55
Compare
jjohnstn
force-pushed
the
cleanuptestasserts
branch
2 times, most recently
from
October 17, 2024 17:41
898a187
to
8959467
Compare
jjohnstn
force-pushed
the
cleanuptestasserts
branch
from
October 18, 2024 00:51
8959467
to
88287c9
Compare
jjohnstn
approved these changes
Oct 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it does
junit asserts generate an error message based on the parameters presented. So when the fixed part and the actual part of an equal assertion is wrong it creates a wrong message about what was expected. The assertion still "works" because in an equal comparison the order does not matter regarding the failure of the junit test but the error message is wrong.
Another case is when you do the comparison inside of an assertion to check for true. That way junit is inable to generate a useful error message.
This PR fixes some of these issues. It has been generated by using the "Autorefactor" cleanup at
https://github.com/carstenartur/AutoRefactor/tree/master
Afaik this cleanup will not make it into jdt.ui any longer.
I looked at it while checking what could help to migrate tests to junit5.
Since the change was applied to no longer support old java versions in a lot of code of jdt.ui test environments are a mess with wrong name for the testenvironment to be used.
How to test
Nothing should change but the messages should be better in case of an issue.
Author checklist