Fixes mockito#2311 #2320
Fixes mockito#2311 #2320
Conversation
Prints fully qualified class name when the simple names of arguments match.
| @@ -55,8 +55,18 @@ public String toStringWithType() { | |||
| return "(" + wanted.getClass().getSimpleName() + ") " + describe(wanted); | |||
| } | |||
|
|
|||
| @Override | |||
| public String toStringWithFullName() { | |||
There was a problem hiding this comment.
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)
| /** | ||
| * Suspiciously not matching arguments are those that don't match, and the classes have same simple name. | ||
| */ | ||
| public static Integer[] getNotMatchingArgsWithSameNameIndexes( |
There was a problem hiding this comment.
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
Made changes to Equals.toStringWithType by reusing same method to achieve both cases by sending a boolean as an input, based on which either simple name or fully qualified name will be used for describing the type. Also in the method ArgumentMatchingTool.getNotMatchingArgsWithSameName return Set<String> which return the simple names of classes having more than one classes with different FQCN.
There was a problem hiding this comment.
We are almost there!
Lastly, can we add another test to https://github.com/mockito/mockito/blob/6f9108b833323345bd926f10914224d3e0a11b80/src/test/java/org/mockitousage/verification/DescriptiveMessagesWhenVerificationFailsTest.java which verifies that if a method takes two of the same arguments the output is correct? I think if you take should_print_method_when_matcher_used as a starting point, it should be relatively straightforward to add more tests with the following cases:
- 2 arguments with the same simple type name
- 2 arguments with different simple type names
- 3 arguments of which the first and last have the same simple type name, but the middle one has a different one.
src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java
Outdated
Show resolved
Hide resolved
In Equals have passed qualified class name instead of the boolean. Also removed getWantedClass and instead made getWanted public and used it. Have also added 3 test cases for cases where some arguments have same simple class name.
| @@ -386,6 +388,118 @@ 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() { | |||
There was a problem hiding this comment.
I am loving these tests, thanks!
|
Oh, you need to run |
Fixed the style issues, by running gradlew :spotlessApply Also needed to handle the case where argument is null, which gave null pointer while getting class name from the object.
|
Sorry, missed to run the gradle check before pushing. |
Codecov Report
@@ Coverage Diff @@
## main #2320 +/- ##
============================================
+ Coverage 84.65% 84.88% +0.22%
- Complexity 2768 2794 +26
============================================
Files 328 328
Lines 8428 8480 +52
Branches 1011 1025 +14
============================================
+ Hits 7135 7198 +63
+ Misses 1015 1004 -11
Partials 278 278
Continue to review full report at Codecov.
|
|
100% test coverage on the diff. Nice job! Merging this 🎉 |
|
Nice!!! |
Prints fully qualified class name when the simple names of arguments match.
Checklist
including project members to get a better picture of the change
commit is meaningful and help the people that will explore a change in 2 years
Fixes #<issue number>in the description if relevantFixes #<issue number>if relevant