From cc62bdd19821b51bf783f7bfc9cdb5e46a64e972 Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Tue, 31 May 2022 10:29:23 +0100 Subject: [PATCH 1/4] Add example failing test for issue #2656 Example failing test for https://github.com/mockito/mockito/issues/2656 --- .../java/org/mockitousage/ProductionCode.java | 11 ++++++++ .../java/org/mockitousage/StrictnessTest.java | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java diff --git a/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java b/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java new file mode 100644 index 0000000000..7c1147501c --- /dev/null +++ b/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java @@ -0,0 +1,11 @@ +package org.mockitousage; + +import java.util.function.Predicate; + +public class ProductionCode { + + @SuppressWarnings("ReturnValueIgnored") + public static void simpleMethod(Predicate mock, String argument) { + mock.test(argument); + } +} diff --git a/subprojects/junit-jupiter/src/test/java/org/mockitousage/StrictnessTest.java b/subprojects/junit-jupiter/src/test/java/org/mockitousage/StrictnessTest.java index 31a5d6a649..1b63d66cc4 100644 --- a/subprojects/junit-jupiter/src/test/java/org/mockitousage/StrictnessTest.java +++ b/subprojects/junit-jupiter/src/test/java/org/mockitousage/StrictnessTest.java @@ -21,7 +21,9 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import java.util.Optional; import java.util.function.Function; +import java.util.function.Predicate; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; @@ -162,6 +164,29 @@ void inherits_strictness_from_base_class() { assertThat(result.getStatus()).isEqualTo(TestExecutionResult.Status.SUCCESSFUL); } + @ExtendWith(MockitoExtension.class) + @MockitoSettings(strictness = Strictness.LENIENT) + static class LenientMockitoSettings { + + @Mock + private Predicate rootMock; + + @Test + void should_not_throw_on_potential_stubbing_issue() { + Mockito.doReturn(true).when(rootMock).test("Foo"); + + ProductionCode.simpleMethod(rootMock, "Bar"); + } + } + + @Test + void use_strictness_from_settings_annotation() { + TestExecutionResult result = invokeTestClassAndRetrieveMethodResult(LenientMockitoSettings.class); + + assertThat(result.getThrowable()).isEqualTo(Optional.empty()); + assertThat(result.getStatus()).isEqualTo(TestExecutionResult.Status.SUCCESSFUL); + } + private TestExecutionResult invokeTestClassAndRetrieveMethodResult(Class clazz) { LauncherDiscoveryRequest request = LauncherDiscoveryRequestBuilder.request() .selectors( From d0bf836510a44ddeb65d6492f2fa3d35f3c3caf3 Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Tue, 31 May 2022 23:25:12 +0100 Subject: [PATCH 2/4] Add fix fixes: #2656 --- src/main/java/org/mockito/Mock.java | 45 ++++++++++++-- .../MockAnnotationProcessor.java | 5 +- .../internal/creation/MockSettingsImpl.java | 2 +- .../creation/settings/CreationSettings.java | 2 +- src/test/java/org/mockito/MockTest.java | 32 ++++++++++ .../StrictnessMockAnnotationTest.java | 58 ++++++++++++++----- .../java/org/mockitousage/ProductionCode.java | 4 ++ 7 files changed, 126 insertions(+), 22 deletions(-) create mode 100644 src/test/java/org/mockito/MockTest.java diff --git a/src/main/java/org/mockito/Mock.java b/src/main/java/org/mockito/Mock.java index 7c3e036083..fd9f76bee8 100644 --- a/src/main/java/org/mockito/Mock.java +++ b/src/main/java/org/mockito/Mock.java @@ -13,7 +13,6 @@ import java.lang.annotation.Target; import org.mockito.junit.MockitoJUnitRunner; -import org.mockito.quality.Strictness; import org.mockito.stubbing.Answer; /** @@ -34,12 +33,13 @@ * @Mock(name = "database") private ArticleDatabase dbMock; * @Mock(answer = RETURNS_MOCKS) private UserProvider userProvider; * @Mock(extraInterfaces = {Queue.class, Observer.class}) private ArticleMonitor articleMonitor; + * @Mock(strictness = Mock.Strictness.LENIENT) private ArticleConsumer articleConsumer; * @Mock(stubOnly = true) private Logger logger; * * private ArticleManager manager; * * @Before public void setup() { - * manager = new ArticleManager(userProvider, database, calculator, articleMonitor, logger); + * manager = new ArticleManager(userProvider, database, calculator, articleMonitor, articleConsumer, logger); * } * } * @@ -117,10 +117,45 @@ boolean lenient() default false; /** - * Mock will have custom strictness, see {@link MockSettings#strictness(Strictness)}. + * Mock will have custom strictness, see {@link MockSettings#strictness(org.mockito.quality.Strictness)}. * For examples how to use 'Mock' annotation and parameters see {@link Mock}. * - * @since 4.6.0 + * @since 4.6.1 */ - Strictness strictness() default Strictness.STRICT_STUBS; + Strictness strictness() default Strictness.NOT_SET; + + enum Strictness { + + /** + * Default value used to indicate the mock does not override the test level strictness. + * + * @since 4.6.1 + */ + NOT_SET(null), + + /** + * See {@link org.mockito.quality.Strictness#LENIENT} + */ + LENIENT(org.mockito.quality.Strictness.LENIENT), + + /** + * See {@link org.mockito.quality.Strictness#WARN} + */ + WARN(org.mockito.quality.Strictness.WARN), + + /** + * See {@link org.mockito.quality.Strictness#STRICT_STUBS} + */ + STRICT_STUBS(org.mockito.quality.Strictness.STRICT_STUBS); + + private final org.mockito.quality.Strictness outer; + + Strictness(org.mockito.quality.Strictness outer) { + this.outer = outer; + } + + public org.mockito.quality.Strictness outer() { + return outer; + } + } } diff --git a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java index 484c9102e4..7cf2877325 100644 --- a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java +++ b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java @@ -28,6 +28,7 @@ public Object process(Mock annotation, Field field) { annotation, field.getType(), field::getGenericType, field.getName()); } + @SuppressWarnings("deprecation") public static Object processAnnotationForMock( Mock annotation, Class type, Supplier genericType, String name) { MockSettings mockSettings = Mockito.withSettings(); @@ -45,10 +46,12 @@ public static Object processAnnotationForMock( if (annotation.stubOnly()) { mockSettings.stubOnly(); } - mockSettings.strictness(annotation.strictness()); if (annotation.lenient()) { mockSettings.lenient(); } + if (annotation.strictness() != Mock.Strictness.NOT_SET) { + mockSettings.strictness(annotation.strictness().outer()); + } // see @Mock answer default value mockSettings.defaultAnswer(annotation.answer()); diff --git a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java index e1dcd48f9e..f73a718298 100644 --- a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java +++ b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java @@ -247,10 +247,10 @@ public MockSettings lenient() { @Override public MockSettings strictness(Strictness strictness) { - this.strictness = strictness; if (strictness == null) { throw strictnessDoesNotAcceptNullParameter(); } + this.strictness = strictness; return this; } diff --git a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java index 50bb937696..13939f6fbd 100644 --- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java +++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java @@ -45,7 +45,7 @@ public class CreationSettings implements MockCreationSettings, Serializabl private boolean useConstructor; private Object outerClassInstance; private Object[] constructorArgs; - protected Strictness strictness = Strictness.STRICT_STUBS; + protected Strictness strictness = null; public CreationSettings() {} diff --git a/src/test/java/org/mockito/MockTest.java b/src/test/java/org/mockito/MockTest.java new file mode 100644 index 0000000000..484c4011b1 --- /dev/null +++ b/src/test/java/org/mockito/MockTest.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2022 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(value = Parameterized.class) +public class MockTest { + + public org.mockito.quality.Strictness strictness; + + public MockTest(org.mockito.quality.Strictness strictness) { + this.strictness = strictness; + } + + @Test + public void should_have_matching_enum_in_mock_strictness_enum() { + final Mock.Strictness mockStrictness = Mock.Strictness.valueOf(this.strictness.name()); + assertThat(mockStrictness.outer()).isEqualTo(strictness); + } + + @Parameterized.Parameters + public static org.mockito.quality.Strictness[] data() { + return org.mockito.quality.Strictness.values(); + } +} diff --git a/src/test/java/org/mockitousage/strictness/StrictnessMockAnnotationTest.java b/src/test/java/org/mockitousage/strictness/StrictnessMockAnnotationTest.java index c035b3d708..7250555880 100644 --- a/src/test/java/org/mockitousage/strictness/StrictnessMockAnnotationTest.java +++ b/src/test/java/org/mockitousage/strictness/StrictnessMockAnnotationTest.java @@ -7,6 +7,8 @@ import org.assertj.core.api.Assertions; import org.junit.Rule; import org.junit.Test; +import org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.exceptions.misusing.PotentialStubbingProblem; import org.mockito.junit.MockitoJUnit; @@ -16,28 +18,56 @@ import static org.mockito.Mockito.when; +@RunWith(Enclosed.class) public class StrictnessMockAnnotationTest { - public @Rule MockitoRule rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); + public static class StrictStubsTest { + public @Rule MockitoRule rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); - @Mock(strictness = Strictness.LENIENT) - IMethods lenientMock; + @Mock(strictness = Mock.Strictness.LENIENT) + IMethods lenientMock; - @Mock IMethods regularMock; + @Mock IMethods regularMock; - @Test - public void mock_is_lenient() { - when(lenientMock.simpleMethod("1")).thenReturn("1"); + @Test + public void mock_is_lenient() { + when(lenientMock.simpleMethod("1")).thenReturn("1"); - // then lenient mock does not throw: - ProductionCode.simpleMethod(lenientMock, "3"); + // then lenient mock does not throw: + ProductionCode.simpleMethod(lenientMock, "3"); + } + + @Test + public void mock_is_strict() { + when(regularMock.simpleMethod("2")).thenReturn("2"); + + Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(regularMock, "4")) + .isInstanceOf(PotentialStubbingProblem.class); + } } - @Test - public void mock_is_strict() { - when(regularMock.simpleMethod("2")).thenReturn("2"); + public static class LenientStubsTest { + public @Rule MockitoRule rule = MockitoJUnit.rule().strictness(Strictness.LENIENT); + + @Mock IMethods lenientMock; + + @Mock(strictness = Mock.Strictness.STRICT_STUBS) + IMethods regularMock; + + @Test + public void mock_is_lenient() { + when(lenientMock.simpleMethod("1")).thenReturn("1"); + + // then lenient mock does not throw: + ProductionCode.simpleMethod(lenientMock, "3"); + } + + @Test + public void mock_is_strict() { + when(regularMock.simpleMethod("2")).thenReturn("2"); - Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(regularMock, "4")) - .isInstanceOf(PotentialStubbingProblem.class); + Assertions.assertThatThrownBy(() -> ProductionCode.simpleMethod(regularMock, "4")) + .isInstanceOf(PotentialStubbingProblem.class); + } } } diff --git a/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java b/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java index 7c1147501c..f2953c7bc6 100644 --- a/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java +++ b/subprojects/junit-jupiter/src/test/java/org/mockitousage/ProductionCode.java @@ -1,3 +1,7 @@ +/* + * Copyright (c) 2022 Mockito contributors + * This program is made available under the terms of the MIT License. + */ package org.mockitousage; import java.util.function.Predicate; From 36a27c5306c1057b90e1848b351f0e8ed1e0c162 Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Wed, 1 Jun 2022 18:05:37 +0100 Subject: [PATCH 3/4] Changes requested in review --- src/main/java/org/mockito/Mock.java | 18 ++----- .../MockAnnotationProcessor.java | 3 +- src/test/java/org/mockito/MockTest.java | 52 ++++++++++++++----- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/mockito/Mock.java b/src/main/java/org/mockito/Mock.java index fd9f76bee8..eaeddc57fd 100644 --- a/src/main/java/org/mockito/Mock.java +++ b/src/main/java/org/mockito/Mock.java @@ -131,31 +131,21 @@ enum Strictness { * * @since 4.6.1 */ - NOT_SET(null), + NOT_SET, /** * See {@link org.mockito.quality.Strictness#LENIENT} */ - LENIENT(org.mockito.quality.Strictness.LENIENT), + LENIENT, /** * See {@link org.mockito.quality.Strictness#WARN} */ - WARN(org.mockito.quality.Strictness.WARN), + WARN, /** * See {@link org.mockito.quality.Strictness#STRICT_STUBS} */ - STRICT_STUBS(org.mockito.quality.Strictness.STRICT_STUBS); - - private final org.mockito.quality.Strictness outer; - - Strictness(org.mockito.quality.Strictness outer) { - this.outer = outer; - } - - public org.mockito.quality.Strictness outer() { - return outer; - } + STRICT_STUBS } } diff --git a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java index 7cf2877325..e2a62d4076 100644 --- a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java +++ b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java @@ -17,6 +17,7 @@ import org.mockito.Mockito; import org.mockito.exceptions.base.MockitoException; import org.mockito.internal.util.Supplier; +import org.mockito.quality.Strictness; /** * Instantiates a mock on a field annotated by {@link Mock} @@ -50,7 +51,7 @@ public static Object processAnnotationForMock( mockSettings.lenient(); } if (annotation.strictness() != Mock.Strictness.NOT_SET) { - mockSettings.strictness(annotation.strictness().outer()); + mockSettings.strictness(Strictness.valueOf(annotation.strictness().toString())); } // see @Mock answer default value diff --git a/src/test/java/org/mockito/MockTest.java b/src/test/java/org/mockito/MockTest.java index 484c4011b1..1ef19425b6 100644 --- a/src/test/java/org/mockito/MockTest.java +++ b/src/test/java/org/mockito/MockTest.java @@ -4,29 +4,57 @@ */ package org.mockito; +import org.hamcrest.Matchers; import org.junit.Test; +import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assume.assumeThat; -@RunWith(value = Parameterized.class) +@RunWith(Enclosed.class) public class MockTest { - public org.mockito.quality.Strictness strictness; + @RunWith(value = Parameterized.class) + public static class StrictnessToMockStrictnessTest { - public MockTest(org.mockito.quality.Strictness strictness) { - this.strictness = strictness; - } + public org.mockito.quality.Strictness strictness; + + public StrictnessToMockStrictnessTest(org.mockito.quality.Strictness strictness) { + this.strictness = strictness; + } - @Test - public void should_have_matching_enum_in_mock_strictness_enum() { - final Mock.Strictness mockStrictness = Mock.Strictness.valueOf(this.strictness.name()); - assertThat(mockStrictness.outer()).isEqualTo(strictness); + @Test + public void should_have_matching_enum_in_mock_strictness_enum() { + Mock.Strictness.valueOf(strictness.name()); + } + + @Parameterized.Parameters(name = "{0}") + public static org.mockito.quality.Strictness[] data() { + return org.mockito.quality.Strictness.values(); + } } - @Parameterized.Parameters - public static org.mockito.quality.Strictness[] data() { - return org.mockito.quality.Strictness.values(); + @RunWith(value = Parameterized.class) + public static class MockStrictnessToStrictnessTest { + + public Mock.Strictness strictness; + + public MockStrictnessToStrictnessTest(Mock.Strictness strictness) { + this.strictness = strictness; + } + + @Test + public void should_have_matching_enum_in_strictness_enum() { + assumeThat("Ignore NOT_SET", strictness, not(Mock.Strictness.NOT_SET)); + org.mockito.quality.Strictness.valueOf(strictness.name()); + } + + @Parameterized.Parameters(name = "{0}") + public static Mock.Strictness[] data() { + return Mock.Strictness.values(); + } } } From c7af1faa4778e18301c25c4ae72b039b89eafc3d Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Wed, 1 Jun 2022 22:37:55 +0100 Subject: [PATCH 4/4] Final change requested in review --- src/main/java/org/mockito/Mock.java | 4 ++-- .../internal/configuration/MockAnnotationProcessor.java | 2 +- src/test/java/org/mockito/MockTest.java | 6 ++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/mockito/Mock.java b/src/main/java/org/mockito/Mock.java index eaeddc57fd..e8469b830d 100644 --- a/src/main/java/org/mockito/Mock.java +++ b/src/main/java/org/mockito/Mock.java @@ -122,7 +122,7 @@ * * @since 4.6.1 */ - Strictness strictness() default Strictness.NOT_SET; + Strictness strictness() default Strictness.TEST_LEVEL_DEFAULT; enum Strictness { @@ -131,7 +131,7 @@ enum Strictness { * * @since 4.6.1 */ - NOT_SET, + TEST_LEVEL_DEFAULT, /** * See {@link org.mockito.quality.Strictness#LENIENT} diff --git a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java index e2a62d4076..66c3f31d12 100644 --- a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java +++ b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java @@ -50,7 +50,7 @@ public static Object processAnnotationForMock( if (annotation.lenient()) { mockSettings.lenient(); } - if (annotation.strictness() != Mock.Strictness.NOT_SET) { + if (annotation.strictness() != Mock.Strictness.TEST_LEVEL_DEFAULT) { mockSettings.strictness(Strictness.valueOf(annotation.strictness().toString())); } diff --git a/src/test/java/org/mockito/MockTest.java b/src/test/java/org/mockito/MockTest.java index 1ef19425b6..e904ec5d45 100644 --- a/src/test/java/org/mockito/MockTest.java +++ b/src/test/java/org/mockito/MockTest.java @@ -4,14 +4,12 @@ */ package org.mockito; -import org.hamcrest.Matchers; import org.junit.Test; import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.not; import static org.junit.Assume.assumeThat; @RunWith(Enclosed.class) @@ -48,7 +46,7 @@ public MockStrictnessToStrictnessTest(Mock.Strictness strictness) { @Test public void should_have_matching_enum_in_strictness_enum() { - assumeThat("Ignore NOT_SET", strictness, not(Mock.Strictness.NOT_SET)); + assumeThat("Ignore NOT_SET", strictness, not(Mock.Strictness.TEST_LEVEL_DEFAULT)); org.mockito.quality.Strictness.valueOf(strictness.name()); }