Add a friendly warning to usages of .isEqualTo and .isNotEqualTo on a… #2921
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
…ssertions.
Currently, if I were to perform the following code...
var assertion = assertThat(something);
assertThat(assertion).isEqualTo(assertion);
... I would get an error about the use of .equals being unsupported, and
that I should consider using .isEqualTo instead. This is somewhat confusing
to the reader, since they *ARE* using .isEqualTo. This is due to the fact
that .isEqualTo internally will invoke calls to .equals on the "actual" member
which will trigger the aforementioned warning.
I have added two new tests to .isEqualTo and .isNotEqualTo that check if the
"actual" member is an assertion itself, and then raise an error if the
condition is met. The advisory note is to use .isSameAs and .isNotSameAs
instead.
This is useful when using assertj to test custom assertion types for extension
libraries, where we may have wanted to use ".satisfies" instead of ".isEqualTo".
ascopes
force-pushed
the
task/friendly-errors-on-equality-assertion-test
branch
from
7702968 to
eec2d32
Compare
scordio
force-pushed
the
main
branch
2 times, most recently
from
38c7b4b to
3cde5bf
Compare
scordio
pushed a commit
that referenced
this pull request
Aug 1, 2023
Before this change, the following code:
AbstractAssert<?, ?> assertion = assertThat(something);
assertThat(assertion).isEqualTo(assertion);
would throw an exception with a message about the use of `equals` being
unsupported, suggesting to use `isEqualTo` instead. The message is
somewhat confusing to the reader, since `isEqualTo` is indeed used, and
is due to the fact that `isEqualTo` internally relies on the `equals` of
`actual`, which is an `AbstractAssert` instance and throws the
aforementioned exception.
`isEqualTo` and `isNotEqualTo` now check if `actual` is an assertion
instance and raise an `UnsupportedOperationException` if so, with
a message suggesting to use `isSameAs` and `isNotSameAs` instead.
This is, for example, useful for testing custom assertion types for
extension libraries, where the use of `satisfies` instead of `isEqualTo`
may be desirable.
Closes #2921
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.
…ssertions.
Currently, if I were to perform the following code...
... I would get an error about the use of .equals being unsupported, and that I should consider using .isEqualTo instead. This is somewhat confusing to the reader, since they ARE using .isEqualTo. This is due to the fact that .isEqualTo internally will invoke calls to .equals on the "actual" member which will trigger the aforementioned warning.
I have added two new tests to .isEqualTo and .isNotEqualTo that check if the "actual" member is an assertion itself, and then raise an error if the condition is met. The advisory note is to use .isSameAs and .isNotSameAs instead.
This is useful when using assertj to test custom assertion types for extension libraries, where we may have wanted to use ".satisfies" instead of ".isEqualTo".
An example of where I hit this error:
...of course, what I really meant to do was this: