From 1669d8b1c5d174dbfbbb6e04a9c614efe5e30bf8 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 10 Nov 2019 17:33:53 +0100 Subject: [PATCH 1/2] Guard against multiple evaluations of before statement Some rules evaluate the base statement multiple times, e.g. to execute tests repeatedly. The changes made in #1672 led to an exception in such cases because the `MockitoListener` was registered multiple times. Now, we only add the listener the first time the statement is evaluated in order to restore the old behavior. Fixes #1767. --- .../runners/DefaultInternalRunner.java | 12 ++++--- .../runners/DefaultInternalRunnerTest.java | 33 ++++++++++++++++--- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java b/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java index 93aa32d46e..59d337a20b 100644 --- a/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java +++ b/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java @@ -36,11 +36,13 @@ protected Statement withBefores(FrameworkMethod method, final Object target, Sta return new Statement() { @Override public void evaluate() throws Throwable { - // get new test listener and add it to the framework - mockitoTestListener = listenerSupplier.get(); - Mockito.framework().addListener(mockitoTestListener); - // init annotated mocks before tests - MockitoAnnotations.initMocks(target); + if (mockitoTestListener == null) { + // get new test listener and add it to the framework + mockitoTestListener = listenerSupplier.get(); + Mockito.framework().addListener(mockitoTestListener); + // init annotated mocks before tests + MockitoAnnotations.initMocks(target); + } base.evaluate(); } }; diff --git a/src/test/java/org/mockito/internal/runners/DefaultInternalRunnerTest.java b/src/test/java/org/mockito/internal/runners/DefaultInternalRunnerTest.java index b2aa95e54b..123ee2965e 100644 --- a/src/test/java/org/mockito/internal/runners/DefaultInternalRunnerTest.java +++ b/src/test/java/org/mockito/internal/runners/DefaultInternalRunnerTest.java @@ -9,11 +9,14 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestRule; import org.junit.runner.Description; import org.junit.runner.notification.Failure; import org.junit.runner.notification.RunListener; import org.junit.runner.notification.RunNotifier; +import org.junit.runners.model.Statement; import org.mockito.Mock; import org.mockito.internal.junit.MockitoTestListener; import org.mockito.internal.junit.TestFinishedEvent; @@ -34,9 +37,7 @@ public void does_not_fail_when_tests_succeeds() throws Exception { new DefaultInternalRunner(SuccessTest.class, supplier) .run(newNotifier(runListener)); - verify(runListener, never()).testFailure(any(Failure.class)); - verify(runListener, times(1)).testFinished(any(Description.class)); - verify(mockitoTestListener, only()).testFinished(any(TestFinishedEvent.class)); + verifyTestFinishedSuccessfully(); } @Test @@ -53,6 +54,18 @@ public void does_not_fail_second_test_when_first_test_fail() throws Exception { new DefaultInternalRunner(SuccessTest.class, supplier) .run(newNotifier(runListener)); + verifyTestFinishedSuccessfully(); + } + + @Test + public void does_not_fail_when_rule_invokes_statement_multiple_times() throws Exception { + new DefaultInternalRunner(TestWithRepeatingRule.class, supplier) + .run(newNotifier(runListener)); + + verifyTestFinishedSuccessfully(); + } + + private void verifyTestFinishedSuccessfully() throws Exception { verify(runListener, never()).testFailure(any(Failure.class)); verify(runListener, times(1)).testFinished(any(Description.class)); verify(mockitoTestListener, only()).testFinished(any(TestFinishedEvent.class)); @@ -64,7 +77,7 @@ private RunNotifier newNotifier(RunListener listener) { return notifier; } - public static final class SuccessTest { + public static class SuccessTest { @Test public void this_test_is_NOT_supposed_to_fail() { @@ -82,4 +95,16 @@ public void this_test_is_supposed_to_fail() { assertNotNull(system); } } + + public static final class TestWithRepeatingRule extends SuccessTest { + + @Rule + public TestRule rule = (base, description) -> new Statement() { + @Override + public void evaluate() throws Throwable { + base.evaluate(); + base.evaluate(); + } + }; + } } From 49e41aa9809e0a6b669cf3a65a673a518fb77f80 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 10 Nov 2019 20:39:18 +0100 Subject: [PATCH 2/2] Reset listener when removing it --- .../java/org/mockito/internal/runners/DefaultInternalRunner.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java b/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java index 59d337a20b..ad115f09b6 100644 --- a/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java +++ b/src/main/java/org/mockito/internal/runners/DefaultInternalRunner.java @@ -63,6 +63,7 @@ public void testFinished(Description description) throws Exception { if (mockitoTestListener != null) { Mockito.framework().removeListener(mockitoTestListener); mockitoTestListener.testFinished(new DefaultTestFinishedEvent(target, description.getMethodName(), failure)); + mockitoTestListener = null; } Mockito.validateMockitoUsage(); } catch (Throwable t) {