Skip to content
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

MockitoHamcrest#argThat does not allow "super" matchers #1817

Closed
spjoe opened this issue Nov 6, 2019 · 3 comments
Closed

MockitoHamcrest#argThat does not allow "super" matchers #1817

spjoe opened this issue Nov 6, 2019 · 3 comments

Comments

@spjoe
Copy link

spjoe commented Nov 6, 2019

IMHO to really follow the hamcrest matchers design philosophy the argThat interface should look like this: public static <T> T argThat(Matcher<? super T> matcher), but unfortunately it is

public static <T> T argThat(Matcher<T> matcher) {

this change would allow a statement like the following:
verify(mock).aMethod(argThat(nullValue()))

I see that the default value is derived from the concrete matcher type, which of course would not work anymore. To circumvent this problem, it would be possible to provide a overload for primitives types.

Or provide a new argThat method, for example argThatSuper which always returns null, but of course would not work for primitive types. Than the existing argThat would keep its behavior.

@TimvdLippe
Copy link
Contributor

Rather than making changes to MockitoHamcrest, I think our stance is that we are deprecating it. I will send a PR to do that instead. Users should use ArgumentMatchers.argThat instead.

Note that

public static <T> T argThat(ArgumentMatcher<T> matcher) {
reportMatcher(matcher);
return null;
}
probably has the same issue? If so, feel free to send a PR to update the definition and add regression tests.

@spjoe
Copy link
Author

spjoe commented Nov 10, 2019

Thanks for the fast response @TimvdLippe

I am very sorry to hear that hamcrest matcher support may get deprecated :-(

With the hamcrest matcher support in mockito it was very easy to share matcher implantations over multiple usecases.

For example I use the same matcher in:

  • mockito verify
  • junit assertThat
  • awaitility until

For me dropping the support would mean to rewrite (and maintain) a tone of matchers as ArgumentMatcher.

Please do not drop Hamcrest Matcher support.

Best Regards,
spjoe

@TimvdLippe
Copy link
Contributor

We are keeping this deprecated, but won't delete it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants