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

Experimental flexible matching of varargs #1235

Closed
wants to merge 1 commit into from

Conversation

mockitoguy
Copy link
Member

It is a prototype/hack. Not intended for check-in but demonstrates that we can match varargs flexibly.

See #1224

Can we follow down this path and fix problem #1222 and #584 in the way the API is most convenient to use?

Interested in helping out? Can you pull down the "varargs-experiment" branch and add tests to FlexibleVarargsTest class so that we can see the gaps?

Not intended for check-in but demonstrates that we can match varargs flexibly.

See #1224

Can we follow down this path and fix problem #1222 and #584 in the way the API is most convenient to use?
@paulduffin
Copy link
Contributor

The problem with this approach (which I did try) is that you cannot differentiate how to match a single argument properly. e.g. given the following two cases in the first one you want to match against the single expanded argument but in the second case you want to match against the varargs array containing a single item.

mock.mixedVarargs(any(), eq("1"))
mock.mixedVarargs(any(), eq(new String[] {"1"}))

In this case you could conceivably check to see what type the matcher was expecting but that would not work if say the varargs was Object.... In that case one call could pass a String and another could pass a String[] and if you based how you matched it on the type of the matcher then you could end up with false positives or negatives.

The only way that I found which could work in all cases is for the developer to have the ability to explicitly state "I want to match this as an array of all the varargs" at the time where they are defining their match.

@mockitoguy
Copy link
Member Author

In this case you could conceivably check to see what type the matcher was expecting but that would not work if say the varargs was Object

Thank you for explaining!

The problem with this approach (which I did try) is that you cannot differentiate how to match a single argument properly.

Brainstorming idea: what if we implement a double match? First, we attempt to match with expanded arguments. If that fails, we try matching against an array. Offering an explicit “varargsAsArray” is totally conceivable, yet it’s not the ideal user experience. Ideally, Mockito works intuitively out of the box.

Thoughts?

@ChristianSchwarz
Copy link
Contributor

Brainstorming idea: what if we implement a double match?

Won't work in null/non-null checks

verify(mock).vararg(isNonNull());

This is ambiguous. Do we check if the vararg-array is non-null or the first element of the vararg-array?

@mockitoguy
Copy link
Member Author

This is ambiguous. Do we check if the vararg-array is non-null or the first element of the vararg-array?

Good point. The most intuitive expectation is:

mock.varargs("x");

//pass
verify(mock).varargs(ArgumentMatchers.isNotNull());
verify(mock).varargs((String) ArgumentMatchers.isNotNull());
verify(mock).varargs((String[]) ArgumentMatchers.isNotNull());

//fail
verify(mock).varargs(ArgumentMatchers.isNull());
verify(mock).varargs((String) ArgumentMatchers.isNull());
verify(mock).varargs((String[]) ArgumentMatchers.isNull());

I will pass my feedback to the #1224 ticket. I will close this PR 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

Successfully merging this pull request may close these issues.

None yet

3 participants