Merge pull request #23 from mockito/fix-7

Start new MockitoSession for each test methods

Author: slawekjaranowski

src/main/java/org/mockito/testng/MockitoTestNGListener.java

@@ -4,45 +4,42 @@
  */
 package org.mockito.testng;
 
+import java.util.Map;
+import java.util.Optional;
+import java.util.WeakHashMap;
+
+import org.mockito.Mockito;
 import org.mockito.MockitoSession;
-import org.mockito.testng.internal.MockitoAfterTestNGMethod;
-import org.mockito.testng.internal.MockitoBeforeTestNGMethod;
+import org.mockito.quality.Strictness;
 import org.testng.IInvokedMethod;
 import org.testng.IInvokedMethodListener;
 import org.testng.ITestNGListener;
 import org.testng.ITestResult;
 import org.testng.annotations.Listeners;
 
-import java.util.Map;
-import java.util.WeakHashMap;
-
 /**
- * Mockito TestNG Listener, this listener adds the following behavior to your test :
+ * <p>Mockito TestNG Listener, this listener initializes mocks and handles strict stubbing, it is similar to JUnit
+ * <code>MockitoJUnitRunner</code>, <code>MockitoRule</code>, <code>MockitoExtension</code> and adds
+ * the following behavior to your test:</p>
+ *
  * <ul>
  *     <li>
- *         Initializes mocks annotated with {@link org.mockito.Mock}, so that <strong>explicit usage of
- *         {@link org.mockito.MockitoAnnotations#initMocks(Object)} is not necessary</strong>.
- *         <strong>Note :</strong> With TestNG, mocks are initialized before any TestNG method, either a <em>configuration
- *         method</em> (&#064;BeforeMethod, &#064;BeforeClass, etc) or a <em>test</em> method, i.e. mocks are initialized
- *         once only once for each test instance.
+ *         Before any TestNG method, either a <em>configuration method</em> (&#064;BeforeMethod, &#064;BeforeClass, etc)
+ *         or a <em>test</em> method MockitoSession is started by:
+ *         <pre class="code"><code class="java">
+ *         Mockito.mockitoSession()
+ *          .initMocks(testInstance)
+ *          .strictness(Strictness.STRICT_STUBS)
+ *          .startMocking()
+ *         </code></pre>
+ *         See javadoc {@link MockitoSession}
  *     </li>
  *     <li>
- *         As mocks are initialized only once, they will be reset after each <em>test method</em>.
- *         See javadoc {@link org.mockito.Mockito#reset(Object[])}
- *     </li>
- *     <li>
- *         Validates framework usage after each test method. See javadoc for {@link org.mockito.Mockito#validateMockitoUsage()}.
+ *         After each <em>test</em> method {@link MockitoSession#finishMocking()} is called.
  *     </li>
  * </ul>
  *
- * <p>
- * The listener is completely optional - there are other ways you can get &#064;Mock working, for example by writing a base class.
- * Explicitly validating framework usage is also optional because it is triggered automatically by Mockito every time you use the framework.
- * See javadoc for {@link org.mockito.Mockito#validateMockitoUsage()}.
- *
- * <p>
- * Read more about &#064;Mock annotation in javadoc for {@link org.mockito.MockitoAnnotations}
- *
+ * <p>Example usage:</p>
  * <pre class="code"><code class="java">
  * <b>&#064;Listeners(MockitoTestNGListener.class)</b>
  * public class ExampleTest {
@@ -56,27 +53,38 @@
  *     }
  * }
  * </code></pre>
+ *
+ * <p>
+ * <code>MockitoTestNGListener</code> not working with parallel tests,
+ * more information https://github.com/mockito/mockito-testng/issues/20
+ * </p>
  */
 public class MockitoTestNGListener implements IInvokedMethodListener {
 
     private final Map<Object, MockitoSession> sessions = new WeakHashMap<>();
-    private final MockitoBeforeTestNGMethod beforeTest = new MockitoBeforeTestNGMethod(sessions);
-    private final MockitoAfterTestNGMethod afterTest = new MockitoAfterTestNGMethod(sessions);
 
+    @Override
     public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
-        if (hasMockitoTestNGListenerInTestHierarchy(testResult.getTestClass().getRealClass())) {
-            beforeTest.applyFor(method, testResult);
+        if (hasMockitoTestNGListenerInTestHierarchy(testResult)) {
+            sessions.computeIfAbsent(testResult.getInstance(), testInstance ->
+                    Mockito.mockitoSession()
+                            .initMocks(testInstance)
+                            .strictness(Strictness.STRICT_STUBS)
+                            .startMocking()
+            );
         }
     }
 
+    @Override
     public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
-        if (hasMockitoTestNGListenerInTestHierarchy(testResult.getTestClass().getRealClass())) {
-            afterTest.applyFor(method, testResult);
+        if (hasMockitoTestNGListenerInTestHierarchy(testResult) && method.isTestMethod()) {
+            Optional.ofNullable(sessions.remove(testResult.getInstance()))
+                    .ifPresent(mockitoSession -> mockitoSession.finishMocking(testResult.getThrowable()));
         }
     }
 
-    protected boolean hasMockitoTestNGListenerInTestHierarchy(Class<?> testClass) {
-        for (Class<?> clazz = testClass; clazz != Object.class; clazz = clazz.getSuperclass()) {
+    protected boolean hasMockitoTestNGListenerInTestHierarchy(ITestResult testResult) {
+        for (Class<?> clazz = testResult.getTestClass().getRealClass(); clazz != Object.class; clazz = clazz.getSuperclass()) {
             if (hasMockitoTestNGListener(clazz)) {
                 return true;
             }

src/main/java/org/mockito/testng/internal/MockitoAfterTestNGMethod.java

@@ -4,28 +4,31 @@
  */
 package org.mockitousage.testng;
 
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
 import org.assertj.core.api.Assertions;
 import org.mockito.Mock;
 import org.mockito.exceptions.misusing.PotentialStubbingProblem;
 import org.mockito.exceptions.misusing.UnnecessaryStubbingException;
 import org.mockito.testng.MockitoTestNGListener;
 import org.mockitousage.testng.failuretests.HasUnusedStubs;
+import org.mockitousage.testng.failuretests.HasUnusedStubsInSetup;
 import org.mockitousage.testng.utils.TestNGRunner;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
-import java.util.List;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.when;
-
 @Listeners(MockitoTestNGListener.class)
 public class StrictStubsTest {
 
-    @Mock List list;
+    @Mock
+    List<String> list;
 
-    @Test public void detects_potential_stubbing_problem() {
+    @Test
+    public void detects_potential_stubbing_problem() {
         when(list.add("a")).thenReturn(true);
 
         Assertions.assertThatThrownBy(() -> StrictStubsCode.testStrictStubs(list, "b"))
@@ -40,7 +43,17 @@ public class StrictStubsTest {
         assertThat(result.getFailure()).isInstanceOf(UnnecessaryStubbingException.class);
     }
 
-    @Test public void keeps_tests_dry() {
+    @Test
+    public void detects_unused_stubs_in_setup() {
+        //when
+        TestNGRunner.Result result = new TestNGRunner().run(HasUnusedStubsInSetup.class);
+
+        //then
+        assertThat(result.getFailure()).isInstanceOf(UnnecessaryStubbingException.class);
+    }
+
+    @Test
+    public void keeps_tests_dry() {
         //when
         when(list.add("a")).thenReturn(true);
         list.add("a");

src/main/java/org/mockito/testng/internal/MockitoBeforeTestNGMethod.java

@@ -4,16 +4,15 @@
  */
 package org.mockitousage.testng.failuretests;
 
-import org.mockito.exceptions.misusing.InvalidUseOfMatchersException;
-import org.mockito.testng.MockitoTestNGListener;
-import org.testng.annotations.Listeners;
-import org.testng.annotations.Test;
+import static org.mockito.ArgumentMatchers.anySet;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
 
 import java.io.PrintStream;
 
-import static org.mockito.Matchers.anySet;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.mock;
+import org.mockito.testng.MockitoTestNGListener;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
 
 /**
  * Should fail.
@@ -24,10 +23,7 @@
 @Test(description = "Always failing, shouldn't be listed in 'mockito-testng.xml'")
 public class FailingOnPurposeBecauseIncorrectStubbingSyntax {
 
-    @SuppressWarnings("CheckReturnValue")
-    @Test(expectedExceptions = InvalidUseOfMatchersException.class)
-    public void incorrect_stubbing_syntax_in_test() throws Exception {
-        mock(PrintStream.class);
+    public void incorrect_stubbing_syntax_in_test() {
         anyString();
         anySet();
     }

src/test/java/org/mockitousage/testng/StrictStubsTest.java

@@ -0,0 +1,35 @@
+package org.mockitousage.testng.failuretests;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import org.mockito.Mock;
+import org.mockito.testng.MockitoTestNGListener;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+@Listeners(MockitoTestNGListener.class)
+public class HasUnusedStubsInSetup {
+
+    @Mock
+    List<String> mock;
+
+    @BeforeMethod
+    public void setup() {
+        when(mock.add("a")).thenReturn(true);
+    }
+
+    @Test(priority = 1)
+    public void testPass() {
+        assertThat(mock.add("a")).isTrue();
+    }
+
+    @Test(priority = 2)
+    public void testFail() {
+        // stub from setup is not used
+    }
+
+}