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

Fix issue #1222 and #584 #1224

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/main/java/org/mockito/ArgumentMatchers.java
Expand Up @@ -6,12 +6,14 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.EmptyStackException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import org.mockito.exceptions.misusing.InvalidUseOfMatchersException;
import org.mockito.internal.matchers.Any;
import org.mockito.internal.matchers.Contains;
import org.mockito.internal.matchers.EndsWith;
Expand All @@ -22,6 +24,7 @@
import org.mockito.internal.matchers.Null;
import org.mockito.internal.matchers.Same;
import org.mockito.internal.matchers.StartsWith;
import org.mockito.internal.matchers.TreatVarargsAsArray;
import org.mockito.internal.matchers.apachecommons.ReflectionEquals;
import org.mockito.internal.util.Primitives;

Expand Down Expand Up @@ -1317,6 +1320,24 @@ public static double doubleThat(ArgumentMatcher<Double> matcher) {
return 0;
}

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a new point to the main Mockito Javadoc with a short description of the feature? This is a really cool feature and it deserves a note in main Javadoc. That doc acts as Mockito user guide and also shows notable features. Feel free to bump new minor version.

* Indicates that the varargs should be matched as an array rather than individual values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "individual values" -> "individual value"

*
* The value must be specified as a call to a method for matching an argument, e.g.
* {@link #eq(Object)} or {@link ArgumentCaptor#capture()}. Failure to do so will result in
* either an {@link EmptyStackException} or an {@link InvalidUseOfMatchersException}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a unit test for this behavior? Let's see how the unit test looks like and what is the user experience. If the user experience is not great, can we offer a clean exception describing API misuse?

*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add code sample to the Javadoc in a similar way we do it for other main public methods?

* @param value expected to be a matcher such as {@code eq(...) or
* {@code captor.capture()}
* @param <T> the type of the vararg or array of varargs
* @return {@code null}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature is really cool and it deserves new minor version. Can you add "since" tag to the Javadoc with the next minor version? Also, let's update the "version.properties" file accordingly!

public static <T> T varargsAsArray(T value) {
ArgumentMatcher nestedMatcher = mockingProgress().getArgumentMatcherStorage().popMatcher();
reportMatcher(new TreatVarargsAsArray<Object>(nestedMatcher));
return null;
}

private static void reportMatcher(ArgumentMatcher<?> matcher) {
mockingProgress().getArgumentMatcherStorage().reportMatcher(matcher);
}
Expand Down
Expand Up @@ -6,13 +6,15 @@

import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS;
import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.MATCH_EACH_VARARGS_WITH_LAST_MATCHER;
import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ONE_MATCHER_PER_ARGUMENT;
import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ONE_MATCHER_PER_EXPANDED_ARGUMENT;
import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ONE_MATCHER_PER_RAW_ARGUMENT;

import java.util.ArrayList;
import java.util.List;

import org.mockito.ArgumentMatcher;
import org.mockito.internal.matchers.CapturingMatcher;
import org.mockito.internal.matchers.TreatVarargsAsArray;
import org.mockito.internal.matchers.VarargMatcher;
import org.mockito.invocation.Invocation;

Expand Down Expand Up @@ -74,7 +76,7 @@ public boolean forEachMatcherAndArgument(ArgumentMatcherAction action) {
if (matchingType == ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS)
return false;

Object[] arguments = invocation.getArguments();
Object[] arguments = matchingType.getArguments(invocation);
for (int i = 0; i < arguments.length; i++) {
ArgumentMatcher<?> matcher = matchers.get(i);
Object argument = arguments[i];
Expand All @@ -91,21 +93,35 @@ private static MatcherApplicationType getMatcherApplicationType(Invocation invoc
final int expandedArguments = invocation.getArguments().length;
final int matcherCount = matchers.size();

boolean treatVarargsAsArray = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. I'm very impressed that you were able to inject the new algorithm here. The code is pretty complex around here. I'd love to see it refactored at some point in the future.

It is complex but you have added fantastic test coverage that I was not able to break down :)

boolean reuseLastMatcherForVarargs = false;
if (matcherCount > 0 && invocation.getMethod().isVarArgs()) {
ArgumentMatcher<?> lastMatcher = lastMatcher(matchers);
if (lastMatcher instanceof TreatVarargsAsArray) {
treatVarargsAsArray = true;
} else if (lastMatcher instanceof VarargMatcher) {
reuseLastMatcherForVarargs = true;
}
}

if (expandedArguments == matcherCount) {
return ONE_MATCHER_PER_ARGUMENT;
return treatVarargsAsArray
? ONE_MATCHER_PER_RAW_ARGUMENT : ONE_MATCHER_PER_EXPANDED_ARGUMENT;
}

if (rawArguments == matcherCount && isLastMatcherVargargMatcher(matchers)) {
return MATCH_EACH_VARARGS_WITH_LAST_MATCHER;
if (rawArguments == matcherCount) {
if (treatVarargsAsArray) {
return ONE_MATCHER_PER_RAW_ARGUMENT;
}

if (reuseLastMatcherForVarargs) {
return MATCH_EACH_VARARGS_WITH_LAST_MATCHER;
}
}

return ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS;
}

private static boolean isLastMatcherVargargMatcher(final List<ArgumentMatcher<?>> matchers) {
return lastMatcher(matchers) instanceof VarargMatcher;
}

private static List<ArgumentMatcher<?>> appendLastMatcherNTimes(List<ArgumentMatcher<?>> matchers, int timesToAppendLastMatcher) {
ArgumentMatcher<?> lastMatcher = lastMatcher(matchers);

Expand All @@ -127,6 +143,18 @@ private static ArgumentMatcher<?> lastMatcher(List<ArgumentMatcher<?>> matchers)
}

enum MatcherApplicationType {
ONE_MATCHER_PER_ARGUMENT, MATCH_EACH_VARARGS_WITH_LAST_MATCHER, ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS;
ONE_MATCHER_PER_EXPANDED_ARGUMENT,
ONE_MATCHER_PER_RAW_ARGUMENT() {
@Override
Object[] getArguments(Invocation invocation) {
return invocation.getRawArguments();
}
},
MATCH_EACH_VARARGS_WITH_LAST_MATCHER,
ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS;

Object[] getArguments(Invocation invocation) {
return invocation.getArguments();
}
}
}
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2017 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockito.internal.matchers;

import org.mockito.ArgumentMatcher;
import org.mockito.internal.matchers.text.MatcherToString;
import java.io.Serializable;

/**
* Wraps an {@link ArgumentMatcher} and indicates that it should be passed the varargs as a single
* array rather than individual values.
*/
public class TreatVarargsAsArray<T>
implements ArgumentMatcher<T>, ContainsExtraTypeInfo, CapturesArguments, Serializable {

private final ArgumentMatcher<? super T> delegate;

public TreatVarargsAsArray(ArgumentMatcher<? super T> delegate) {
this.delegate = delegate;
}

@Override
public boolean matches(T argument) {
return delegate.matches(argument);
}

@Override
public void captureFrom(Object argument) {
if (delegate instanceof CapturesArguments) {
((CapturesArguments) delegate).captureFrom(argument);
}
}

@Override
public String toStringWithType() {
if (delegate instanceof ContainsExtraTypeInfo) {
String delagateAsString = ((ContainsExtraTypeInfo) delegate).toStringWithType();
return wrapString(delagateAsString);
}
return toString();
}

@Override
public String toString() {
String delegateAsString = MatcherToString.toString(delegate);
return wrapString(delegateAsString);
}

private String wrapString(String delegateAsString) {
return "varargsAsArray(" + delegateAsString + ")";
}

@Override
public boolean typeMatches(Object target) {
if (delegate instanceof ContainsExtraTypeInfo) {
return ((ContainsExtraTypeInfo) delegate).typeMatches(target);
}
return true;
}
}
Expand Up @@ -13,7 +13,7 @@
/**
* Provides better toString() text for matcher that don't have toString() method declared.
*/
class MatcherToString {
public class MatcherToString {

/**
* Attempts to provide more descriptive toString() for given matcher.
Expand All @@ -25,7 +25,7 @@ class MatcherToString {
* @param matcher
* @return
*/
static String toString(ArgumentMatcher<?> matcher) {
public static String toString(ArgumentMatcher<?> matcher) {
Class<?> cls = matcher.getClass();
while(cls != Object.class) {
Method[] methods = cls.getDeclaredMethods();
Expand Down
Expand Up @@ -26,4 +26,5 @@ public interface ArgumentMatcherStorage {

void reset();

ArgumentMatcher<?> popMatcher();
}
Expand Up @@ -84,7 +84,8 @@ private void assertStateFor(String additionalMatcherName, int subMatchersCount)
}
}

private ArgumentMatcher<?> popMatcher() {
@Override
public ArgumentMatcher<?> popMatcher() {
return matcherStack.pop().getMatcher();
}

Expand Down
@@ -0,0 +1,64 @@
package org.mockito.internal.matchers;

import org.junit.Test;

import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;

public class TreatVarargsAsArrayTest {

@Test
public void delegateDoesNotImplementCapturesArgument_captureFrom() {
TreatVarargsAsArray<String[]> matcher = new TreatVarargsAsArray<String[]>(
new Equals(new String[] {"1"}));

// Should not do anything.
matcher.captureFrom("2");
}

@Test
public void delegateDoesImplementCapturesArgument_captureFrom() {
CapturingMatcher<String[]> capturingMatcher = new CapturingMatcher<String[]>();
TreatVarargsAsArray<String[]> matcher =
new TreatVarargsAsArray<String[]>(capturingMatcher);

// Should not do anything.
matcher.captureFrom(new String[] {"2"});

assertArrayEquals(new String[] {"2"}, capturingMatcher.getLastValue());
}

@Test
public void delegateDoesNotImplementContainsExtraTypeInfo_toStringWithType() {
TreatVarargsAsArray<String[]> matcher = new TreatVarargsAsArray<String[]>(NotNull.NOT_NULL);

assertEquals("varargsAsArray(notNull())", matcher.toStringWithType());
}

@Test
public void delegateDoesImplementContainsExtraTypeInfo_toStringWithType() {
TreatVarargsAsArray<String[]> matcher = new TreatVarargsAsArray<String[]>(
new Equals(new String[] {"1"}));

assertEquals("varargsAsArray((String[]) [\"1\"])", matcher.toStringWithType());
}

@Test
public void delegateDoesNotImplementContainsExtraTypeInfo_typeMatches() {
TreatVarargsAsArray<String[]> matcher = new TreatVarargsAsArray<String[]>(NotNull.NOT_NULL);

assertTrue(matcher.typeMatches(new String[] {}));
assertTrue(matcher.typeMatches(1));
}

@Test
public void delegateDoesImplementContainsExtraTypeInfo_typeMatches() {
TreatVarargsAsArray<String[]> matcher = new TreatVarargsAsArray<String[]>(
new Equals(new String[] {"1"}));

assertTrue("String[]", matcher.typeMatches(new String[] {}));
assertFalse("int", matcher.typeMatches(1));
}
}
51 changes: 39 additions & 12 deletions src/test/java/org/mockitousage/matchers/VarargsTest.java
Expand Up @@ -9,6 +9,8 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNotNull;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.ArgumentMatchers.varargsAsArray;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -303,26 +305,51 @@ public void shouldNotCaptureVarArgs_1args2captures() {

}

/**
* As of v2.0.0-beta.118 this test fails. Once the github issues:
* <ul>
* <li>'#584 ArgumentCaptor can't capture varargs-arrays
* <li>#565 ArgumentCaptor should be type aware' are fixed this test must
* succeed
* </ul>
*/
@Test
@Ignore("Blocked by github issue: #584 & #565")
public void shouldCaptureVarArgsAsArray() {
mock.varargs("1", "2");
public void shouldCaptureVarArgsAsArray_noVarargs() {
mock.varargs("1");

ArgumentCaptor<String[]> varargCaptor = ArgumentCaptor.forClass(String[].class);

verify(mock).varargs(varargCaptor.capture());
verify(mock).varargs(varargsAsArray(varargCaptor.capture()));

assertThat(varargCaptor).containsExactly(new String[] { "1" });
}

@Test
public void shouldCaptureVarArgsAsArray_oneVararg() {
mock.varargs("1");
mock.varargs("2");

ArgumentCaptor<String[]> varargCaptor = ArgumentCaptor.forClass(String[].class);

verify(mock, times(2)).varargs(varargsAsArray(varargCaptor.capture()));

assertThat(varargCaptor).containsExactly(new String[] { "1" }, new String[] { "2" });
}

@Test
public void shouldCaptureVarArgsAsArray_twoVarargs() {
mock.mixedVarargs(getClass(), "1", "2");

ArgumentCaptor<String[]> varargCaptor = ArgumentCaptor.forClass(String[].class);

verify(mock).mixedVarargs(any(), varargsAsArray(varargCaptor.capture()));

assertThat(varargCaptor).containsExactly(new String[] { "1", "2" });
}

@Test
public void shouldCaptureVarArgsAsByteArray_twoVarargs() {
mock.varargsbyte((byte) 1, (byte) 2);

ArgumentCaptor<byte[]> varargCaptor = ArgumentCaptor.forClass(byte[].class);

verify(mock).varargsbyte(varargsAsArray(varargCaptor.capture()));

assertThat(varargCaptor).containsExactly(new byte[] { (byte) 1, (byte) 2 });
}

@Test
public void shouldNotMatchRegualrAndVaraArgs() {
mock.varargsString(1, "a","b");
Expand Down