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

Fixes mockito#2311 #2320

Merged
merged 4 commits into from Jun 26, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Expand Up @@ -18,9 +18,20 @@ public interface ContainsExtraTypeInfo {
*/
String toStringWithType();

/**
* Returns more verbose description of the object which include type information and fully qualified class name
*/
String toStringWithFullName();

/**
* Checks if target target has matching type.
* If the type matches, there is no point in rendering result from {@link #toStringWithType()}
*/
boolean typeMatches(Object target);

/**
* Checks if target target's class has same simple name.
* If the simple names matches, we need to use {@link #toStringWithFullName()}
*/
boolean sameName(Object target);
}
10 changes: 10 additions & 0 deletions src/main/java/org/mockito/internal/matchers/Equals.java
Expand Up @@ -55,8 +55,18 @@ public String toStringWithType() {
return "(" + wanted.getClass().getSimpleName() + ") " + describe(wanted);
}

@Override
public String toStringWithFullName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not the biggest fan of adding another method here. Especially as it is very similar to the toStringWithType(), but the only difference is the name retrieval.

What if instead we get the wanted object and perform the computation in the MatchersPrinter? E.g. avoid introductions of new methods and pass in the desired name into toStringWithType(String className)

return "(" + wanted.getClass().getCanonicalName() + ") " + describe(wanted);
}

@Override
public boolean typeMatches(Object target) {
return wanted != null && target != null && target.getClass() == wanted.getClass();
}

@Override
public boolean sameName(Object target) {
return wanted != null && target != null && target.getClass().getSimpleName().equals(wanted.getClass().getSimpleName());
}
}
Expand Up @@ -32,6 +32,8 @@ private Iterator<FormattedText> applyPrintSettings(
for (final ArgumentMatcher matcher : matchers) {
if (matcher instanceof ContainsExtraTypeInfo && printSettings.extraTypeInfoFor(i)) {
out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithType()));
} else if(matcher instanceof ContainsExtraTypeInfo && printSettings.fullyQualifiedNameFor(i)){
out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithFullName()));
} else {
out.add(new FormattedText(MatcherToString.toString(matcher)));
}
Expand Down
Expand Up @@ -19,6 +19,7 @@ public class PrintSettings {
public static final int MAX_LINE_LENGTH = 45;
private boolean multiline;
private List<Integer> withTypeInfo = new LinkedList<>();
private List<Integer> withFullyQualifiedName = new LinkedList<>();

public void setMultiline(boolean multiline) {
this.multiline = multiline;
Expand All @@ -38,10 +39,18 @@ public boolean extraTypeInfoFor(int argumentIndex) {
return withTypeInfo.contains(argumentIndex);
}

public boolean fullyQualifiedNameFor(int argumentIndex) {
return withFullyQualifiedName.contains(argumentIndex);
}

public void setMatchersToBeDescribedWithExtraTypeInfo(Integer[] indexesOfMatchers) {
this.withTypeInfo = Arrays.asList(indexesOfMatchers);
}

public void setMatchersToBeDescribedWithFullName(Integer[] indexesOfMatchers) {
this.withFullyQualifiedName= Arrays.asList(indexesOfMatchers);
}

public String print(List<ArgumentMatcher> matchers, Invocation invocation) {
MatchersPrinter matchersPrinter = new MatchersPrinter();
String qualifiedName =
Expand Down
Expand Up @@ -28,17 +28,21 @@ public SmartPrinter(
this(
wanted,
Collections.singletonList(actual),
indexesOfMatchersToBeDescribedWithExtraTypeInfo);
indexesOfMatchersToBeDescribedWithExtraTypeInfo,
new Integer[0]);
}

public SmartPrinter(
MatchableInvocation wanted,
List<Invocation> allActualInvocations,
Integer... indexesOfMatchersToBeDescribedWithExtraTypeInfo) {
Integer[] indexesOfMatchersToBeDescribedWithExtraTypeInfo,
Integer[] indexesOfMatchersToBeDescribedWithFullName) {
PrintSettings printSettings = new PrintSettings();
printSettings.setMultiline(isMultiLine(wanted, allActualInvocations));
printSettings.setMatchersToBeDescribedWithExtraTypeInfo(
indexesOfMatchersToBeDescribedWithExtraTypeInfo);
printSettings.setMatchersToBeDescribedWithFullName(
indexesOfMatchersToBeDescribedWithFullName);

this.wanted = printSettings.print(wanted);

Expand Down
Expand Up @@ -48,4 +48,28 @@ 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 Integer[] getNotMatchingArgsWithSameNameIndexes(
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than returning Integers, can we instead return all names that have duplicates? Then we can do the lookup on the name. E.g. return a Set<String> here, where the set contains all names that occur at least twice in arguments

List<ArgumentMatcher> matchers, Object[] arguments) {
if (matchers.size() != arguments.length) {
return new Integer[0];
}

List<Integer> suspicious = new LinkedList<>();
int i = 0;
for (ArgumentMatcher m : matchers) {
if (m instanceof ContainsExtraTypeInfo
&& !safelyMatches(m, arguments[i])
&& !((ContainsExtraTypeInfo) m).typeMatches(arguments[i])
&& ((ContainsExtraTypeInfo) m).sameName(arguments[i])) {
suspicious.add(i);
}
i++;
}
return suspicious.toArray(new Integer[0]);
}

}
Expand Up @@ -11,6 +11,7 @@
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.getNotMatchingArgsWithSameNameIndexes;
import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getSuspiciouslyNotMatchingArgsIndexes;

import java.util.List;
Expand Down Expand Up @@ -41,7 +42,9 @@ public static void checkMissingInvocation(

Integer[] indexesOfSuspiciousArgs =
getSuspiciouslyNotMatchingArgsIndexes(wanted.getMatchers(), similar.getArguments());
SmartPrinter smartPrinter = new SmartPrinter(wanted, invocations, indexesOfSuspiciousArgs);
Integer[] indexesOfArgsWithSameSimpleName =
getNotMatchingArgsWithSameNameIndexes(wanted.getMatchers(), similar.getArguments());
SmartPrinter smartPrinter = new SmartPrinter(wanted, invocations, indexesOfSuspiciousArgs,indexesOfArgsWithSameSimpleName);
List<Location> actualLocations =
ListUtil.convert(
invocations,
Expand Down
Expand Up @@ -115,10 +115,21 @@ public String toStringWithType() {
return "";
}

@Override
public String toStringWithFullName() {
return "";
}

@Override
public boolean typeMatches(Object target) {
return true;
}

@Override
public boolean sameName(Object target) {
return true;
}

}

// given
Expand Down