diff --git a/src/main/java/org/mockito/AdditionalMatchers.java b/src/main/java/org/mockito/AdditionalMatchers.java index 059693bd8e..fa20fbc491 100644 --- a/src/main/java/org/mockito/AdditionalMatchers.java +++ b/src/main/java/org/mockito/AdditionalMatchers.java @@ -49,7 +49,7 @@ public class AdditionalMatchers { * the given value. * @return null. */ - public static > T geq(Comparable value) { + public static > T geq(T value) { reportMatcher(new GreaterOrEqual(value)); return null; } @@ -147,7 +147,7 @@ public static short geq(short value) { * the given value. * @return null. */ - public static > T leq(Comparable value) { + public static > T leq(T value) { reportMatcher(new LessOrEqual(value)); return null; } @@ -245,7 +245,7 @@ public static short leq(short value) { * the given value. * @return null. */ - public static > T gt(Comparable value) { + public static > T gt(T value) { reportMatcher(new GreaterThan(value)); return null; } @@ -343,7 +343,7 @@ public static short gt(short value) { * the given value. * @return null. */ - public static > T lt(Comparable value) { + public static > T lt(T value) { reportMatcher(new LessThan(value)); return null; } @@ -442,7 +442,7 @@ public static short lt(short value) { * the given value. * @return null. */ - public static > T cmpEq(Comparable value) { + public static > T cmpEq(T value) { reportMatcher(new CompareEqual(value)); return null; } diff --git a/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java b/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java index 2054cff0c2..7999f73ab8 100644 --- a/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java +++ b/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java @@ -4,12 +4,12 @@ */ package org.mockito.internal.invocation; +import java.lang.reflect.Method; +import java.util.List; import org.mockito.ArgumentMatcher; import org.mockito.internal.matchers.VarargMatcher; import org.mockito.invocation.Invocation; -import java.util.List; - @SuppressWarnings("unchecked") public class ArgumentsComparator { public boolean argumentsMatch(InvocationMatcher invocationMatcher, Invocation actual) { @@ -22,21 +22,24 @@ public boolean argumentsMatch(InvocationMatcher invocationMatcher, Object[] actu return false; } for (int i = 0; i < actualArgs.length; i++) { - if (!invocationMatcher.getMatchers().get(i).matches(actualArgs[i])) { + ArgumentMatcher argumentMatcher = invocationMatcher.getMatchers().get(i); + Object argument = actualArgs[i]; + + if (!matches(argumentMatcher, argument)) { return false; } } return true; } - //ok, this method is a little bit messy but the vararg business unfortunately is messy... - private boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation actual) { + // ok, this method is a little bit messy but the vararg business unfortunately is messy... + private static boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation actual) { if (!actual.getMethod().isVarArgs()) { - //if the method is not vararg forget about it + // if the method is not vararg forget about it return false; } - //we must use raw arguments, not arguments... + // we must use raw arguments, not arguments... Object[] rawArgs = actual.getRawArguments(); List matchers = invocationMatcher.getMatchers(); @@ -46,18 +49,66 @@ private boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation act for (int i = 0; i < rawArgs.length; i++) { ArgumentMatcher m = matchers.get(i); - //it's a vararg because it's the last array in the arg list - if (rawArgs[i] != null && rawArgs[i].getClass().isArray() && i == rawArgs.length-1) { - //this is very important to only allow VarargMatchers here. If you're not sure why remove it and run all tests. + // it's a vararg because it's the last array in the arg list + if (rawArgs[i] != null && rawArgs[i].getClass().isArray() && i == rawArgs.length - 1) { + // this is very important to only allow VarargMatchers here. If + // you're not sure why remove it and run all tests. if (!(m instanceof VarargMatcher) || !m.matches(rawArgs[i])) { return false; } - //it's not a vararg (i.e. some ordinary argument before varargs), just do the ordinary check - } else if (!m.matches(rawArgs[i])){ + // it's not a vararg (i.e. some ordinary argument before + // varargs), just do the ordinary check + } else if (!m.matches(rawArgs[i])) { return false; } } return true; } -} \ No newline at end of file + + private static boolean matches(ArgumentMatcher argumentMatcher, Object argument) { + return isCompatible(argumentMatcher, argument) && argumentMatcher.matches(argument); + } + + /** + * Returns true if the given argument can be passed to + * the given argumentMatcher without causing a + * {@link ClassCastException}. + */ + private static boolean isCompatible(ArgumentMatcher argumentMatcher, Object argument) { + if (argument == null) + return true; + + Class expectedArgumentType = getArgumentType(argumentMatcher); + + return expectedArgumentType.isInstance(argument); + } + + /** + * Returns the type of {@link ArgumentMatcher#matches(Object)} of the given + * {@link ArgumentMatcher} implementation. + */ + private static Class getArgumentType(ArgumentMatcher argumentMatcher) { + Method[] methods = argumentMatcher.getClass().getMethods(); + for (Method method : methods) { + if (isMatchesMethod(method)) { + return method.getParameterTypes()[0]; + } + } + throw new NoSuchMethodError("Method 'matches(T)' not found in ArgumentMatcher: " + argumentMatcher + " !\r\n Please file a bug with this stack trace at: https://github.com/mockito/mockito/issues/new "); + } + + /** + * Returns true if the given method is + * {@link ArgumentMatcher#matches(Object)} + */ + private static boolean isMatchesMethod(Method method) { + if (method.getParameterTypes().length != 1) { + return false; + } + if (method.isBridge()) { + return false; + } + return method.getName().equals("matches"); + } +} diff --git a/src/main/java/org/mockito/internal/matchers/CompareEqual.java b/src/main/java/org/mockito/internal/matchers/CompareEqual.java index c94ce8f59b..eb21c46e3e 100644 --- a/src/main/java/org/mockito/internal/matchers/CompareEqual.java +++ b/src/main/java/org/mockito/internal/matchers/CompareEqual.java @@ -9,7 +9,7 @@ public class CompareEqual> extends CompareTo implements Serializable { - public CompareEqual(Comparable value) { + public CompareEqual(T value) { super(value); } diff --git a/src/main/java/org/mockito/internal/matchers/CompareTo.java b/src/main/java/org/mockito/internal/matchers/CompareTo.java index 57392088ce..9be4b58ebb 100644 --- a/src/main/java/org/mockito/internal/matchers/CompareTo.java +++ b/src/main/java/org/mockito/internal/matchers/CompareTo.java @@ -9,24 +9,32 @@ import java.io.Serializable; - public abstract class CompareTo> implements ArgumentMatcher, Serializable { - private final Comparable wanted; + private final T wanted; - public CompareTo(Comparable value) { + public CompareTo(T value) { this.wanted = value; } - @SuppressWarnings("unchecked") - public boolean matches(T actual) { - return matchResult(((Comparable)actual).compareTo(wanted)); + @Override + public final boolean matches(T actual) { + if (actual == null) { + return false; + } + if (!actual.getClass().isInstance(wanted)){ + return false; + } + + int result = actual.compareTo(wanted); + return matchResult(result); } - public String toString() { + @Override + public final String toString() { return getName() + "(" + wanted + ")"; } - + protected abstract String getName(); - + protected abstract boolean matchResult(int result); } diff --git a/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java b/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java index 7377478fc0..f1c8ff2a06 100644 --- a/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java +++ b/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java @@ -9,7 +9,7 @@ public class GreaterOrEqual> extends CompareTo implements Serializable { - public GreaterOrEqual(Comparable value) { + public GreaterOrEqual(T value) { super(value); } diff --git a/src/main/java/org/mockito/internal/matchers/GreaterThan.java b/src/main/java/org/mockito/internal/matchers/GreaterThan.java index da8f75e6fc..4ca82cfb06 100644 --- a/src/main/java/org/mockito/internal/matchers/GreaterThan.java +++ b/src/main/java/org/mockito/internal/matchers/GreaterThan.java @@ -9,7 +9,7 @@ public class GreaterThan> extends CompareTo implements Serializable { - public GreaterThan(Comparable value) { + public GreaterThan(T value) { super(value); } diff --git a/src/main/java/org/mockito/internal/matchers/LessOrEqual.java b/src/main/java/org/mockito/internal/matchers/LessOrEqual.java index 5cd36e55a3..1b3d737ede 100644 --- a/src/main/java/org/mockito/internal/matchers/LessOrEqual.java +++ b/src/main/java/org/mockito/internal/matchers/LessOrEqual.java @@ -9,7 +9,7 @@ public class LessOrEqual> extends CompareTo implements Serializable { - public LessOrEqual(Comparable value) { + public LessOrEqual(T value) { super(value); } diff --git a/src/main/java/org/mockito/internal/matchers/LessThan.java b/src/main/java/org/mockito/internal/matchers/LessThan.java index a14727c03b..c0c931e263 100644 --- a/src/main/java/org/mockito/internal/matchers/LessThan.java +++ b/src/main/java/org/mockito/internal/matchers/LessThan.java @@ -9,7 +9,7 @@ public class LessThan> extends CompareTo implements Serializable { - public LessThan(Comparable value) { + public LessThan(T value) { super(value); } diff --git a/src/test/java/org/mockito/internal/util/collections/HashCodeAndEqualsSafeSetTest.java b/src/test/java/org/mockito/internal/util/collections/HashCodeAndEqualsSafeSetTest.java index e8dc0c2688..d2c209c85c 100644 --- a/src/test/java/org/mockito/internal/util/collections/HashCodeAndEqualsSafeSetTest.java +++ b/src/test/java/org/mockito/internal/util/collections/HashCodeAndEqualsSafeSetTest.java @@ -4,41 +4,53 @@ */ package org.mockito.internal.util.collections; -import org.junit.Test; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Observer; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; public class HashCodeAndEqualsSafeSetTest { + @Rule + public MockitoRule r = MockitoJUnit.rule(); + @Mock + private UnmockableHashCodeAndEquals mock1; + @Test public void can_add_mock_that_have_failing_hashCode_method() throws Exception { - new HashCodeAndEqualsSafeSet().add(mock(UnmockableHashCodeAndEquals.class)); + new HashCodeAndEqualsSafeSet().add(mock1); } @Test public void mock_with_failing_hashCode_method_can_be_added() throws Exception { - new HashCodeAndEqualsSafeSet().add(mock(UnmockableHashCodeAndEquals.class)); + new HashCodeAndEqualsSafeSet().add(mock1); } @Test public void mock_with_failing_equals_method_can_be_used() throws Exception { HashCodeAndEqualsSafeSet mocks = new HashCodeAndEqualsSafeSet(); - UnmockableHashCodeAndEquals mock = mock(UnmockableHashCodeAndEquals.class); - mocks.add(mock); + mocks.add(mock1); + + assertThat(mocks.contains(mock1)).isTrue(); - assertThat(mocks.contains(mock)).isTrue(); - assertThat(mocks.contains(mock(UnmockableHashCodeAndEquals.class))).isFalse(); + UnmockableHashCodeAndEquals mock2 = mock(UnmockableHashCodeAndEquals.class); + assertThat(mocks.contains(mock2)).isFalse(); } @Test public void can_remove() throws Exception { HashCodeAndEqualsSafeSet mocks = new HashCodeAndEqualsSafeSet(); - UnmockableHashCodeAndEquals mock = mock(UnmockableHashCodeAndEquals.class); + UnmockableHashCodeAndEquals mock = mock1; mocks.add(mock); mocks.remove(mock); @@ -49,7 +61,7 @@ public void can_remove() throws Exception { @Test public void can_add_a_collection() throws Exception { HashCodeAndEqualsSafeSet mocks = HashCodeAndEqualsSafeSet.of( - mock(UnmockableHashCodeAndEquals.class), + mock1, mock(Observer.class)); HashCodeAndEqualsSafeSet workingSet = new HashCodeAndEqualsSafeSet(); @@ -62,7 +74,7 @@ public void can_add_a_collection() throws Exception { @Test public void can_retain_a_collection() throws Exception { HashCodeAndEqualsSafeSet mocks = HashCodeAndEqualsSafeSet.of( - mock(UnmockableHashCodeAndEquals.class), + mock1, mock(Observer.class)); HashCodeAndEqualsSafeSet workingSet = new HashCodeAndEqualsSafeSet(); @@ -77,7 +89,7 @@ public void can_retain_a_collection() throws Exception { @Test public void can_remove_a_collection() throws Exception { HashCodeAndEqualsSafeSet mocks = HashCodeAndEqualsSafeSet.of( - mock(UnmockableHashCodeAndEquals.class), + mock1, mock(Observer.class)); HashCodeAndEqualsSafeSet workingSet = new HashCodeAndEqualsSafeSet(); @@ -92,7 +104,7 @@ public void can_remove_a_collection() throws Exception { @Test public void can_iterate() throws Exception { HashCodeAndEqualsSafeSet mocks = HashCodeAndEqualsSafeSet.of( - mock(UnmockableHashCodeAndEquals.class), + mock1, mock(Observer.class)); LinkedList accumulator = new LinkedList(); @@ -104,13 +116,66 @@ public void can_iterate() throws Exception { @Test public void toArray_just_work() throws Exception { - UnmockableHashCodeAndEquals mock1 = mock(UnmockableHashCodeAndEquals.class); HashCodeAndEqualsSafeSet mocks = HashCodeAndEqualsSafeSet.of(mock1); assertThat(mocks.toArray()[0]).isSameAs(mock1); assertThat(mocks.toArray(new UnmockableHashCodeAndEquals[0])[0]).isSameAs(mock1); } + + @Test(expected=CloneNotSupportedException.class) + public void cloneIsNotSupported() throws CloneNotSupportedException{ + HashCodeAndEqualsSafeSet.of().clone(); + } + + @Test + public void isEmptyAfterClear() throws Exception { + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + set.clear(); + + assertThat(set).isEmpty(); + } + + @Test + public void isEqualToItself(){ + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + assertThat(set).isEqualTo(set); + } + + @Test + public void isNotEqualToAnOtherTypeOfSetWithSameContent(){ + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(); + assertThat(set).isNotEqualTo(new HashSet()); + } + + @Test + public void isNotEqualWhenContentIsDifferent(){ + + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + assertThat(set).isNotEqualTo(HashCodeAndEqualsSafeSet.of()); + } + + @Test + public void hashCodeIsEqualIfContentIsEqual(){ + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + assertThat(set.hashCode()).isEqualTo(HashCodeAndEqualsSafeSet.of(mock1).hashCode()); + } + + @Test + public void toStringIsNotNullOrEmpty() throws Exception { + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + assertThat(set.toString()).isNotEmpty(); + } + + @Test + public void removeByIterator() throws Exception { + HashCodeAndEqualsSafeSet set = HashCodeAndEqualsSafeSet.of(mock1); + Iterator iterator = set.iterator(); + iterator.next(); + iterator.remove(); + + assertThat(set).isEmpty(); + } private static class UnmockableHashCodeAndEquals { @Override public final int hashCode() { @@ -121,4 +186,4 @@ private static class UnmockableHashCodeAndEquals { throw new NullPointerException("I'm failing on equals and I don't care"); } } -} +} \ No newline at end of file diff --git a/src/test/java/org/mockitousage/bugs/CompareMatcherTest.java b/src/test/java/org/mockitousage/bugs/CompareMatcherTest.java new file mode 100644 index 0000000000..0154479f89 --- /dev/null +++ b/src/test/java/org/mockitousage/bugs/CompareMatcherTest.java @@ -0,0 +1,131 @@ +package org.mockitousage.bugs; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.AdditionalMatchers.leq; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.startsWith; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Date; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.ArgumentMatcher; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockitousage.IMethods; + +public class CompareMatcherTest { + private static final Object NOT_A_COMPARABLE = new Object(); + + @Rule + public MockitoRule mockitoRule = MockitoJUnit.rule(); + + @Mock + public IMethods mock; + + /** + * Should not throw an {@link NullPointerException} + * + * @see Bug-ID https://github.com/mockito/mockito/issues/457 + */ + @Test + public void compareNullArgument() { + final IMethods mock = mock(IMethods.class); + + when(mock.forInteger(leq(5))).thenReturn(""); + + assertThat(mock.forInteger(null)).isNull();// a default value must be returned + } + + /** + * Should not throw an {@link ClassCastException} + */ + @Test + public void compareToNonCompareable() { + when(mock.forObject(leq(5))).thenReturn(""); + + assertThat(mock.forObject(NOT_A_COMPARABLE)).isNull();// a default value must be returned + } + + /** + * Should not throw an {@link ClassCastException} + */ + @Test + public void compareToNull() { + when(mock.forInteger(leq((Integer) null))).thenReturn(""); + + assertThat(mock.forInteger(null)).isNull();// a default value must be returned + } + + /** + * Should not throw an {@link ClassCastException} + */ + @Test + public void compareToStringVsInt() { + when(mock.forObject(startsWith("Hello"))).thenReturn(""); + + assertThat(mock.forObject(123)).isNull();// a default value must be returned + } + + @Test + public void compareToIntVsString() throws Exception { + when(mock.forObject(leq(5))).thenReturn(""); + + mock.forObject("abc"); + } + + @Test + public void matchesOverloadsMustBeIgnored() { + class TestMatcher implements ArgumentMatcher { + @Override + public boolean matches(Integer arg) { + return false; + } + + @SuppressWarnings("unused") + public boolean matches(Date arg) { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + public boolean matches(Integer arg, Void v) { + throw new UnsupportedOperationException(); + } + + } + + when(mock.forObject(argThat(new TestMatcher()))).thenReturn("x"); + + assertThat(mock.forObject(123)).isNull(); + } + + @Test + public void matchesWithSubTypeExtendingGenericClass() { + abstract class GenericMatcher implements ArgumentMatcher {} + class TestMatcher extends GenericMatcher { + @Override + public boolean matches(Integer argument) { + return false; + } + } + when(mock.forObject(argThat(new TestMatcher()))).thenReturn("x"); + + assertThat(mock.forObject(123)).isNull(); + } + + @Test + public void matchesWithSubTypeGenericMethod() { + class GenericMatcher implements ArgumentMatcher { + @Override + public boolean matches(T argument) { + return false; + } + } + when(mock.forObject(argThat(new GenericMatcher()))).thenReturn("x"); + + assertThat(mock.forObject(123)).isNull(); + } + +} \ No newline at end of file