From 1ad82350765566b2afabbab9ec15ad5ccd54951a Mon Sep 17 00:00:00 2001 From: saurabh7248 Date: Sat, 26 Jun 2021 19:42:15 +0530 Subject: [PATCH] Print fully qualified class name when the simple names of arguments match (#2320) Co-authored-by: thisisdexter --- .../matchers/ContainsExtraTypeInfo.java | 13 +- .../org/mockito/internal/matchers/Equals.java | 7 +- .../matchers/text/MatchersPrinter.java | 21 ++- .../internal/reporting/PrintSettings.java | 11 ++ .../internal/reporting/SmartPrinter.java | 8 +- .../ArgumentMatchingTool.java | 38 ++++++ .../checkers/MissingInvocationChecker.java | 8 +- .../mockito/internal/matchers/EqualsTest.java | 6 +- .../ArgumentMatchingToolTest.java | 7 +- src/test/java/org/mockitousage/IMethods.java | 48 +++++++ .../java/org/mockitousage/MethodsImpl.java | 20 +++ ...tiveMessagesWhenVerificationFailsTest.java | 120 ++++++++++++++++++ 12 files changed, 292 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java b/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java index 3b69fbcf9c..8c698fdccf 100644 --- a/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java +++ b/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java @@ -9,18 +9,25 @@ * When ArgumentMatcher fails, chance is that the actual object has the same output of toString() than * the wanted object. This looks weird when failures are reported. * Therefore when matcher fails but toString() yields the same outputs, - * we will try to use the {@link #toStringWithType()} method. + * we will try to use the {@link #toStringWithType(String)} method. */ public interface ContainsExtraTypeInfo { /** + * @param className - name of the class to be printed in description * Returns more verbose description of the object which include type information */ - String toStringWithType(); + String toStringWithType(String className); /** * Checks if target target has matching type. - * If the type matches, there is no point in rendering result from {@link #toStringWithType()} + * If the type matches, there is no point in rendering result from {@link #toStringWithType(String)} */ boolean typeMatches(Object target); + + /** + * + * @return Returns the wanted argument + */ + Object getWanted(); } diff --git a/src/main/java/org/mockito/internal/matchers/Equals.java b/src/main/java/org/mockito/internal/matchers/Equals.java index d04131d414..8b07b2da75 100644 --- a/src/main/java/org/mockito/internal/matchers/Equals.java +++ b/src/main/java/org/mockito/internal/matchers/Equals.java @@ -31,7 +31,8 @@ private String describe(Object object) { return ValuePrinter.print(object); } - protected final Object getWanted() { + @Override + public final Object getWanted() { return wanted; } @@ -51,8 +52,8 @@ public int hashCode() { } @Override - public String toStringWithType() { - return "(" + wanted.getClass().getSimpleName() + ") " + describe(wanted); + public String toStringWithType(String className) { + return "(" + className + ") " + describe(wanted); } @Override diff --git a/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java b/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java index 589b072edb..9ceab39755 100644 --- a/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java +++ b/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java @@ -30,8 +30,25 @@ private Iterator applyPrintSettings( List out = new LinkedList<>(); int i = 0; for (final ArgumentMatcher matcher : matchers) { - if (matcher instanceof ContainsExtraTypeInfo && printSettings.extraTypeInfoFor(i)) { - out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithType())); + if (matcher instanceof ContainsExtraTypeInfo) { + ContainsExtraTypeInfo typeInfoMatcher = (ContainsExtraTypeInfo) matcher; + Object wanted = typeInfoMatcher.getWanted(); + String simpleNameOfArgument = + wanted != null ? wanted.getClass().getSimpleName() : ""; + String fullyQualifiedClassName = + wanted != null ? wanted.getClass().getCanonicalName() : ""; + + if (printSettings.extraTypeInfoFor(i)) { + out.add( + new FormattedText( + typeInfoMatcher.toStringWithType(simpleNameOfArgument))); + } else if (printSettings.fullyQualifiedNameFor(simpleNameOfArgument)) { + out.add( + new FormattedText( + typeInfoMatcher.toStringWithType(fullyQualifiedClassName))); + } else { + out.add(new FormattedText(MatcherToString.toString(matcher))); + } } else { out.add(new FormattedText(MatcherToString.toString(matcher))); } diff --git a/src/main/java/org/mockito/internal/reporting/PrintSettings.java b/src/main/java/org/mockito/internal/reporting/PrintSettings.java index 2869ed316c..41794adcb0 100644 --- a/src/main/java/org/mockito/internal/reporting/PrintSettings.java +++ b/src/main/java/org/mockito/internal/reporting/PrintSettings.java @@ -5,8 +5,10 @@ package org.mockito.internal.reporting; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Set; import org.mockito.ArgumentMatcher; import org.mockito.internal.matchers.text.MatchersPrinter; @@ -19,6 +21,7 @@ public class PrintSettings { public static final int MAX_LINE_LENGTH = 45; private boolean multiline; private List withTypeInfo = new LinkedList<>(); + private Set withFullyQualifiedName = Collections.emptySet(); public void setMultiline(boolean multiline) { this.multiline = multiline; @@ -38,10 +41,18 @@ public boolean extraTypeInfoFor(int argumentIndex) { return withTypeInfo.contains(argumentIndex); } + public boolean fullyQualifiedNameFor(String simpleClassName) { + return withFullyQualifiedName.contains(simpleClassName); + } + public void setMatchersToBeDescribedWithExtraTypeInfo(Integer[] indexesOfMatchers) { this.withTypeInfo = Arrays.asList(indexesOfMatchers); } + public void setMatchersToBeDescribedWithFullName(Set indexesOfMatchers) { + this.withFullyQualifiedName = indexesOfMatchers; + } + public String print(List matchers, Invocation invocation) { MatchersPrinter matchersPrinter = new MatchersPrinter(); String qualifiedName = diff --git a/src/main/java/org/mockito/internal/reporting/SmartPrinter.java b/src/main/java/org/mockito/internal/reporting/SmartPrinter.java index 71a7c369a4..f4b1f71d40 100644 --- a/src/main/java/org/mockito/internal/reporting/SmartPrinter.java +++ b/src/main/java/org/mockito/internal/reporting/SmartPrinter.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import org.mockito.invocation.Invocation; import org.mockito.invocation.MatchableInvocation; @@ -28,17 +29,20 @@ public SmartPrinter( this( wanted, Collections.singletonList(actual), - indexesOfMatchersToBeDescribedWithExtraTypeInfo); + indexesOfMatchersToBeDescribedWithExtraTypeInfo, + Collections.emptySet()); } public SmartPrinter( MatchableInvocation wanted, List allActualInvocations, - Integer... indexesOfMatchersToBeDescribedWithExtraTypeInfo) { + Integer[] indexesOfMatchersToBeDescribedWithExtraTypeInfo, + Set classNamesToBeDescribedWithFullName) { PrintSettings printSettings = new PrintSettings(); printSettings.setMultiline(isMultiLine(wanted, allActualInvocations)); printSettings.setMatchersToBeDescribedWithExtraTypeInfo( indexesOfMatchersToBeDescribedWithExtraTypeInfo); + printSettings.setMatchersToBeDescribedWithFullName(classNamesToBeDescribedWithFullName); this.wanted = printSettings.print(wanted); diff --git a/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java b/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java index f13d980659..060ea0913e 100644 --- a/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java +++ b/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java @@ -4,8 +4,13 @@ */ package org.mockito.internal.verification.argumentmatching; +import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import org.mockito.ArgumentMatcher; import org.mockito.internal.matchers.ContainsExtraTypeInfo; @@ -48,4 +53,37 @@ private static boolean safelyMatches(ArgumentMatcher m, Object arg) { private static boolean toStringEquals(ArgumentMatcher m, Object arg) { return m.toString().equals(String.valueOf(arg)); } + + /** + * Suspiciously not matching arguments are those that don't match, and the classes have same simple name. + */ + public static Set getNotMatchingArgsWithSameName( + List matchers, Object[] arguments) { + Map> classesHavingSameName = new HashMap<>(); + for (ArgumentMatcher m : matchers) { + if (m instanceof ContainsExtraTypeInfo) { + Object wanted = ((ContainsExtraTypeInfo) m).getWanted(); + if (wanted == null) { + continue; + } + Class wantedClass = wanted.getClass(); + classesHavingSameName + .computeIfAbsent(wantedClass.getSimpleName(), className -> new HashSet<>()) + .add(wantedClass.getCanonicalName()); + } + } + for (Object argument : arguments) { + if (argument == null) { + continue; + } + Class wantedClass = argument.getClass(); + classesHavingSameName + .computeIfAbsent(wantedClass.getSimpleName(), className -> new HashSet<>()) + .add(wantedClass.getCanonicalName()); + } + return classesHavingSameName.entrySet().stream() + .filter(classEntry -> classEntry.getValue().size() > 1) + .map(classEntry -> classEntry.getKey()) + .collect(Collectors.toSet()); + } } diff --git a/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java b/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java index e34555621d..aeb86b962d 100644 --- a/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java +++ b/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java @@ -11,9 +11,11 @@ import static org.mockito.internal.invocation.InvocationsFinder.findInvocations; import static org.mockito.internal.invocation.InvocationsFinder.findPreviousVerifiedInOrder; import static org.mockito.internal.invocation.InvocationsFinder.findSimilarInvocation; +import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getNotMatchingArgsWithSameName; import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getSuspiciouslyNotMatchingArgsIndexes; import java.util.List; +import java.util.Set; import org.mockito.internal.reporting.SmartPrinter; import org.mockito.internal.util.collections.ListUtil; @@ -41,7 +43,11 @@ public static void checkMissingInvocation( Integer[] indexesOfSuspiciousArgs = getSuspiciouslyNotMatchingArgsIndexes(wanted.getMatchers(), similar.getArguments()); - SmartPrinter smartPrinter = new SmartPrinter(wanted, invocations, indexesOfSuspiciousArgs); + Set classesWithSameSimpleName = + getNotMatchingArgsWithSameName(wanted.getMatchers(), similar.getArguments()); + SmartPrinter smartPrinter = + new SmartPrinter( + wanted, invocations, indexesOfSuspiciousArgs, classesWithSameSimpleName); List actualLocations = ListUtil.convert( invocations, diff --git a/src/test/java/org/mockito/internal/matchers/EqualsTest.java b/src/test/java/org/mockito/internal/matchers/EqualsTest.java index 26f87c2dbc..ba5c578b57 100644 --- a/src/test/java/org/mockito/internal/matchers/EqualsTest.java +++ b/src/test/java/org/mockito/internal/matchers/EqualsTest.java @@ -28,21 +28,21 @@ public void shouldArraysBeEqual() { @Test public void shouldDescribeWithExtraTypeInfo() throws Exception { - String descStr = new Equals(100).toStringWithType(); + String descStr = new Equals(100).toStringWithType(Integer.class.getSimpleName()); assertEquals("(Integer) 100", descStr); } @Test public void shouldDescribeWithExtraTypeInfoOfLong() throws Exception { - String descStr = new Equals(100L).toStringWithType(); + String descStr = new Equals(100L).toStringWithType(Long.class.getSimpleName()); assertEquals("(Long) 100L", descStr); } @Test public void shouldDescribeWithTypeOfString() throws Exception { - String descStr = new Equals("x").toStringWithType(); + String descStr = new Equals("x").toStringWithType(String.class.getSimpleName()); assertEquals("(String) \"x\"", descStr); } diff --git a/src/test/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java b/src/test/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java index d54c7f4cbc..5f55faf3b2 100644 --- a/src/test/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java +++ b/src/test/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java @@ -111,7 +111,7 @@ public boolean matches(String item) { } @Override - public String toStringWithType() { + public String toStringWithType(String className) { return ""; } @@ -119,6 +119,11 @@ public String toStringWithType() { public boolean typeMatches(Object target) { return true; } + + @Override + public Object getWanted() { + return ""; + } } // given diff --git a/src/test/java/org/mockitousage/IMethods.java b/src/test/java/org/mockitousage/IMethods.java index 5a12b47ba9..12897422da 100644 --- a/src/test/java/org/mockitousage/IMethods.java +++ b/src/test/java/org/mockitousage/IMethods.java @@ -238,4 +238,52 @@ String simpleMethod( String forObject(Object object); String genericToString(T arg); + + void overloadedMethodWithSameClassNameArguments(java.sql.Date javaDate, Date date); + + void overloadedMethodWithSameClassNameArguments(Date date, java.sql.Date javaDate); + + void overloadedMethodWithDifferentClassNameArguments(String String, Integer i); + + void overloadedMethodWithDifferentClassNameArguments(Integer i, String string); + + void overloadedMethodWithSameClassNameArguments( + java.sql.Date javaDate, String string, Date date); + + void overloadedMethodWithSameClassNameArguments( + Date date, String string, java.sql.Date javaDate); + + /** + * Using this class to test cases where two classes have same simple name + */ + public static class Date { + + private int value; + + public Date(int value) { + this.value = value; + } + + @Override + public String toString() { + return String.valueOf(value); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Date date = (Date) o; + return value == date.value; + } + + @Override + public int hashCode() { + return Objects.hash(value); + } + } } diff --git a/src/test/java/org/mockitousage/MethodsImpl.java b/src/test/java/org/mockitousage/MethodsImpl.java index eafb91b093..bb69656712 100644 --- a/src/test/java/org/mockitousage/MethodsImpl.java +++ b/src/test/java/org/mockitousage/MethodsImpl.java @@ -448,4 +448,24 @@ public Void voidReturningMethod() { public String genericToString(T arg) { return null; } + + @Override + public void overloadedMethodWithSameClassNameArguments(java.sql.Date javaDate, Date date) {} + + @Override + public void overloadedMethodWithSameClassNameArguments(Date date, java.sql.Date javaDate) {} + + @Override + public void overloadedMethodWithDifferentClassNameArguments(String string, Integer i) {} + + @Override + public void overloadedMethodWithDifferentClassNameArguments(Integer i, String string) {} + + @Override + public void overloadedMethodWithSameClassNameArguments( + java.sql.Date javaDate, String string, Date date) {} + + @Override + public void overloadedMethodWithSameClassNameArguments( + Date date, String string, java.sql.Date javaDate) {} } diff --git a/src/test/java/org/mockitousage/verification/DescriptiveMessagesWhenVerificationFailsTest.java b/src/test/java/org/mockitousage/verification/DescriptiveMessagesWhenVerificationFailsTest.java index f0350f93cf..94f660284c 100644 --- a/src/test/java/org/mockitousage/verification/DescriptiveMessagesWhenVerificationFailsTest.java +++ b/src/test/java/org/mockitousage/verification/DescriptiveMessagesWhenVerificationFailsTest.java @@ -9,6 +9,8 @@ import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.Mockito.*; +import java.sql.Date; + import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -386,6 +388,124 @@ public void should_never_break_method_string_when_no_args_in_method() throws Exc } } + @Test + public void should_print_fully_qualified_name_when_arguments_classes_have_same_simple_name() { + try { + mock.overloadedMethodWithSameClassNameArguments(new Date(0), new IMethods.Date(0)); + verify(mock) + .overloadedMethodWithSameClassNameArguments(new IMethods.Date(0), new Date(0)); + fail(); + } catch (Throwable e) { + String wanted = + "\n" + + "Argument(s) are different! Wanted:" + + "\n" + + "iMethods.overloadedMethodWithSameClassNameArguments(" + + "\n" + + " (org.mockitousage.IMethods.Date) 0," + + "\n" + + " (java.sql.Date) 1970-01-01" + + "\n" + + ");"; + + assertThat(e).hasMessageContaining(wanted); + + String actual = + "\n" + + "Actual invocations have different arguments:" + + "\n" + + "iMethods.overloadedMethodWithSameClassNameArguments(" + + "\n" + + " (java.sql.Date) 1970-01-01," + + "\n" + + " (org.mockitousage.IMethods.Date) 0" + + "\n" + + ");"; + assertThat(e).hasMessageContaining(actual); + } + } + + @Test + public void + should_not_print_fully_qualified_name_when_arguments_classes_have_different_simple_name() { + try { + mock.overloadedMethodWithDifferentClassNameArguments("string", 0); + verify(mock).overloadedMethodWithDifferentClassNameArguments(0, "string"); + fail(); + } catch (Throwable e) { + String wanted = + "\n" + + "Argument(s) are different! Wanted:" + + "\n" + + "iMethods.overloadedMethodWithDifferentClassNameArguments(" + + "\n" + + " 0," + + "\n" + + " \"string\"" + + "\n" + + ");"; + + assertThat(e).hasMessageContaining(wanted); + + String actual = + "\n" + + "Actual invocations have different arguments:" + + "\n" + + "iMethods.overloadedMethodWithDifferentClassNameArguments(" + + "\n" + + " \"string\"," + + "\n" + + " 0" + + "\n" + + ");"; + assertThat(e).hasMessageContaining(actual); + } + } + + @Test + public void + should_print_fully_qualified_name_when_some_arguments_classes_have_same_simple_name() { + try { + mock.overloadedMethodWithSameClassNameArguments( + new Date(0), "string", new IMethods.Date(0)); + verify(mock) + .overloadedMethodWithSameClassNameArguments( + new IMethods.Date(0), "string", new Date(0)); + fail(); + } catch (Throwable e) { + String wanted = + "\n" + + "Argument(s) are different! Wanted:" + + "\n" + + "iMethods.overloadedMethodWithSameClassNameArguments(" + + "\n" + + " (org.mockitousage.IMethods.Date) 0," + + "\n" + + " \"string\"," + + "\n" + + " (java.sql.Date) 1970-01-01" + + "\n" + + ");"; + + assertThat(e).hasMessageContaining(wanted); + + String actual = + "\n" + + "Actual invocations have different arguments:" + + "\n" + + "iMethods.overloadedMethodWithSameClassNameArguments(" + + "\n" + + " (java.sql.Date) 1970-01-01," + + "\n" + + " \"string\"," + + "\n" + + " (org.mockitousage.IMethods.Date) 0" + + "\n" + + ");"; + assertThat(e).hasMessageContaining(actual); + } + } + @Test @Ignore("issue 380 related") public void should_print_method_name_and_arguments_of_other_interactions_of_same_method()