thecoop commented on PR #14518: URL: https://github.com/apache/lucene/pull/14518#issuecomment-2821367957
The focus of this PR was to change `assertTrue`/`assertFalse` to assertions that gave what was actually wrong. The `assertEquals` are things I spotted alongside those. I prefer the `assertThat` hamcrest matchers over `assertEquals` because it is unequivocal what the expected value is vs what is being tested, which is not the same with `assertEquals`, and I think the matcher assertions read better for what the test is checking, especially with more complicated checks such as `arrayWithSize(greaterThan(6))`. I used hamcrest as that is used more in the codebase than assertj. However, using hamcrest in this way is a personal preference, that can easily be removed from this PR if people disagree. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
