Convert uses of JUnit's assertThrows to use AssertJ's assertThatExceptionOfType #2306
Conversation
There was a problem hiding this comment.
Please revert your changes to this file, as it's not related to the PR
There was a problem hiding this comment.
Please revert your changes to this file, as it's not related to the PR
There was a problem hiding this comment.
Please revert your changes to this file, as it's not related to the PR
There was a problem hiding this comment.
Please revert your changes to this file, as it's not related to the PR
| } | ||
|
|
||
| @Ignore("Remove to run test") | ||
| //@Ignore("Remove to run test") |
There was a problem hiding this comment.
Please revert this change, we want all tests except the first test to be ignored by default.
| () -> new SgfParsing().parse(input)); | ||
| assertThatExceptionOfType(SgfParsingException.class) | ||
| .isThrownBy(() -> new SgfParsing().parse(input)) | ||
| .withMessage("tree missing"); |
There was a problem hiding this comment.
This .withMessage() is incorrect: the first argument to JUnit's assertThrows() doesn't assert the exception message but is used as the message printed when the assertion fails.
In fact, the original test doesn't perform an assertion of the exception message, only on its type, so there is no need to call .withMessage() in any of the tests in this file.
sanderploegsma
changed the title
|
Closing this PR as it's currently inactive. @GitteVandevenne if you want to pick this up again just let me know and we'll reopen it. |
pull request
changed assertThrows to assertThatExceptionOfType in:
Contributes to #2147
Reviewer Resources:
Track Policies