Uh oh!
There was an error while loading. Please reload this page.
java.util.Optional matchers - #421
Conversation
tumbarumba
commented
Aug 27, 2024
Thanks for this, @seregamorph! I appreciate the time you've put into this PR. The code is clean and readable, and I think it will be a valuable addition to Hamcrest. If you don't mind, can I request some changes? Firstly, I'm not keen on the use of anonymous inner classes for published matchers in Hamcrest. It would be great if you re-write them as top level public classes that subclass the same Secondly, I'm not definite about this, but I think it would be nicer to put them in a separate sub-package (e.g. Thirdly (and finally), I think static factory method names could give a bit more context, to make them more readable when they've been staticly imported. e.g. change Putting those points together, we'd end up with something like this: importstaticorg.hamcrest.MatcherAssert.assertThat;
importstaticorg.hamcrest.Matchers.is;
importstaticorg.hamcrest.Matchers.not;
importstaticorg.hamcrest.optional.OptionalMatchers.emptyOptional;
importstaticorg.hamcrest.optional.OptionalMatchers.optionalWithValue;
importstaticorg.hamcrest.text.MatchesPattern.matchesPattern;
importorg.junit.Test;
importjava.util.Optional;
publicclassOptionalMatchersTest {
@TestpublicvoidtestEmptyOptional() {
Optional<Object> actual = Optional.empty();
// assertThat(actual, isEmpty());assertThat(actual, is(emptyOptional()));
assertThat(actual, not(optionalWithValue()));
}
@TestpublicvoidtestOptionalWithValue() {
Optional<String> actual = Optional.of("Hello, world");
// assertThat(actual, isPresent());assertThat(actual, not(emptyOptional()));
assertThat(actual, is(optionalWithValue()));
assertThat(actual, optionalWithValue("Hello, world"));
assertThat(actual, optionalWithValue(matchesPattern("Hell")));
}
}What do you think? |
246d63e to
25d7501CompareThank you for the review and comments. Done:
Also please note: I adjusted a bit the description message from These tests clarify the failure messages: AssertionErrorfailure = assertThrows(AssertionError.class, () -> {
assertThat(Optional.of(1), is(emptyOptional()));
});
assertEquals("\n" +
"Expected: is empty\n" +
" but: is Optional[1]", failure.getMessage());or without AssertionErrorfailure = assertThrows(AssertionError.class, () -> {
assertThat(Optional.of(1), emptyOptional());
});
assertEquals("\n" +
"Expected: empty\n" +
" but: is Optional[1]", failure.getMessage()); |
tumbarumba
commented
Aug 27, 2024
Thanks again @seregamorph. Now that I can see the changes in context, I have a bit more feedback:
|
seregamorph
commented
Aug 28, 2024
Done |
This change was waiting for releasing 3.0 with JDK 8 compatibility.
Matchers for
java.util.Optional:OptionalMatchers.emptyOptional()OptionalMatchers.optionalWithValue()OptionalMatchers.optionalWithValue(value)OptionalMatchers.optionalWithValue(Matcher)