Repository: mockito/mockito
Issue: Undesired invocation message improvements
Labels: please contribute

Let’s take a look at the following code.

```kotlin
package whatever

import org.junit.Test
import org.mockito.Mockito

class MultipleUndesiredInvocations {

    interface Inner {
        fun call()
    }

    interface SideEffect {
        fun call()

        class Impl(private val inner: Inner) : SideEffect {
            override fun call() = inner.call()
        }
    }

    interface Outer {
        fun call()

        class Impl(private val inner: Inner, sideEffect: SideEffect) : Outer {

            init {
                sideEffect.call()
            }

            override fun call() {
                inner.call()
            }
        }
    }

    @Test
    fun test() {
        val inner = Mockito.mock(Inner::class.java)
        val sideEffect = SideEffect.Impl(inner)
        val outer = Outer.Impl(inner, sideEffect)

        outer.call()

        Mockito.verify(inner).call()
    }
}
```
The `test` will fail, of course. The following message will be shown.
```
org.mockito.exceptions.verification.TooManyActualInvocations: 
inner.call();
Wanted 1 time:
-> at whatever.MultipleUndesiredInvocations.test(MultipleUndesiredInvocations.kt:43)
But was 2 times. Undesired invocation:
-> at whatever.MultipleUndesiredInvocations$Outer$Impl.call(MultipleUndesiredInvocations.kt:30)
```
It is kind of fine, but the undesired call (for me) is actually inside the `SideEffect` class.

Is it possible to extend the message to show all undesired invocations somehow? It would be extremely helpful when debugging failed tests similar to the sample above — i. e. when a side-effect calls the target object. 

Issue comments:
Good idea. Do you want to submit a PR?
@mockitoguy I can work on it.
Also, to make it consistent I would suggest to extend the message to include all the invocations for other cases. As example for `TooLittleActualInvocations`.
Submitted PR #1319 

Pull request review comments:
Not sure if it follows the convention since the `*` was used before and `java.*` imports were listed below.
`invocations` → `locations`?
Indentation might have changed.
`infocation` → `location`?
`allLocations` → `locations`? At this point it not entirely consistent with other calls if I understand correctly.
Same thing with imports as above.
As I can see the import layouts that was used before violates the conventions that are described here: https://github.com/mockito/mockito/blob/release/2.x/.github/CONTRIBUTING.md#coding-style
Should I revert it or is it better to follow the conventions that are in the guide?
Here the imports also violates the described import layout
Done
Done
Done
Done
Let's put this assert in a separate test

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/exceptions/Reporter.java b/src/main/java/org/mockito/internal/exceptions/Reporter.java
index 57094c0fa0..17823ffe5e 100644
--- a/src/main/java/org/mockito/internal/exceptions/Reporter.java
+++ b/src/main/java/org/mockito/internal/exceptions/Reporter.java
@@ -5,9 +5,27 @@
 
 package org.mockito.internal.exceptions;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
 import org.mockito.exceptions.base.MockitoAssertionError;
 import org.mockito.exceptions.base.MockitoException;
-import org.mockito.exceptions.misusing.*;
+import org.mockito.exceptions.misusing.CannotStubVoidMethodWithReturnValue;
+import org.mockito.exceptions.misusing.CannotVerifyStubOnlyMock;
+import org.mockito.exceptions.misusing.FriendlyReminderException;
+import org.mockito.exceptions.misusing.InvalidUseOfMatchersException;
+import org.mockito.exceptions.misusing.MissingMethodInvocationException;
+import org.mockito.exceptions.misusing.NotAMockException;
+import org.mockito.exceptions.misusing.NullInsteadOfMockException;
+import org.mockito.exceptions.misusing.PotentialStubbingProblem;
+import org.mockito.exceptions.misusing.RedundantListenerException;
+import org.mockito.exceptions.misusing.UnfinishedMockingSessionException;
+import org.mockito.exceptions.misusing.UnfinishedStubbingException;
+import org.mockito.exceptions.misusing.UnfinishedVerificationException;
+import org.mockito.exceptions.misusing.UnnecessaryStubbingException;
+import org.mockito.exceptions.misusing.WrongTypeOfReturnValue;
 import org.mockito.exceptions.verification.NeverWantedButInvoked;
 import org.mockito.exceptions.verification.NoInteractionsWanted;
 import org.mockito.exceptions.verification.SmartNullPointerException;
@@ -28,12 +46,6 @@
 import org.mockito.mock.MockName;
 import org.mockito.mock.SerializableMode;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-
 import static org.mockito.internal.reporting.Pluralizer.pluralize;
 import static org.mockito.internal.reporting.Pluralizer.were_exactly_x_interactions;
 import static org.mockito.internal.util.StringUtil.join;
@@ -358,63 +370,71 @@ public static MockitoAssertionError wantedButNotInvokedInOrder(DescribedInvocati
         ));
     }
 
-    public static MockitoAssertionError tooManyActualInvocations(int wantedCount, int actualCount, DescribedInvocation wanted, Location firstUndesired) {
-        String message = createTooManyInvocationsMessage(wantedCount, actualCount, wanted, firstUndesired);
+    public static MockitoAssertionError tooManyActualInvocations(int wantedCount, int actualCount, DescribedInvocation wanted, List<Location> locations) {
+        String message = createTooManyInvocationsMessage(wantedCount, actualCount, wanted, locations);
         return new TooManyActualInvocations(message);
     }
 
     private static String createTooManyInvocationsMessage(int wantedCount, int actualCount, DescribedInvocation wanted,
-                                                          Location firstUndesired) {
+                                                          List<Location> invocations) {
         return join(
                 wanted.toString(),
                 "Wanted " + pluralize(wantedCount) + ":",
                 new LocationImpl(),
-                "But was " + pluralize(actualCount) + ". Undesired invocation:",
-                firstUndesired,
+                "But was " + pluralize(actualCount) + ":",
+                createAllLocationsMessage(invocations),
                 ""
         );
     }
 
-    public static MockitoAssertionError neverWantedButInvoked(DescribedInvocation wanted, Location firstUndesired) {
+    public static MockitoAssertionError neverWantedButInvoked(DescribedInvocation wanted, List<Location> invocations) {
         return new NeverWantedButInvoked(join(
                 wanted.toString(),
                 "Never wanted here:",
                 new LocationImpl(),
                 "But invoked here:",
-                firstUndesired,
-                ""
+                createAllLocationsMessage(invocations)
         ));
     }
 
-    public static MockitoAssertionError tooManyActualInvocationsInOrder(int wantedCount, int actualCount, DescribedInvocation wanted, Location firstUndesired) {
-        String message = createTooManyInvocationsMessage(wantedCount, actualCount, wanted, firstUndesired);
+    public static MockitoAssertionError tooManyActualInvocationsInOrder(int wantedCount, int actualCount, DescribedInvocation wanted, List<Location> invocations) {
+        String message = createTooManyInvocationsMessage(wantedCount, actualCount, wanted, invocations);
         return new VerificationInOrderFailure(join(
                 "Verification in order failure:" + message
         ));
     }
 
-    private static String createTooLittleInvocationsMessage(org.mockito.internal.reporting.Discrepancy discrepancy, DescribedInvocation wanted,
-                                                            Location lastActualInvocation) {
-        String ending =
-                (lastActualInvocation != null) ? lastActualInvocation + "\n" : "\n";
+    private static String createAllLocationsMessage(List<Location> locations) {
+        if (locations == null) {
+            return "\n";
+        }
+        StringBuilder sb = new StringBuilder();
+        for (Location location : locations) {
+            sb.append(location).append("\n");
+        }
+        return sb.toString();
+    }
 
+    private static String createTooLittleInvocationsMessage(org.mockito.internal.reporting.Discrepancy discrepancy,
+                                                            DescribedInvocation wanted,
+                                                            List<Location> locations) {
         return join(
                 wanted.toString(),
                 "Wanted " + discrepancy.getPluralizedWantedCount() + (discrepancy.getWantedCount() == 0 ? "." : ":"),
                 new LocationImpl(),
                 "But was " + discrepancy.getPluralizedActualCount() + (discrepancy.getActualCount() == 0 ? "." : ":"),
-                ending
+                createAllLocationsMessage(locations)
         );
     }
 
-    public static MockitoAssertionError tooLittleActualInvocations(org.mockito.internal.reporting.Discrepancy discrepancy, DescribedInvocation wanted, Location lastActualLocation) {
-        String message = createTooLittleInvocationsMessage(discrepancy, wanted, lastActualLocation);
+    public static MockitoAssertionError tooLittleActualInvocations(org.mockito.internal.reporting.Discrepancy discrepancy, DescribedInvocation wanted, List<Location> allLocations) {
+        String message = createTooLittleInvocationsMessage(discrepancy, wanted, allLocations);
 
         return new TooLittleActualInvocations(message);
     }
 
-    public static MockitoAssertionError tooLittleActualInvocationsInOrder(org.mockito.internal.reporting.Discrepancy discrepancy, DescribedInvocation wanted, Location lastActualLocation) {
-        String message = createTooLittleInvocationsMessage(discrepancy, wanted, lastActualLocation);
+    public static MockitoAssertionError tooLittleActualInvocationsInOrder(org.mockito.internal.reporting.Discrepancy discrepancy, DescribedInvocation wanted, List<Location> locations) {
+        String message = createTooLittleInvocationsMessage(discrepancy, wanted, locations);
 
         return new VerificationInOrderFailure(join(
                 "Verification in order failure:" + message
diff --git a/src/main/java/org/mockito/internal/invocation/InvocationsFinder.java b/src/main/java/org/mockito/internal/invocation/InvocationsFinder.java
index 57c335fcef..260321babb 100644
--- a/src/main/java/org/mockito/internal/invocation/InvocationsFinder.java
+++ b/src/main/java/org/mockito/internal/invocation/InvocationsFinder.java
@@ -138,6 +138,14 @@ private static List<Invocation> removeVerifiedInOrder(List<Invocation> invocatio
         return unverified;
     }
 
+    public static List<Location> getAllLocations(List<Invocation> invocations) {
+        List<Location> locations = new LinkedList<Location>();
+        for (Invocation invocation : invocations) {
+            locations.add(invocation.getLocation());
+        }
+        return locations;
+    }
+
     private static class RemoveNotMatching implements Filter<Invocation> {
         private final MatchableInvocation wanted;
 
diff --git a/src/main/java/org/mockito/internal/verification/checkers/AtLeastXNumberOfInvocationsChecker.java b/src/main/java/org/mockito/internal/verification/checkers/AtLeastXNumberOfInvocationsChecker.java
index 36e35a36d1..d0eb0b5eb8 100644
--- a/src/main/java/org/mockito/internal/verification/checkers/AtLeastXNumberOfInvocationsChecker.java
+++ b/src/main/java/org/mockito/internal/verification/checkers/AtLeastXNumberOfInvocationsChecker.java
@@ -17,7 +17,7 @@
 import static org.mockito.internal.invocation.InvocationMarker.markVerifiedInOrder;
 import static org.mockito.internal.invocation.InvocationsFinder.findAllMatchingUnverifiedChunks;
 import static org.mockito.internal.invocation.InvocationsFinder.findInvocations;
-import static org.mockito.internal.invocation.InvocationsFinder.getLastLocation;
+import static org.mockito.internal.invocation.InvocationsFinder.getAllLocations;
 
 public class AtLeastXNumberOfInvocationsChecker {
 
@@ -26,8 +26,8 @@ public static void checkAtLeastNumberOfInvocations(List<Invocation> invocations,
 
         int actualCount = actualInvocations.size();
         if (wantedCount > actualCount) {
-            Location lastLocation = getLastLocation(actualInvocations);
-            throw tooLittleActualInvocations(new AtLeastDiscrepancy(wantedCount, actualCount), wanted, lastLocation);
+            List<Location> allLocations = getAllLocations(actualInvocations);
+            throw tooLittleActualInvocations(new AtLeastDiscrepancy(wantedCount, actualCount), wanted, allLocations);
         }
 
         markVerified(actualInvocations, wanted);
@@ -39,8 +39,8 @@ public static void checkAtLeastNumberOfInvocations(List<Invocation> invocations,
         int actualCount = chunk.size();
 
         if (wantedCount > actualCount) {
-            Location lastLocation = getLastLocation(chunk);
-            throw tooLittleActualInvocationsInOrder(new AtLeastDiscrepancy(wantedCount, actualCount), wanted, lastLocation);
+            List<Location> allLocations = getAllLocations(chunk);
+            throw tooLittleActualInvocationsInOrder(new AtLeastDiscrepancy(wantedCount, actualCount), wanted, allLocations);
         }
 
         markVerifiedInOrder(chunk, wanted, orderingContext);
diff --git a/src/main/java/org/mockito/internal/verification/checkers/NumberOfInvocationsChecker.java b/src/main/java/org/mockito/internal/verification/checkers/NumberOfInvocationsChecker.java
index 3fa340f3dc..1dfc2f1f25 100644
--- a/src/main/java/org/mockito/internal/verification/checkers/NumberOfInvocationsChecker.java
+++ b/src/main/java/org/mockito/internal/verification/checkers/NumberOfInvocationsChecker.java
@@ -5,6 +5,7 @@
 
 package org.mockito.internal.verification.checkers;
 
+import java.util.Arrays;
 import java.util.List;
 import org.mockito.internal.reporting.Discrepancy;
 import org.mockito.internal.verification.api.InOrderContext;
@@ -22,7 +23,7 @@
 import static org.mockito.internal.invocation.InvocationsFinder.findFirstMatchingUnverifiedInvocation;
 import static org.mockito.internal.invocation.InvocationsFinder.findInvocations;
 import static org.mockito.internal.invocation.InvocationsFinder.findMatchingChunk;
-import static org.mockito.internal.invocation.InvocationsFinder.getLastLocation;
+import static org.mockito.internal.invocation.InvocationsFinder.getAllLocations;
 
 public class NumberOfInvocationsChecker {
 
@@ -34,16 +35,14 @@ public static void checkNumberOfInvocations(List<Invocation> invocations, Matcha
 
         int actualCount = actualInvocations.size();
         if (wantedCount > actualCount) {
-            Location lastInvocation = getLastLocation(actualInvocations);
-            throw tooLittleActualInvocations(new Discrepancy(wantedCount, actualCount), wanted, lastInvocation);
+            List<Location> allLocations = getAllLocations(actualInvocations);
+            throw tooLittleActualInvocations(new Discrepancy(wantedCount, actualCount), wanted, allLocations);
         }
         if (wantedCount == 0 && actualCount > 0) {
-            Location firstUndesired = actualInvocations.get(wantedCount).getLocation();
-            throw neverWantedButInvoked(wanted, firstUndesired);
+            throw neverWantedButInvoked(wanted, getAllLocations(actualInvocations));
         }
         if (wantedCount < actualCount) {
-            Location firstUndesired = actualInvocations.get(wantedCount).getLocation();
-            throw tooManyActualInvocations(wantedCount, actualCount, wanted, firstUndesired);
+            throw tooManyActualInvocations(wantedCount, actualCount, wanted, getAllLocations(actualInvocations));
         }
 
         markVerified(actualInvocations, wanted);
@@ -55,12 +54,11 @@ public static void checkNumberOfInvocations(List<Invocation> invocations, Matcha
         int actualCount = chunk.size();
 
         if (wantedCount > actualCount) {
-            Location lastInvocation = getLastLocation(chunk);
-            throw tooLittleActualInvocationsInOrder(new Discrepancy(wantedCount, actualCount), wanted, lastInvocation);
+            List<Location> allLocations = getAllLocations(chunk);
+            throw tooLittleActualInvocationsInOrder(new Discrepancy(wantedCount, actualCount), wanted, allLocations);
         }
         if (wantedCount < actualCount) {
-            Location firstUndesired = chunk.get(wantedCount).getLocation();
-            throw tooManyActualInvocationsInOrder(wantedCount, actualCount, wanted, firstUndesired);
+            throw tooManyActualInvocationsInOrder(wantedCount, actualCount, wanted, getAllLocations(chunk));
         }
 
         markVerifiedInOrder(chunk, wanted, context);
@@ -72,7 +70,7 @@ public static void checkNumberOfInvocationsNonGreedy(List<Invocation> invocation
         while( actualCount < wantedCount ){
             Invocation next = findFirstMatchingUnverifiedInvocation(invocations, wanted, context );
             if( next == null ){
-                throw tooLittleActualInvocationsInOrder(new Discrepancy(wantedCount, actualCount), wanted, lastLocation );
+                throw tooLittleActualInvocationsInOrder(new Discrepancy(wantedCount, actualCount), wanted, Arrays.asList(lastLocation));
             }
             markVerified( next, wanted );
             context.markVerified( next );


Test patch:
diff --git a/src/test/java/org/mockito/internal/invocation/InvocationsFinderTest.java b/src/test/java/org/mockito/internal/invocation/InvocationsFinderTest.java
index b07ece43cd..7172998c7e 100644
--- a/src/test/java/org/mockito/internal/invocation/InvocationsFinderTest.java
+++ b/src/test/java/org/mockito/internal/invocation/InvocationsFinderTest.java
@@ -185,4 +185,15 @@ public void shouldFindPreviousInOrder() throws Exception {
         previous = InvocationsFinder.findPreviousVerifiedInOrder(invocations, context);
         assertSame(simpleMethodInvocationTwo, previous);
     }
+
+    @Test
+    public void shouldFindAllStackTraces() {
+        List<Location> all = InvocationsFinder.getAllLocations(invocations);
+        Assertions.assertThat(all).contains(simpleMethodInvocation.getLocation(), simpleMethodInvocationTwo.getLocation(), differentMethodInvocation.getLocation());
+    }
+
+    @Test
+    public void shouldNotFindLocationsForEmptyInvocationsList() {
+        Assertions.assertThat(InvocationsFinder.getAllLocations(Collections.<Invocation>emptyList())).isEmpty();
+    }
 }
diff --git a/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsCheckerTest.java b/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsCheckerTest.java
index af9d3cb801..61b2f2e7b4 100644
--- a/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsCheckerTest.java
+++ b/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsCheckerTest.java
@@ -5,10 +5,6 @@
 
 package org.mockito.internal.verification.checkers;
 
-import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
-import static org.assertj.core.api.Assertions.assertThat;
-
 import java.util.List;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
@@ -28,6 +24,10 @@
 import org.mockito.junit.MockitoJUnitRunner;
 import org.mockitousage.IMethods;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static org.assertj.core.api.Assertions.assertThat;
+
 @RunWith(MockitoJUnitRunner.class)
 public class NumberOfInvocationsCheckerTest {
 
@@ -58,7 +58,7 @@ public void shouldReportTooLittleActual() throws Exception {
     }
 
     @Test
-    public void shouldReportWithLastInvocationStackTrace() throws Exception {
+    public void shouldReportAllInvocationsStackTrace() throws Exception {
         wanted = buildSimpleMethod().toInvocationMatcher();
         invocations = asList(buildSimpleMethod().toInvocation(), buildSimpleMethod().toInvocation());
 
@@ -66,7 +66,7 @@ public void shouldReportWithLastInvocationStackTrace() throws Exception {
         exception.expectMessage("mock.simpleMethod()");
         exception.expectMessage("Wanted 100 times");
         exception.expectMessage("But was 2 times");
-        exception.expectMessage(containsTimes("-> at", 2));
+        exception.expectMessage(containsTimes("-> at", 3));
 
         NumberOfInvocationsChecker.checkNumberOfInvocations(invocations, wanted, 100);
     }
@@ -86,7 +86,7 @@ public void shouldNotReportWithLastInvocationStackTraceIfNoInvocationsFound() th
     }
 
     @Test
-    public void shouldReportWithFirstUndesiredInvocationStackTrace() throws Exception {
+    public void shouldReportWithAllInvocationsStackTrace() throws Exception {
         Invocation first = buildSimpleMethod().toInvocation();
         Invocation second = buildSimpleMethod().toInvocation();
         Invocation third = buildSimpleMethod().toInvocation();
@@ -95,6 +95,8 @@ public void shouldReportWithFirstUndesiredInvocationStackTrace() throws Exceptio
         wanted = buildSimpleMethod().toInvocationMatcher();
 
         exception.expect(TooManyActualInvocations.class);
+        exception.expectMessage("" + first.getLocation());
+        exception.expectMessage("" + second.getLocation());
         exception.expectMessage("" + third.getLocation());
         NumberOfInvocationsChecker.checkNumberOfInvocations(invocations, wanted, 2);
     }
diff --git a/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsInOrderCheckerTest.java b/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsInOrderCheckerTest.java
index 2e72033238..bc031ac904 100644
--- a/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsInOrderCheckerTest.java
+++ b/src/test/java/org/mockito/internal/verification/checkers/NumberOfInvocationsInOrderCheckerTest.java
@@ -5,11 +5,6 @@
 
 package org.mockito.internal.verification.checkers;
 
-import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-
 import java.util.List;
 
 import org.hamcrest.BaseMatcher;
@@ -27,6 +22,11 @@
 import org.mockito.invocation.Invocation;
 import org.mockitousage.IMethods;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
 public class NumberOfInvocationsInOrderCheckerTest {
 
     private InvocationMatcher wanted;
@@ -102,7 +102,7 @@ public void shouldReportTooLittleActual() throws Exception {
     }
 
     @Test
-    public void shouldReportWithLastInvocationStackTrace() throws Exception {
+    public void shouldReportWithAllInvocationsStackTrace() throws Exception {
         wanted = buildSimpleMethod().toInvocationMatcher();
         invocations = asList(buildSimpleMethod().toInvocation(), buildSimpleMethod().toInvocation());
 
@@ -110,7 +110,7 @@ public void shouldReportWithLastInvocationStackTrace() throws Exception {
         exception.expectMessage("mock.simpleMethod()");
         exception.expectMessage("Wanted 100 times");
         exception.expectMessage("But was 2 times");
-        exception.expectMessage(containsTimes("-> at", 2));
+        exception.expectMessage(containsTimes("-> at", 3));
 
         NumberOfInvocationsChecker.checkNumberOfInvocations(invocations, wanted, 100, context);
 
@@ -168,7 +168,7 @@ public void shouldReportNeverWantedButInvoked() throws Exception {
         exception.expect(VerificationInOrderFailure.class);
         exception.expectMessage("mock.simpleMethod()");
         exception.expectMessage("Wanted 0 times");
-        exception.expectMessage("But was 1 time. Undesired invocation");
+        exception.expectMessage("But was 1 time:");
         exception.expectMessage("" + first.getLocation());
 
         NumberOfInvocationsChecker.checkNumberOfInvocations(invocations, wanted, 0, context);
diff --git a/src/test/java/org/mockitousage/verification/DescriptiveMessagesOnVerificationInOrderErrorsTest.java b/src/test/java/org/mockitousage/verification/DescriptiveMessagesOnVerificationInOrderErrorsTest.java
index 4e5f464c53..8e1bf06322 100644
--- a/src/test/java/org/mockitousage/verification/DescriptiveMessagesOnVerificationInOrderErrorsTest.java
+++ b/src/test/java/org/mockitousage/verification/DescriptiveMessagesOnVerificationInOrderErrorsTest.java
@@ -155,7 +155,7 @@ public void shouldPrintTooManyInvocations() {
 
             String expectedCause =
                 "\n" +
-                "But was 2 times. Undesired invocation:" +
+                "But was 2 times:" +
                 "\n" +
                 "-> at";
             assertThat(e).hasMessageContaining(expectedCause);