From faeae6ac604e0103229a2ef23e35f88e10e026dd Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 11 Feb 2019 17:46:03 +0000 Subject: [PATCH 01/42] Fix issue with mocking of java.util.* classes (#1617) * Fix issue with mocking of java.util.* classes Fixes #1615 * Fix other broken test --- .../bytebuddy/SubclassBytecodeGenerator.java | 2 +- .../AcrossClassLoaderSerializationTest.java | 2 +- .../moduletest/ModuleHandlingTest.java | 29 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java index 8968b9e8e4..093978dd22 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java @@ -98,7 +98,7 @@ public Class mockClass(MockFeatures features) { if (localMock || loader instanceof MultipleParentClassLoader && !isComingFromJDK(features.mockedType)) { typeName = features.mockedType.getName(); } else { - typeName = InjectionBase.class.getPackage().getName() + features.mockedType.getSimpleName(); + typeName = InjectionBase.class.getPackage().getName() + "." + features.mockedType.getSimpleName(); } String name = String.format("%s$%s$%d", typeName, "MockitoMock", Math.abs(random.nextInt())); diff --git a/src/test/java/org/mockitousage/serialization/AcrossClassLoaderSerializationTest.java b/src/test/java/org/mockitousage/serialization/AcrossClassLoaderSerializationTest.java index 240cf7d027..f8a262d15d 100644 --- a/src/test/java/org/mockitousage/serialization/AcrossClassLoaderSerializationTest.java +++ b/src/test/java/org/mockitousage/serialization/AcrossClassLoaderSerializationTest.java @@ -33,7 +33,7 @@ public void check_that_mock_can_be_serialized_in_a_classloader_and_deserialized_ byte[] bytes = create_mock_and_serialize_it_in_class_loader_A(); Object the_deserialized_mock = read_stream_and_deserialize_it_in_class_loader_B(bytes); - assertThat(the_deserialized_mock.getClass().getName()).startsWith("org.mockito.codegenAClassToBeMockedInThisTestOnlyAndInCallablesOnly"); + assertThat(the_deserialized_mock.getClass().getName()).startsWith("org.mockito.codegen.AClassToBeMockedInThisTestOnlyAndInCallablesOnly"); } private Object read_stream_and_deserialize_it_in_class_loader_B(byte[] bytes) throws Exception { diff --git a/subprojects/module-test/src/test/java/org/mockito/moduletest/ModuleHandlingTest.java b/subprojects/module-test/src/test/java/org/mockito/moduletest/ModuleHandlingTest.java index 6ec32893a6..ed732c8089 100644 --- a/subprojects/module-test/src/test/java/org/mockito/moduletest/ModuleHandlingTest.java +++ b/subprojects/module-test/src/test/java/org/mockito/moduletest/ModuleHandlingTest.java @@ -4,6 +4,7 @@ */ package org.mockito.moduletest; +import java.util.concurrent.locks.Lock; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.dynamic.loading.ClassInjector; @@ -91,6 +92,34 @@ public void can_define_class_in_open_reading_module() throws Exception { } } + @Test + public void can_define_class_in_open_java_util_module() throws Exception { + assumeThat(Plugins.getMockMaker() instanceof InlineByteBuddyMockMaker, is(false)); + + Path jar = modularJar(true, true, true); + ModuleLayer layer = layer(jar, true); + + ClassLoader loader = layer.findLoader("mockito.test"); + Class type = loader.loadClass("java.util.concurrent.locks.Lock"); + + ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader(loader); + try { + Class mockito = loader.loadClass(Mockito.class.getName()); + @SuppressWarnings("unchecked") + Lock mock = (Lock) mockito.getMethod("mock", Class.class).invoke(null, type); + Object stubbing = mockito.getMethod("when", Object.class).invoke(null, mock.tryLock()); + loader.loadClass(OngoingStubbing.class.getName()).getMethod("thenReturn", Object.class).invoke(stubbing, true); + + boolean relocated = !Boolean.getBoolean("org.mockito.internal.noUnsafeInjection") && ClassInjector.UsingReflection.isAvailable(); + String prefix = relocated ? "org.mockito.codegen.Lock$MockitoMock$" : "java.util.concurrent.locks.Lock$MockitoMock$"; + assertThat(mock.getClass().getName()).startsWith(prefix); + assertThat(mock.tryLock()).isEqualTo(true); + } finally { + Thread.currentThread().setContextClassLoader(contextLoader); + } + } + @Test public void inline_mock_maker_can_mock_closed_modules() throws Exception { assumeThat(Plugins.getMockMaker() instanceof InlineByteBuddyMockMaker, is(true)); From a9d998fff27bc5019ba1ad78fc068bb8a2a424b0 Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Mon, 11 Feb 2019 17:52:56 +0000 Subject: [PATCH 02/42] 2.24.2 release (previous 2.24.1) + release notes updated by CI build 3874 [ci skip-release] --- doc/release-notes/official.md | 5 +++++ version.properties | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index 58570c9b1e..ce1e9c77a0 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,10 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.2 + - 2019-02-11 - [1 commit](https://github.com/mockito/mockito/compare/v2.24.1...v2.24.2) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.2-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.2) + - Fix issue with mocking of java.util.* classes [(#1617)](https://github.com/mockito/mockito/pull/1617) + - Issue with mocking type in "java.util.*", Java 12 [(#1615)](https://github.com/mockito/mockito/issues/1615) + #### 2.24.1 - 2019-02-04 - [1 commit](https://github.com/mockito/mockito/compare/v2.24.0...v2.24.1) by [zoujinhe](https://github.com/zoujinhe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.1-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.1) - typo? ... 'thenReturn' instruction if completed -> ... 'thenReturn' instruction is completed [(#1608)](https://github.com/mockito/mockito/pull/1608) diff --git a/version.properties b/version.properties index 54435d6413..b930a88249 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.2 +version=2.24.3 #Previous version used to generate release notes delta -previousVersion=2.24.1 +previousVersion=2.24.2 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From ac9195f740280f1964382acb469b14955a08d359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Zaj=C4=85czkowski?= Date: Mon, 11 Feb 2019 22:05:48 +0100 Subject: [PATCH 03/42] Automatic dependency update with Dependabot (#1600) http://dependabot.com/ [ci skip-release] --- .dependabot/config.yml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .dependabot/config.yml diff --git a/.dependabot/config.yml b/.dependabot/config.yml new file mode 100644 index 0000000000..cd85cf642e --- /dev/null +++ b/.dependabot/config.yml @@ -0,0 +1,9 @@ +version: 1 + +update_configs: + - package_manager: "java:gradle" + directory: "/" + update_schedule: "daily" + # Redundant - default repository branch by default + target_branch: "release/2.x" + From cbedbe567d5bf902b13e8cebc4a2a2ccfef7758f Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 12 Feb 2019 13:14:01 +0000 Subject: [PATCH 04/42] Return null instead of causing a CCE (#1612) This solves a large number of edge-cases where `null` will actually remove the runtime ClassCastException. This essentially negates the whole MockitoCast ErrorProne check. We can still not support every use case, but causing a NPE instead of a CCE does not seem to make this worse. I am still running internal tests within Google to see if there are any regressions, but I already saw that some of the test failures we had with ByteBuddy were resolved with this particular patch. Note that this now fully closes #357. A previous PR resolved the same issue with ReturnsSmartNulls: #1576. Fixes #357 --- gradle/errorprone.gradle | 4 ++++ .../defaultanswers/ReturnsDeepStubs.java | 9 ++++++++ .../ReturnsGenericDeepStubsTest.java | 23 +++++++++++++++++++ .../DeepStubsSerializableTest.java | 2 +- 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/gradle/errorprone.gradle b/gradle/errorprone.gradle index 1d9cf7428e..43c8633fcd 100644 --- a/gradle/errorprone.gradle +++ b/gradle/errorprone.gradle @@ -10,3 +10,7 @@ if (JavaVersion.current() == JavaVersion.VERSION_1_8) { dependencies { errorprone libraries.errorprone } + +tasks.named("compileTestJava").configure { + options.errorprone.errorproneArgs << "-Xep:MockitoCast:OFF" +} diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java index 3909ff041c..8729171255 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java @@ -53,6 +53,15 @@ public Object answer(InvocationOnMock invocation) throws Throwable { return delegate().returnValueFor(rawType); } + // When dealing with erasured generics, we only receive the Object type as rawType. At this + // point, there is nothing to salvage for Mockito. Instead of trying to be smart and generate + // a mock that would potentially match the return signature, instead return `null`. This + // is valid per the CheckCast JVM instruction and is better than causing a ClassCastException + // on runtime. + if (rawType.equals(Object.class)) { + return null; + } + return deepStub(invocation, returnTypeGenericMetadata); } diff --git a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java index d91df8c40e..5890f5db0d 100644 --- a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java +++ b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java @@ -14,6 +14,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @SuppressWarnings("unused") public class ReturnsGenericDeepStubsTest { @@ -110,4 +111,26 @@ public void as_expected_fail_with_a_CCE_on_callsite_when_erasure_takes_place_for StringBuilder stringBuilder_assignment_that_should_throw_a_CCE = mock.twoTypeParams(new StringBuilder()).append(2).append(3); } + + class WithGenerics { + T execute() { + throw new IllegalArgumentException(); + } + } + class SubClass extends WithGenerics {} + + class UserOfSubClass { + SubClass generate() { + return null; + } + } + + @Test + public void can_handle_deep_stubs_with_generics_at_end_of_deep_invocation() { + UserOfSubClass mock = mock(UserOfSubClass.class, RETURNS_DEEP_STUBS); + + when(mock.generate().execute()).thenReturn("sub"); + + assertThat(mock.generate().execute()).isEqualTo("sub"); + } } diff --git a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java index 3294614c24..989d3a195c 100644 --- a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java +++ b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java @@ -45,7 +45,7 @@ public void should_serialize_and_deserialize_parameterized_class_mocked_with_dee assertThat(deserialized_deep_stub.iterator().next().add("yes")).isEqualTo(true); } - @Test(expected = ClassCastException.class) + @Test(expected = NullPointerException.class) public void should_discard_generics_metadata_when_serialized_then_disabling_deep_stubs_with_generics() throws Exception { // given ListContainer deep_stubbed = mock(ListContainer.class, withSettings().defaultAnswer(RETURNS_DEEP_STUBS).serializable()); From 34ebca1a8cd88465dd6e8ba395f78a6593522958 Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Tue, 12 Feb 2019 13:20:41 +0000 Subject: [PATCH 05/42] 2.24.3 release (previous 2.24.2) + release notes updated by CI build 3877 [ci skip-release] --- doc/release-notes/official.md | 7 +++++++ version.properties | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index ce1e9c77a0..a041c5e6e8 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,12 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.3 + - 2019-02-12 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.2...v2.24.3) by [Marcin Zajączkowski](https://github.com/szpak) (1), [Tim van der Lippe](https://github.com/TimvdLippe) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.3-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.3) + - [Java 9 support] ClassCastExceptions with JDK9 javac [(#357)](https://github.com/mockito/mockito/issues/357) + - Return null instead of causing a CCE [(#1612)](https://github.com/mockito/mockito/pull/1612) + - Automatic dependency update with Dependabot [(#1600)](https://github.com/mockito/mockito/pull/1600) + - Fix/bug 1551 cce on smart not null answers [(#1576)](https://github.com/mockito/mockito/pull/1576) + #### 2.24.2 - 2019-02-11 - [1 commit](https://github.com/mockito/mockito/compare/v2.24.1...v2.24.2) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.2-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.2) - Fix issue with mocking of java.util.* classes [(#1617)](https://github.com/mockito/mockito/pull/1617) diff --git a/version.properties b/version.properties index b930a88249..9daf4d2f34 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.3 +version=2.24.4 #Previous version used to generate release notes delta -previousVersion=2.24.2 +previousVersion=2.24.3 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From d9cbe3e8ec9cafa521e61371d9e9600b7262cda7 Mon Sep 17 00:00:00 2001 From: Alex Simkin Date: Thu, 14 Feb 2019 04:04:31 +1100 Subject: [PATCH 06/42] Fixes #1618 : Fix strict stubbing profile serialization support. (#1620) --- .../junit/DefaultStubbingLookupListener.java | 5 +- .../StrictStubsSerializableTest.java | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/mockitousage/serialization/StrictStubsSerializableTest.java diff --git a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java index 21212a6754..b6a80c64a3 100644 --- a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java +++ b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java @@ -12,6 +12,7 @@ import org.mockito.quality.Strictness; import org.mockito.stubbing.Stubbing; +import java.io.Serializable; import java.util.Collection; import java.util.LinkedList; import java.util.List; @@ -22,7 +23,9 @@ * Default implementation of stubbing lookup listener. * Fails early if stub called with unexpected arguments, but only if current strictness is set to STRICT_STUBS. */ -class DefaultStubbingLookupListener implements StubbingLookupListener { +class DefaultStubbingLookupListener implements StubbingLookupListener, Serializable { + + private static final long serialVersionUID = -6789800638070123629L; private Strictness currentStrictness; private boolean mismatchesReported; diff --git a/src/test/java/org/mockitousage/serialization/StrictStubsSerializableTest.java b/src/test/java/org/mockitousage/serialization/StrictStubsSerializableTest.java new file mode 100644 index 0000000000..9d786695fc --- /dev/null +++ b/src/test/java/org/mockitousage/serialization/StrictStubsSerializableTest.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2017 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockitousage.serialization; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.io.Serializable; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; +import static org.mockitoutil.SimpleSerializationUtil.serializeAndBack; + +@RunWith(MockitoJUnitRunner.StrictStubs.class) +public class StrictStubsSerializableTest { + + @Mock(serializable = true) private SampleClass sampleClass; + + @Test + public void should_serialize_and_deserialize_mock_created_with_serializable_and_strict_stubs() throws Exception { + // given + when(sampleClass.isFalse()).thenReturn(true); + + // when + SampleClass deserializedSample = serializeAndBack(sampleClass); + // to satisfy strict stubbing + deserializedSample.isFalse(); + verify(deserializedSample).isFalse(); + verify(sampleClass, never()).isFalse(); + + // then + assertThat(deserializedSample.isFalse()).isEqualTo(true); + assertThat(sampleClass.isFalse()).isEqualTo(true); + } + + static class SampleClass implements Serializable { + + boolean isFalse() { + return false; + } + } +} From 4c254058f1f1606a5de4c4fdc9f1a0b3c10ad15e Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Wed, 13 Feb 2019 17:10:34 +0000 Subject: [PATCH 07/42] 2.24.4 release (previous 2.24.3) + release notes updated by CI build 3883 [ci skip-release] --- doc/release-notes/official.md | 5 +++++ version.properties | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index a041c5e6e8..baef9ca6d3 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,10 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.4 + - 2019-02-13 - [1 commit](https://github.com/mockito/mockito/compare/v2.24.3...v2.24.4) by [Alex Simkin](https://github.com/SimY4) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.4-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.4) + - Fixes #1618 : Fix strict stubbing profile serialization support. [(#1620)](https://github.com/mockito/mockito/pull/1620) + - Serializable flag doesn't make mock serializable [(#1618)](https://github.com/mockito/mockito/issues/1618) + #### 2.24.3 - 2019-02-12 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.2...v2.24.3) by [Marcin Zajączkowski](https://github.com/szpak) (1), [Tim van der Lippe](https://github.com/TimvdLippe) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.3-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.3) - [Java 9 support] ClassCastExceptions with JDK9 javac [(#357)](https://github.com/mockito/mockito/issues/357) diff --git a/version.properties b/version.properties index 9daf4d2f34..3eb3f8c0be 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.4 +version=2.24.5 #Previous version used to generate release notes delta -previousVersion=2.24.3 +previousVersion=2.24.4 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From 1ba92767c19c2dd3d3bf3ebf697f32851b2b7af9 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 18 Feb 2019 15:07:09 +0000 Subject: [PATCH 08/42] [ci maven-central-release] Release 2.24.4 --- version.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.properties b/version.properties index 3eb3f8c0be..3e71f116c8 100644 --- a/version.properties +++ b/version.properties @@ -1,5 +1,5 @@ #Currently building Mockito version -version=2.24.5 +version=2.24.4 #Previous version used to generate release notes delta previousVersion=2.24.4 From e2e7dc8bd8c4244ba864dd8687a0dc30cace967f Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 18 Feb 2019 15:14:14 +0000 Subject: [PATCH 09/42] [ci maven-central-release] Release 2.24.5 --- version.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.properties b/version.properties index 3e71f116c8..3eb3f8c0be 100644 --- a/version.properties +++ b/version.properties @@ -1,5 +1,5 @@ #Currently building Mockito version -version=2.24.4 +version=2.24.5 #Previous version used to generate release notes delta previousVersion=2.24.4 From 41c5606349e13a1e3e060ab8348b412662219d62 Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Mon, 18 Feb 2019 15:22:08 +0000 Subject: [PATCH 10/42] 2.24.5 release (previous 2.24.4) + release notes updated by CI build 3887 [ci skip-release] --- doc/release-notes/official.md | 4 ++++ version.properties | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index baef9ca6d3..d15b1d6d24 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,9 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.5 + - 2019-02-18 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.4...v2.24.5) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.5-green.svg)](https://bintray.com/mockito/maven/mockito/2.24.5) + - No pull requests referenced in commit messages. + #### 2.24.4 - 2019-02-13 - [1 commit](https://github.com/mockito/mockito/compare/v2.24.3...v2.24.4) by [Alex Simkin](https://github.com/SimY4) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.4-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.4) - Fixes #1618 : Fix strict stubbing profile serialization support. [(#1620)](https://github.com/mockito/mockito/pull/1620) diff --git a/version.properties b/version.properties index 3eb3f8c0be..ddbd9bc93a 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.5 +version=2.24.6 #Previous version used to generate release notes delta -previousVersion=2.24.4 +previousVersion=2.24.5 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From 866c5d2d49216b3b4767060737fbde53839eabf2 Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Wed, 20 Feb 2019 02:01:12 +0100 Subject: [PATCH 11/42] Make use of shipkit v2.1.0 After this update the site: https://github.com/mockito/mockito/releases will be updated by Shipkit --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 0dac0c72f3..1991f9d166 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ buildscript { classpath 'net.ltgt.gradle:gradle-errorprone-plugin:0.6' //Using buildscript.classpath so that we can resolve shipkit from maven local, during local testing - classpath 'org.shipkit:shipkit:2.0.28' + classpath 'org.shipkit:shipkit:2.1.0' } } @@ -110,7 +110,7 @@ subprojects { name = rootProject.name + '-' + project.name } } - + afterEvaluate { def lib = publishing.publications.javaLibrary if(lib && !lib.artifactId.startsWith("mockito-")) { From bbb2b6961d97e01582dab1bf84d2b2c2075e73e9 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Wed, 27 Feb 2019 05:39:03 -0800 Subject: [PATCH 12/42] Bumped shipkit to v2.1.3 This way we're picking an upstream bugfix --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 1991f9d166..c8142db658 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ buildscript { classpath 'net.ltgt.gradle:gradle-errorprone-plugin:0.6' //Using buildscript.classpath so that we can resolve shipkit from maven local, during local testing - classpath 'org.shipkit:shipkit:2.1.0' + classpath 'org.shipkit:shipkit:2.1.3' } } From 3722958ba4db09b6ea45b613317b5f888d39a3f1 Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Tue, 19 Feb 2019 22:26:45 +0100 Subject: [PATCH 13/42] Fixes #1621: Deep stubs didn't extract bounds from terminal TypeVariable This case was not handled correctly by the GenericMetadataSupport, the issue was that in this case it didn't extracted the bounds from the terminal variable which happen to be a type variable _argument_. This likely fixes a whole area of generics issues with deep stubs. --- .../bytebuddy/MockMethodInterceptor.java | 32 +++++++++---------- .../reflection/GenericMetadataSupport.java | 24 +++++++++++--- .../GenericMetadataSupportTest.java | 32 +++++++++++++++++-- .../DeepStubsSerializableTest.java | 20 ++++++++---- .../stubbing/DeepStubbingTest.java | 16 +++++----- 5 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java index 906692768d..e57a82e739 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java @@ -46,13 +46,11 @@ Object doIntercept(Object mock, Method invokedMethod, Object[] arguments, RealMethod realMethod) throws Throwable { - return doIntercept( - mock, - invokedMethod, - arguments, - realMethod, - new LocationImpl() - ); + return doIntercept(mock, + invokedMethod, + arguments, + realMethod, + new LocationImpl()); } Object doIntercept(Object mock, @@ -108,11 +106,11 @@ public static Object interceptSuperCallable(@This Object mock, return superCall.call(); } return interceptor.doIntercept( - mock, - invokedMethod, - arguments, - new RealMethod.FromCallable(superCall) - ); + mock, + invokedMethod, + arguments, + new RealMethod.FromCallable(superCall) + ); } @SuppressWarnings("unused") @@ -126,11 +124,11 @@ public static Object interceptAbstract(@This Object mock, return stubValue; } return interceptor.doIntercept( - mock, - invokedMethod, - arguments, - RealMethod.IsIllegal.INSTANCE - ); + mock, + invokedMethod, + arguments, + RealMethod.IsIllegal.INSTANCE + ); } } } diff --git a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java index 596827c64c..d7838fb612 100644 --- a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java +++ b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java @@ -97,10 +97,20 @@ protected Class extractRawTypeOf(Type type) { } if (type instanceof TypeVariable) { /* - * If type is a TypeVariable, then it is needed to gather data elsewhere. Usually TypeVariables are declared - * on the class definition, such as such as List. + * If type is a TypeVariable, then it is needed to gather data elsewhere. + * Usually TypeVariables are declared on the class definition, such as + * such as List. + * + * If the data cannot be found in the contextual map, try harder on the + * TypeVariable itself looking for defined bounds. */ - return extractRawTypeOf(contextualActualTypeParameters.get(type)); + Type typeArgument = contextualActualTypeParameters.get(type); + if (typeArgument == null) { + BoundedType boundedType = boundsOf((TypeVariable) type); + contextualActualTypeParameters.put((TypeVariable) type, boundedType); + return extractRawTypeOf(boundedType); + } + return extractRawTypeOf(typeArgument); } throw new MockitoException("Raw extraction not supported for : '" + type + "'"); } @@ -164,8 +174,12 @@ private BoundedType boundsOf(TypeVariable typeParameter) { private BoundedType boundsOf(WildcardType wildCard) { /* * According to JLS(http://docs.oracle.com/javase/specs/jls/se5.0/html/typesValues.html#4.5.1): - * - Lower and upper can't coexist: (for instance, this is not allowed: & super MyInterface>) - * - Multiple bounds are not supported (for instance, this is not allowed: & MyInterface>) + * - Lower and upper can't coexist: (for instance, this is not allowed: + * & super MyInterface>) + * - Multiple concrete type bounds are not supported (for instance, this is not allowed: + * & MyInterface>) + * But the following form is possible where there is a single concrete tyep bound followed by interface type bounds + * & Comparable> */ WildCardBoundedType wildCardBoundedType = new WildCardBoundedType(wildCard); diff --git a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java index 323efbf843..56fff2069c 100644 --- a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java +++ b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java @@ -10,7 +10,12 @@ import java.lang.reflect.Method; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; @@ -48,6 +53,17 @@ interface GenericsNest & Cloneable> extends Map { } + public interface TopInterface { + T generic(); + } + public interface MiddleInterface extends TopInterface { } + public class OwningClassWithDeclaredUpperBounds & Comparable> { + public abstract class AbstractInner implements MiddleInterface { } + } + public class OwningClassWithNoDeclaredUpperBounds { + public abstract class AbstractInner implements MiddleInterface { } + } + @Test public void typeVariable_of_self_type() { GenericMetadataSupport genericMetadata = inferFrom(GenericsSelfReference.class).resolveGenericReturnType(firstNamedMethod("self", GenericsSelfReference.class)); @@ -56,7 +72,7 @@ public void typeVariable_of_self_type() { } @Test - public void can_get_raw_type_from_Class() throws Exception { + public void can_get_raw_type_from_Class() { assertThat(inferFrom(ListOfAnyNumbers.class).rawType()).isEqualTo(ListOfAnyNumbers.class); assertThat(inferFrom(ListOfNumbers.class).rawType()).isEqualTo(ListOfNumbers.class); assertThat(inferFrom(GenericsNest.class).rawType()).isEqualTo(GenericsNest.class); @@ -199,6 +215,18 @@ public void paramType_with_wildcard_return_type_of____returning_wildcard_with_ty assertThat(boundedType.interfaceBounds()).contains(Cloneable.class); } + @Test + public void can_extract_raw_type_from_bounds_on_terminal_typeVariable() { + assertThat(inferFrom(OwningClassWithDeclaredUpperBounds.AbstractInner.class) + .resolveGenericReturnType(firstNamedMethod("generic", OwningClassWithDeclaredUpperBounds.AbstractInner.class)) + .rawType() + ).isEqualTo(List.class); + assertThat(inferFrom(OwningClassWithNoDeclaredUpperBounds.AbstractInner.class) + .resolveGenericReturnType(firstNamedMethod("generic", OwningClassWithNoDeclaredUpperBounds.AbstractInner.class)) + .rawType() + ).isEqualTo(Object.class); + } + private Type typeVariableValue(Map, Type> typeVariables, String typeVariableName) { for (Map.Entry, Type> typeVariableTypeEntry : typeVariables.entrySet()) { if (typeVariableTypeEntry.getKey().getName().equals(typeVariableName)) { diff --git a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java index 989d3a195c..e3891c6047 100644 --- a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java +++ b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java @@ -11,7 +11,11 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; +import static org.assertj.core.api.Assertions.fail; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; import static org.mockitoutil.SimpleSerializationUtil.serializeAndBack; public class DeepStubsSerializableTest { @@ -45,7 +49,7 @@ public void should_serialize_and_deserialize_parameterized_class_mocked_with_dee assertThat(deserialized_deep_stub.iterator().next().add("yes")).isEqualTo(true); } - @Test(expected = NullPointerException.class) + @Test public void should_discard_generics_metadata_when_serialized_then_disabling_deep_stubs_with_generics() throws Exception { // given ListContainer deep_stubbed = mock(ListContainer.class, withSettings().defaultAnswer(RETURNS_DEEP_STUBS).serializable()); @@ -53,10 +57,14 @@ public void should_discard_generics_metadata_when_serialized_then_disabling_deep ListContainer deserialized_deep_stub = serializeAndBack(deep_stubbed); - // when stubbing on a deserialized mock - when(deserialized_deep_stub.iterator().next().get(42)).thenReturn("no"); - - // then revert to the default RETURNS_DEEP_STUBS and the code will raise a ClassCastException + try { + // when stubbing on a deserialized mock + // then revert to the default RETURNS_DEEP_STUBS and the code will raise a ClassCastException + when(deserialized_deep_stub.iterator().next().get(42)).thenReturn("no"); + fail("Expected an exception to be thrown as deep stubs and serialization does not play well together"); + } catch (ClassCastException e) { + assertThat(e).hasMessageContaining("java.util.List"); + } } static class SampleClass implements Serializable { diff --git a/src/test/java/org/mockitousage/stubbing/DeepStubbingTest.java b/src/test/java/org/mockitousage/stubbing/DeepStubbingTest.java index df41f062fb..0e574f0c5e 100644 --- a/src/test/java/org/mockitousage/stubbing/DeepStubbingTest.java +++ b/src/test/java/org/mockitousage/stubbing/DeepStubbingTest.java @@ -224,7 +224,7 @@ public void withPatternPrimitive() throws Exception { Person person = mock(Person.class, RETURNS_DEEP_STUBS); @Test - public void shouldStubbingBasicallyWorkFine() throws Exception { + public void shouldStubbingBasicallyWorkFine() { //given given(person.getAddress().getStreet().getName()).willReturn("Norymberska"); @@ -236,7 +236,7 @@ public void shouldStubbingBasicallyWorkFine() throws Exception { } @Test - public void shouldVerificationBasicallyWorkFine() throws Exception { + public void shouldVerificationBasicallyWorkFine() { //given person.getAddress().getStreet().getName(); @@ -245,7 +245,7 @@ public void shouldVerificationBasicallyWorkFine() throws Exception { } @Test - public void verification_work_with_argument_Matchers_in_nested_calls() throws Exception { + public void verification_work_with_argument_Matchers_in_nested_calls() { //given person.getAddress("111 Mock Lane").getStreet(); person.getAddress("111 Mock Lane").getStreet(Locale.ITALIAN).getName(); @@ -257,7 +257,7 @@ public void verification_work_with_argument_Matchers_in_nested_calls() throws Ex } @Test - public void deep_stub_return_same_mock_instance_if_invocation_matchers_matches() throws Exception { + public void deep_stub_return_same_mock_instance_if_invocation_matchers_matches() { when(person.getAddress(anyString()).getStreet().getName()).thenReturn("deep"); person.getAddress("the docks").getStreet().getName(); @@ -270,7 +270,7 @@ public void deep_stub_return_same_mock_instance_if_invocation_matchers_matches() } @Test - public void times_never_atLeast_atMost_verificationModes_should_work() throws Exception { + public void times_never_atLeast_atMost_verificationModes_should_work() { when(person.getAddress(anyString()).getStreet().getName()).thenReturn("deep"); person.getAddress("the docks").getStreet().getName(); @@ -285,7 +285,7 @@ public void times_never_atLeast_atMost_verificationModes_should_work() throws Ex @Test - public void inOrder_only_work_on_the_very_last_mock_but_it_works() throws Exception { + public void inOrder_only_work_on_the_very_last_mock_but_it_works() { when(person.getAddress(anyString()).getStreet().getName()).thenReturn("deep"); when(person.getAddress(anyString()).getStreet(Locale.ITALIAN).getName()).thenReturn("deep"); when(person.getAddress(anyString()).getStreet(Locale.CHINESE).getName()).thenReturn("deep"); @@ -307,7 +307,7 @@ public void inOrder_only_work_on_the_very_last_mock_but_it_works() throws Except } @Test - public void verificationMode_only_work_on_the_last_returned_mock() throws Exception { + public void verificationMode_only_work_on_the_last_returned_mock() { // 1st invocation on Address mock (stubbing) when(person.getAddress("the docks").getStreet().getName()).thenReturn("deep"); @@ -328,7 +328,7 @@ public void verificationMode_only_work_on_the_last_returned_mock() throws Except } @Test - public void shouldFailGracefullyWhenClassIsFinal() throws Exception { + public void shouldFailGracefullyWhenClassIsFinal() { //when FinalClass value = new FinalClass(); given(person.getFinalClass()).willReturn(value); From 0cf45e2157e86a06dfd280a7d6e7fce63b4d764b Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Tue, 19 Feb 2019 22:38:30 +0100 Subject: [PATCH 14/42] Fixes #1621: Partly reverting #1612 cbedbe567d5bf902b13e8cebc4a2a2ccfef7758f `ReturnsDeepStubs` was returning null instead of MockitoMock to avoid a `ClassCastException` on the call site, this change seems not necessary after the change done in df9b85eebb06106a6d2a629cf55eb9de5c3bf923. But keep the MockitoCast check in errorprone disabled as this PR improves what was done in #1612 by making the generics support cover additional generics scenarios. http://errorprone.info/bugpattern/MockitoCast --- .../internal/stubbing/defaultanswers/ReturnsDeepStubs.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java index 8729171255..cc963679b2 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java @@ -58,9 +58,9 @@ public Object answer(InvocationOnMock invocation) throws Throwable { // a mock that would potentially match the return signature, instead return `null`. This // is valid per the CheckCast JVM instruction and is better than causing a ClassCastException // on runtime. - if (rawType.equals(Object.class)) { - return null; - } +// if (rawType.equals(Object.class)) { +// return null; +// } return deepStub(invocation, returnTypeGenericMetadata); } From c8bc6bc04e087b54a5fe3a8f8805ba5df26fd4e9 Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Tue, 26 Feb 2019 18:27:37 +0100 Subject: [PATCH 15/42] Cleanup, removes non needed `throws exception` --- .../ReturnsGenericDeepStubsTest.java | 16 +++++------ .../GenericMetadataSupportTest.java | 28 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java index 5890f5db0d..40b9ecb575 100644 --- a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java +++ b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java @@ -34,7 +34,7 @@ interface GenericsNest & Cloneable> extends Map mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Set>> entries = mock.entrySet(); @@ -50,7 +50,7 @@ public void generic_deep_mock_frenzy__look_at_these_chained_calls() throws Excep } @Test - public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_of_parameterized_method_is_a_parameterizedtype_that_is_referencing_a_typevar_on_class() throws Exception { + public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_of_parameterized_method_is_a_parameterizedType_that_is_referencing_a_typeVar_on_class() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Cloneable cloneable_bound_that_is_declared_on_typevar_K_in_the_class_which_is_referenced_by_typevar_O_declared_on_the_method = @@ -60,7 +60,7 @@ public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_ } @Test - public void can_create_mock_from_multiple_type_variable_bounds_when_method_return_type_is_referencing_a_typevar_on_class() throws Exception { + public void can_create_mock_from_multiple_type_variable_bounds_when_method_return_type_is_referencing_a_typeVar_on_class() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Cloneable cloneable_bound_of_typevar_K = mock.returningK(); @@ -68,7 +68,7 @@ public void can_create_mock_from_multiple_type_variable_bounds_when_method_retur } @Test - public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_of_parameterized_method_is_a_typevar_that_is_referencing_a_typevar_on_class() throws Exception { + public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_of_parameterized_method_is_a_typeVar_that_is_referencing_a_typeVar_on_class() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Cloneable cloneable_bound_of_typevar_K_referenced_by_typevar_O = (Cloneable) mock.typeVarWithTypeParams(); @@ -76,7 +76,7 @@ public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_ } @Test - public void can_create_mock_from_return_types_declared_with_a_bounded_wildcard() throws Exception { + public void can_create_mock_from_return_types_declared_with_a_bounded_wildcard() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); List objects = mock.returningWildcard(); @@ -85,7 +85,7 @@ public void can_create_mock_from_return_types_declared_with_a_bounded_wildcard() } @Test - public void can_still_work_with_raw_type_in_the_return_type() throws Exception { + public void can_still_work_with_raw_type_in_the_return_type() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Number the_raw_type_that_should_be_returned = mock.returnsNormalType(); @@ -93,7 +93,7 @@ public void can_still_work_with_raw_type_in_the_return_type() throws Exception { } @Test - public void will_return_default_value_on_non_mockable_nested_generic() throws Exception { + public void will_return_default_value_on_non_mockable_nested_generic() { GenericsNest genericsNest = mock(GenericsNest.class, RETURNS_DEEP_STUBS); ListOfInteger listOfInteger = mock(ListOfInteger.class, RETURNS_DEEP_STUBS); AnotherListOfInteger anotherListOfInteger = mock(AnotherListOfInteger.class, RETURNS_DEEP_STUBS); @@ -104,7 +104,7 @@ public void will_return_default_value_on_non_mockable_nested_generic() throws Ex } @Test(expected = ClassCastException.class) - public void as_expected_fail_with_a_CCE_on_callsite_when_erasure_takes_place_for_example___StringBuilder_is_subject_to_erasure() throws Exception { + public void as_expected_fail_with_a_CCE_on_call_site_when_erasure_takes_place_for_example___StringBuilder_is_subject_to_erasure() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); // following assignment needed to create a ClassCastException on the call site (i.e. : here) diff --git a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java index 56fff2069c..57d0cb2925 100644 --- a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java +++ b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java @@ -80,7 +80,7 @@ public void can_get_raw_type_from_Class() { } @Test - public void can_get_raw_type_from_ParameterizedType() throws Exception { + public void can_get_raw_type_from_ParameterizedType() { assertThat(inferFrom(ListOfAnyNumbers.class.getGenericInterfaces()[0]).rawType()).isEqualTo(List.class); assertThat(inferFrom(ListOfNumbers.class.getGenericInterfaces()[0]).rawType()).isEqualTo(List.class); assertThat(inferFrom(GenericsNest.class.getGenericInterfaces()[0]).rawType()).isEqualTo(Map.class); @@ -88,7 +88,7 @@ public void can_get_raw_type_from_ParameterizedType() throws Exception { } @Test - public void can_get_type_variables_from_Class() throws Exception { + public void can_get_type_variables_from_Class() { assertThat(inferFrom(GenericsNest.class).actualTypeArguments().keySet()).hasSize(1).extracting("name").contains("K"); assertThat(inferFrom(ListOfNumbers.class).actualTypeArguments().keySet()).isEmpty(); assertThat(inferFrom(ListOfAnyNumbers.class).actualTypeArguments().keySet()).hasSize(1).extracting("name").contains("N"); @@ -105,7 +105,7 @@ public void can_resolve_type_variables_from_ancestors() throws Exception { } @Test - public void can_get_type_variables_from_ParameterizedType() throws Exception { + public void can_get_type_variables_from_ParameterizedType() { assertThat(inferFrom(GenericsNest.class.getGenericInterfaces()[0]).actualTypeArguments().keySet()).hasSize(2).extracting("name").contains("K", "V"); assertThat(inferFrom(ListOfAnyNumbers.class.getGenericInterfaces()[0]).actualTypeArguments().keySet()).hasSize(1).extracting("name").contains("E"); assertThat(inferFrom(Integer.class.getGenericInterfaces()[0]).actualTypeArguments().keySet()).hasSize(1).extracting("name").contains("T"); @@ -114,7 +114,7 @@ public void can_get_type_variables_from_ParameterizedType() throws Exception { } @Test - public void typeVariable_return_type_of____iterator____resolved_to_Iterator_and_type_argument_to_String() throws Exception { + public void typeVariable_return_type_of____iterator____resolved_to_Iterator_and_type_argument_to_String() { GenericMetadataSupport genericMetadata = inferFrom(StringList.class).resolveGenericReturnType(firstNamedMethod("iterator", StringList.class)); assertThat(genericMetadata.rawType()).isEqualTo(Iterator.class); @@ -122,7 +122,7 @@ public void typeVariable_return_type_of____iterator____resolved_to_Iterator_and_ } @Test - public void typeVariable_return_type_of____get____resolved_to_Set_and_type_argument_to_Number() throws Exception { + public void typeVariable_return_type_of____get____resolved_to_Set_and_type_argument_to_Number() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("get", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(Set.class); @@ -130,7 +130,7 @@ public void typeVariable_return_type_of____get____resolved_to_Set_and_type_argum } @Test - public void bounded_typeVariable_return_type_of____returningK____resolved_to_Comparable_and_with_BoundedType() throws Exception { + public void bounded_typeVariable_return_type_of____returningK____resolved_to_Comparable_and_with_BoundedType() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("returningK", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(Comparable.class); @@ -139,7 +139,7 @@ public void bounded_typeVariable_return_type_of____returningK____resolved_to_Com } @Test - public void fixed_ParamType_return_type_of____remove____resolved_to_Set_and_type_argument_to_Number() throws Exception { + public void fixed_ParamType_return_type_of____remove____resolved_to_Set_and_type_argument_to_Number() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("remove", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(Set.class); @@ -147,7 +147,7 @@ public void fixed_ParamType_return_type_of____remove____resolved_to_Set_and_type } @Test - public void paramType_return_type_of____values____resolved_to_Collection_and_type_argument_to_Parameterized_Set() throws Exception { + public void paramType_return_type_of____values____resolved_to_Collection_and_type_argument_to_Parameterized_Set() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("values", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(Collection.class); @@ -157,7 +157,7 @@ public void paramType_return_type_of____values____resolved_to_Collection_and_typ } @Test - public void paramType_with_type_parameters_return_type_of____paramType_with_type_params____resolved_to_Collection_and_type_argument_to_Parameterized_Set() throws Exception { + public void paramType_with_type_parameters_return_type_of____paramType_with_type_params____resolved_to_Collection_and_type_argument_to_Parameterized_Set() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("paramType_with_type_params", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(List.class); @@ -166,7 +166,7 @@ public void paramType_with_type_parameters_return_type_of____paramType_with_type } @Test - public void typeVariable_with_type_parameters_return_type_of____typeVar_with_type_params____resolved_K_hence_to_Comparable_and_with_BoundedType() throws Exception { + public void typeVariable_with_type_parameters_return_type_of____typeVar_with_type_params____resolved_K_hence_to_Comparable_and_with_BoundedType() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("typeVar_with_type_params", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(Comparable.class); @@ -175,7 +175,7 @@ public void typeVariable_with_type_parameters_return_type_of____typeVar_with_typ } @Test - public void class_return_type_of____append____resolved_to_StringBuilder_and_type_arguments() throws Exception { + public void class_return_type_of____append____resolved_to_StringBuilder_and_type_arguments() { GenericMetadataSupport genericMetadata = inferFrom(StringBuilder.class).resolveGenericReturnType(firstNamedMethod("append", StringBuilder.class)); assertThat(genericMetadata.rawType()).isEqualTo(StringBuilder.class); @@ -185,7 +185,7 @@ public void class_return_type_of____append____resolved_to_StringBuilder_and_type @Test - public void paramType_with_wildcard_return_type_of____returning_wildcard_with_class_lower_bound____resolved_to_List_and_type_argument_to_Integer() throws Exception { + public void paramType_with_wildcard_return_type_of____returning_wildcard_with_class_lower_bound____resolved_to_List_and_type_argument_to_Integer() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("returning_wildcard_with_class_lower_bound", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(List.class); @@ -195,7 +195,7 @@ public void paramType_with_wildcard_return_type_of____returning_wildcard_with_cl } @Test - public void paramType_with_wildcard_return_type_of____returning_wildcard_with_typeVar_lower_bound____resolved_to_List_and_type_argument_to_Integer() throws Exception { + public void paramType_with_wildcard_return_type_of____returning_wildcard_with_typeVar_lower_bound____resolved_to_List_and_type_argument_to_Integer() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("returning_wildcard_with_typeVar_lower_bound", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(List.class); @@ -205,7 +205,7 @@ public void paramType_with_wildcard_return_type_of____returning_wildcard_with_ty assertThat(boundedType.interfaceBounds()).contains(Cloneable.class); } @Test - public void paramType_with_wildcard_return_type_of____returning_wildcard_with_typeVar_upper_bound____resolved_to_List_and_type_argument_to_Integer() throws Exception { + public void paramType_with_wildcard_return_type_of____returning_wildcard_with_typeVar_upper_bound____resolved_to_List_and_type_argument_to_Integer() { GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("returning_wildcard_with_typeVar_upper_bound", GenericsNest.class)); assertThat(genericMetadata.rawType()).isEqualTo(List.class); From aab36909e20fc05660fbf49f706011698f135b1a Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Wed, 27 Feb 2019 16:51:25 +0100 Subject: [PATCH 16/42] Fixes #1621 Now retrieves actual bounds for interfaces too It improves the fix in 0cf45e2157e86a06dfd280a7d6e7fce63b4d764b by looking in early inference phase the bounds of the TypeVariable that serves as a type argument of a ParameterizedType. --- .../reflection/GenericMetadataSupport.java | 96 +++++++++------- .../GenericMetadataSupportTest.java | 107 ++++++++++++++++-- 2 files changed, 151 insertions(+), 52 deletions(-) diff --git a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java index d7838fb612..3c519c1a51 100644 --- a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java +++ b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java @@ -8,8 +8,23 @@ import org.mockito.exceptions.base.MockitoException; import org.mockito.internal.util.Checks; -import java.lang.reflect.*; -import java.util.*; +import java.lang.reflect.GenericArrayType; +import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; +import java.lang.reflect.WildcardType; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Queue; +import java.util.Set; /** @@ -17,20 +32,20 @@ * and accessible members. * *

- * The main idea of this code is to create a Map that will help to resolve return types. - * In order to actually work with nested generics, this map will have to be passed along new instances - * as a type context. + * The main idea of this code is to create a Map that will help to resolve return types. + * In order to actually work with nested generics, this map will have to be passed along new instances + * as a type context. *

* *

- * Hence : - *

    - *
  • A new instance representing the metadata is created using the {@link #inferFrom(Type)} method from a real - * Class or from a ParameterizedType, other types are not yet supported.
  • + * Hence : + *
      + *
    • A new instance representing the metadata is created using the {@link #inferFrom(Type)} method from a real + * Class or from a ParameterizedType, other types are not yet supported.
    • * - *
    • Then from this metadata, we can extract meta-data for a generic return type of a method, using - * {@link #resolveGenericReturnType(Method)}.
    • - *
    + *
  • Then from this metadata, we can extract meta-data for a generic return type of a method, using + * {@link #resolveGenericReturnType(Method)}.
  • + *
*

* *

@@ -98,19 +113,10 @@ protected Class extractRawTypeOf(Type type) { if (type instanceof TypeVariable) { /* * If type is a TypeVariable, then it is needed to gather data elsewhere. - * Usually TypeVariables are declared on the class definition, such as - * such as List. - * - * If the data cannot be found in the contextual map, try harder on the - * TypeVariable itself looking for defined bounds. + * Usually TypeVariables are declared on the class definition, such as such + * as List. */ - Type typeArgument = contextualActualTypeParameters.get(type); - if (typeArgument == null) { - BoundedType boundedType = boundsOf((TypeVariable) type); - contextualActualTypeParameters.put((TypeVariable) type, boundedType); - return extractRawTypeOf(boundedType); - } - return extractRawTypeOf(typeArgument); + return extractRawTypeOf(contextualActualTypeParameters.get(type)); } throw new MockitoException("Raw extraction not supported for : '" + type + "'"); } @@ -126,10 +132,21 @@ protected void registerTypeVariablesOn(Type classType) { TypeVariable typeParameter = typeParameters[i]; Type actualTypeArgument = actualTypeArguments[i]; - // Prevent registration of a cycle of TypeVariables. This can happen when we are processing - // type parameters in a Method, while we already processed the type parameters of a class. - if (actualTypeArgument instanceof TypeVariable && contextualActualTypeParameters.containsKey(typeParameter)) { - continue; + if (actualTypeArgument instanceof TypeVariable) { + /* + * If actualTypeArgument is a TypeVariable, and it is not present in + * the context map then it is needed to try harder to gather more data + * from the type argument itself. In some case the type argument do + * define upper bounds, this allow to look for them if not in the + * context map. + */ + registerTypeVariableIfNotPresent((TypeVariable) actualTypeArgument); + + // Prevent registration of a cycle of TypeVariables. This can happen when we are processing + // type parameters in a Method, while we already processed the type parameters of a class. + if (contextualActualTypeParameters.containsKey(typeParameter)) { + continue; + } } if (actualTypeArgument instanceof WildcardType) { @@ -157,7 +174,7 @@ private void registerTypeVariableIfNotPresent(TypeVariable typeVariable) { /** * @param typeParameter The TypeVariable parameter * @return A {@link BoundedType} for easy bound information, if first bound is a TypeVariable - * then retrieve BoundedType of this TypeVariable + * then retrieve BoundedType of this TypeVariable */ private BoundedType boundsOf(TypeVariable typeParameter) { if (typeParameter.getBounds()[0] instanceof TypeVariable) { @@ -169,7 +186,7 @@ private BoundedType boundsOf(TypeVariable typeParameter) { /** * @param wildCard The WildCard type * @return A {@link BoundedType} for easy bound information, if first bound is a TypeVariable - * then retrieve BoundedType of this TypeVariable + * then retrieve BoundedType of this TypeVariable */ private BoundedType boundsOf(WildcardType wildCard) { /* @@ -255,7 +272,7 @@ public GenericMetadataSupport resolveGenericReturnType(Method method) { // logger.log("Method '" + method.toGenericString() + "' has return type : " + genericReturnType.getClass().getInterfaces()[0].getSimpleName() + " : " + genericReturnType); int arity = 0; - while(genericReturnType instanceof GenericArrayType) { + while (genericReturnType instanceof GenericArrayType) { arity++; genericReturnType = ((GenericArrayType) genericReturnType).getGenericComponentType(); } @@ -287,8 +304,8 @@ private GenericMetadataSupport resolveGenericType(Type type, Method method) { * Create an new instance of {@link GenericMetadataSupport} inferred from a {@link Type}. * *

- * At the moment type can only be a {@link Class} or a {@link ParameterizedType}, otherwise - * it'll throw a {@link MockitoException}. + * At the moment type can only be a {@link Class} or a {@link ParameterizedType}, otherwise + * it'll throw a {@link MockitoException}. *

* * @param type The class from which the {@link GenericMetadataSupport} should be built. @@ -314,7 +331,7 @@ public static GenericMetadataSupport inferFrom(Type type) { /** * Generic metadata implementation for {@link Class}. - * + *

* Offer support to retrieve generic metadata on a {@link Class} by reading type parameters and type variables on * the class and its ancestors and interfaces. */ @@ -336,10 +353,10 @@ public Class rawType() { /** * Generic metadata implementation for "standalone" {@link ParameterizedType}. - * + *

* Offer support to retrieve generic metadata on a {@link ParameterizedType} by reading type variables of * the related raw type and declared type variable of this parameterized type. - * + *

* This class is not designed to work on ParameterizedType returned by {@link Method#getGenericReturnType()}, as * the ParameterizedType instance return in these cases could have Type Variables that refer to type declaration(s). * That's what meant the "standalone" word at the beginning of the Javadoc. @@ -419,7 +436,7 @@ private void readTypeVariables() { for (Type type : typeVariable.getBounds()) { registerTypeVariablesOn(type); } - registerTypeParametersOn(new TypeVariable[] { typeVariable }); + registerTypeParametersOn(new TypeVariable[]{typeVariable}); registerTypeVariablesOn(getActualTypeArgumentFor(typeVariable)); } @@ -456,7 +473,7 @@ public Class[] rawExtraInterfaces() { for (Type extraInterface : extraInterfaces) { Class rawInterface = extractRawTypeOf(extraInterface); // avoid interface collision with actual raw type (with typevariables, resolution ca be quite aggressive) - if(!rawType().equals(rawInterface)) { + if (!rawType().equals(rawInterface)) { rawExtraInterfaces.add(rawInterface); } } @@ -528,7 +545,6 @@ public Class rawType() { } - /** * Type representing bounds of a type * @@ -551,7 +567,7 @@ public interface BoundedType extends Type { * *

If upper bounds are declared with SomeClass and additional interfaces, then firstBound will be SomeClass and * interfacesBound will be an array of the additional interfaces. - * + *

* i.e. SomeClass. *


      *     interface UpperBoundedTypeWithClass & Cloneable> {
diff --git a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java
index 57d0cb2925..cc5b5885d1 100644
--- a/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java
+++ b/src/test/java/org/mockito/internal/util/reflection/GenericMetadataSupportTest.java
@@ -8,9 +8,11 @@
 
 import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -26,42 +28,66 @@ public class GenericMetadataSupportTest {
     interface GenericsSelfReference> {
         T self();
     }
+
     interface UpperBoundedTypeWithClass> {
         E get();
     }
+
     interface UpperBoundedTypeWithInterfaces & Cloneable> {
         E get();
     }
-    interface ListOfNumbers extends List {}
-    interface AnotherListOfNumbers extends ListOfNumbers {}
 
-    abstract class ListOfNumbersImpl implements ListOfNumbers {}
-    abstract class AnotherListOfNumbersImpl extends ListOfNumbersImpl {}
+    interface ListOfNumbers extends List {
+    }
 
-    interface ListOfAnyNumbers extends List {}
+    interface AnotherListOfNumbers extends ListOfNumbers {
+    }
+
+    abstract class ListOfNumbersImpl implements ListOfNumbers {
+    }
+
+    abstract class AnotherListOfNumbersImpl extends ListOfNumbersImpl {
+    }
+
+    interface ListOfAnyNumbers extends List {
+    }
 
     interface GenericsNest & Cloneable> extends Map> {
         Set remove(Object key); // override with fixed ParameterizedType
+
         List returning_wildcard_with_class_lower_bound();
+
         List returning_wildcard_with_typeVar_lower_bound();
+
         List returning_wildcard_with_typeVar_upper_bound();
+
         K returningK();
+
          List paramType_with_type_params();
+
          T two_type_params();
+
          O typeVar_with_type_params();
     }
 
-    static class StringList extends ArrayList { }
+    static class StringList extends ArrayList {
+    }
 
     public interface TopInterface {
         T generic();
     }
-    public interface MiddleInterface extends TopInterface { }
-    public class OwningClassWithDeclaredUpperBounds & Comparable> {
-        public abstract class AbstractInner implements MiddleInterface { }
+
+    public interface MiddleInterface extends TopInterface {
+    }
+
+    public class OwningClassWithDeclaredUpperBounds & Comparable & Cloneable> {
+        public abstract class AbstractInner implements MiddleInterface {
+        }
     }
+
     public class OwningClassWithNoDeclaredUpperBounds {
-        public abstract class AbstractInner implements MiddleInterface { }
+        public abstract class AbstractInner implements MiddleInterface {
+        }
     }
 
     @Test
@@ -183,7 +209,6 @@ public void class_return_type_of____append____resolved_to_StringBuilder_and_type
     }
 
 
-
     @Test
     public void paramType_with_wildcard_return_type_of____returning_wildcard_with_class_lower_bound____resolved_to_List_and_type_argument_to_Integer() {
         GenericMetadataSupport genericMetadata = inferFrom(GenericsNest.class).resolveGenericReturnType(firstNamedMethod("returning_wildcard_with_class_lower_bound", GenericsNest.class));
@@ -202,7 +227,8 @@ public void paramType_with_wildcard_return_type_of____returning_wildcard_with_ty
         GenericMetadataSupport.BoundedType boundedType = (GenericMetadataSupport.BoundedType) typeVariableValue(genericMetadata.actualTypeArguments(), "E");
 
         assertThat(inferFrom(boundedType.firstBound()).rawType()).isEqualTo(Comparable.class);
-        assertThat(boundedType.interfaceBounds()).contains(Cloneable.class);    }
+        assertThat(boundedType.interfaceBounds()).contains(Cloneable.class);
+    }
 
     @Test
     public void paramType_with_wildcard_return_type_of____returning_wildcard_with_typeVar_upper_bound____resolved_to_List_and_type_argument_to_Integer() {
@@ -227,6 +253,63 @@ public void can_extract_raw_type_from_bounds_on_terminal_typeVariable() {
                   ).isEqualTo(Object.class);
     }
 
+    @Test
+    public void can_extract_interface_type_from_bounds_on_terminal_typeVariable() {
+
+        assertThat(inferFrom(OwningClassWithDeclaredUpperBounds.AbstractInner.class)
+                       .resolveGenericReturnType(firstNamedMethod("generic", OwningClassWithDeclaredUpperBounds.AbstractInner.class))
+                       .rawExtraInterfaces()
+                  ).containsExactly(Comparable.class, Cloneable.class);
+        assertThat(inferFrom(OwningClassWithDeclaredUpperBounds.AbstractInner.class)
+                       .resolveGenericReturnType(firstNamedMethod("generic", OwningClassWithDeclaredUpperBounds.AbstractInner.class))
+                       .extraInterfaces()
+                  ).containsExactly(parameterizedTypeOf(Comparable.class, null, String.class),
+                                                                                                                                                                         Cloneable.class);
+
+        assertThat(inferFrom(OwningClassWithNoDeclaredUpperBounds.AbstractInner.class)
+                       .resolveGenericReturnType(firstNamedMethod("generic", OwningClassWithNoDeclaredUpperBounds.AbstractInner.class))
+                       .extraInterfaces()
+                  ).isEmpty();
+    }
+
+    private ParameterizedType parameterizedTypeOf(final Class rawType, final Class ownerType, final Type... actualTypeArguments) {
+        return new ParameterizedType() {
+            @Override
+            public Type[] getActualTypeArguments() {
+                return actualTypeArguments;
+            }
+
+            @Override
+            public Type getRawType() {
+                return rawType;
+            }
+
+            @Override
+            public Type getOwnerType() {
+                return ownerType;
+            }
+
+            public boolean equals(Object other) {
+                if (other instanceof ParameterizedType) {
+                    ParameterizedType otherParamType = (ParameterizedType) other;
+                    if (this == otherParamType) {
+                        return true;
+                    } else {
+                        return equals(ownerType, otherParamType.getOwnerType())
+                               && equals(rawType, otherParamType.getRawType())
+                               && Arrays.equals(actualTypeArguments, otherParamType.getActualTypeArguments());
+                    }
+                } else {
+                    return false;
+                }
+            }
+
+            private boolean equals(Object a, Object b) {
+                return (a == b) || (a != null && a.equals(b));
+            }
+        };
+    }
+
     private Type typeVariableValue(Map, Type> typeVariables, String typeVariableName) {
         for (Map.Entry, Type> typeVariableTypeEntry : typeVariables.entrySet()) {
             if (typeVariableTypeEntry.getKey().getName().equals(typeVariableName)) {

From 20416a3c3ed6bac2f54a15f20c98f3a75ec7656b Mon Sep 17 00:00:00 2001
From: Szczepan Faber 
Date: Thu, 13 Sep 2018 14:55:24 -0700
Subject: [PATCH 17/42] Exposed new public API - StubbingLookupListener

The API was used internally to implement strict stubbing features. We want to expose the lower level public API that we use internally to make Mockito more flexible.

Fixes #793
---
 src/main/java/org/mockito/MockSettings.java   |  19 +++
 .../internal/creation/MockSettingsImpl.java   |  27 ++--
 .../creation/settings/CreationSettings.java   |   8 +-
 .../mockito/internal/exceptions/Reporter.java |   4 +-
 .../junit/DefaultStubbingLookupListener.java  |   4 +-
 .../junit/StrictStubsRunnerTestListener.java  |   4 +-
 .../listeners/StubbingLookupNotifier.java     |   2 +
 .../listeners/StubbingLookupEvent.java        |   2 +-
 .../listeners/StubbingLookupListener.java     |  18 ++-
 .../mockito/mock/MockCreationSettings.java    |   9 +-
 .../creation/MockSettingsImplTest.java        |  68 ++++++++
 .../listeners/StubbingLookupNotifierTest.java |   1 +
 .../StubbingLookupListenerCallbackTest.java   | 153 ++++++++++++++++++
 13 files changed, 289 insertions(+), 30 deletions(-)
 rename src/main/java/org/mockito/{internal => }/listeners/StubbingLookupEvent.java (95%)
 rename src/main/java/org/mockito/{internal => }/listeners/StubbingLookupListener.java (62%)
 create mode 100644 src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java

diff --git a/src/main/java/org/mockito/MockSettings.java b/src/main/java/org/mockito/MockSettings.java
index c79c243175..bf98c2af6e 100644
--- a/src/main/java/org/mockito/MockSettings.java
+++ b/src/main/java/org/mockito/MockSettings.java
@@ -9,6 +9,7 @@
 import org.mockito.invocation.InvocationFactory;
 import org.mockito.invocation.MockHandler;
 import org.mockito.listeners.InvocationListener;
+import org.mockito.listeners.StubbingLookupListener;
 import org.mockito.listeners.VerificationStartedListener;
 import org.mockito.mock.MockCreationSettings;
 import org.mockito.mock.SerializableMode;
@@ -203,6 +204,24 @@ public interface MockSettings extends Serializable {
      */
     MockSettings verboseLogging();
 
+    /**
+     * Add stubbing lookup listener to the mock object.
+     *
+     * Multiple listeners may be added and they will be notified in the order they were supplied.
+     *
+     * For use cases and more info see {@link StubbingLookupListener}
+     *
+     * Example:
+     * 

+     *  List mockWithListener = mock(List.class, withSettings().stubbingLookupListeners(new YourStubbingLookupListener()));
+     * 
+ * + * @param listeners The stubbing lookup listeners to add. May not be null. + * @return settings instance so that you can fluently specify other settings + * @since TODO x here and everywhere else + */ + MockSettings stubbingLookupListeners(StubbingLookupListener... listeners); + /** * Registers a listener for method invocations on this mock. The listener is * notified every time a method on this mock is called. diff --git a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java index ca67730793..d842e3e8e3 100644 --- a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java +++ b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java @@ -11,6 +11,7 @@ import org.mockito.internal.util.MockCreationValidator; import org.mockito.internal.util.MockNameImpl; import org.mockito.listeners.InvocationListener; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.listeners.VerificationStartedListener; import org.mockito.mock.MockCreationSettings; import org.mockito.mock.MockName; @@ -19,17 +20,17 @@ import java.io.Serializable; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; +import static java.util.Arrays.asList; import static org.mockito.internal.exceptions.Reporter.defaultAnswerDoesNotAcceptNullParameter; import static org.mockito.internal.exceptions.Reporter.extraInterfacesAcceptsOnlyInterfaces; import static org.mockito.internal.exceptions.Reporter.extraInterfacesDoesNotAcceptNullParameters; import static org.mockito.internal.exceptions.Reporter.extraInterfacesRequiresAtLeastOneInterface; -import static org.mockito.internal.exceptions.Reporter.invocationListenersRequiresAtLeastOneListener; import static org.mockito.internal.exceptions.Reporter.methodDoesNotAcceptParameter; +import static org.mockito.internal.exceptions.Reporter.requiresAtLeastOneListener; import static org.mockito.internal.util.collections.Sets.newSet; @SuppressWarnings("unchecked") @@ -154,7 +155,7 @@ public Object[] getConstructorArgs() { } List resultArgs = new ArrayList(constructorArgs.length + 1); resultArgs.add(outerClassInstance); - resultArgs.addAll(Arrays.asList(constructorArgs)); + resultArgs.addAll(asList(constructorArgs)); return resultArgs.toArray(new Object[constructorArgs.length + 1]); } @@ -173,17 +174,24 @@ public MockSettings verboseLogging() { @Override public MockSettings invocationListeners(InvocationListener... listeners) { - if (listeners == null || listeners.length == 0) { - throw invocationListenersRequiresAtLeastOneListener(); - } addListeners(listeners, invocationListeners, "invocationListeners"); return this; } + @Override + public MockSettings stubbingLookupListeners(StubbingLookupListener... listeners) { + addListeners(listeners, stubbingLookupListeners, "stubbingLookupListeners"); + return this; + } + + //TODO X dedicated unit tests private static void addListeners(T[] listeners, List container, String method) { if (listeners == null) { throw methodDoesNotAcceptParameter(method, "null vararg array."); } + if (listeners.length == 0) { + throw requiresAtLeastOneListener(method); + } for (T listener : listeners) { if (listener == null) { throw methodDoesNotAcceptParameter(method, "null listeners."); @@ -207,13 +215,8 @@ private boolean invocationListenersContainsType(Class clazz) { return false; } - @Override - public List getInvocationListeners() { - return this.invocationListeners; - } - public boolean hasInvocationListeners() { - return !invocationListeners.isEmpty(); + return !getInvocationListeners().isEmpty(); } @Override 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 81f52f982b..acae4a2f9f 100644 --- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java +++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java @@ -4,8 +4,8 @@ */ package org.mockito.internal.creation.settings; -import org.mockito.internal.listeners.StubbingLookupListener; import org.mockito.listeners.InvocationListener; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.listeners.VerificationStartedListener; import org.mockito.mock.MockCreationSettings; import org.mockito.mock.MockName; @@ -30,7 +30,10 @@ public class CreationSettings implements MockCreationSettings, Serializabl protected MockName mockName; protected SerializableMode serializableMode = SerializableMode.NONE; protected List invocationListeners = new ArrayList(); - protected final List stubbingLookupListeners = new ArrayList(); + + //TODO X - concurrent scenario? + protected List stubbingLookupListeners = new ArrayList(); + protected List verificationStartedListeners = new LinkedList(); protected boolean stubOnly; protected boolean stripAnnotations; @@ -51,6 +54,7 @@ public CreationSettings(CreationSettings copy) { this.mockName = copy.mockName; this.serializableMode = copy.serializableMode; this.invocationListeners = copy.invocationListeners; + this.stubbingLookupListeners = copy.stubbingLookupListeners; this.verificationStartedListeners = copy.verificationStartedListeners; this.stubOnly = copy.stubOnly; this.useConstructor = copy.isUsingConstructor(); diff --git a/src/main/java/org/mockito/internal/exceptions/Reporter.java b/src/main/java/org/mockito/internal/exceptions/Reporter.java index 3158228f22..1b68fbb657 100644 --- a/src/main/java/org/mockito/internal/exceptions/Reporter.java +++ b/src/main/java/org/mockito/internal/exceptions/Reporter.java @@ -684,8 +684,8 @@ public static MockitoException methodDoesNotAcceptParameter(String method, Strin return new MockitoException(method + "() does not accept " + parameter + " See the Javadoc."); } - public static MockitoException invocationListenersRequiresAtLeastOneListener() { - return new MockitoException("invocationListeners() requires at least one listener"); + public static MockitoException requiresAtLeastOneListener(String method) { + return new MockitoException(method + "() requires at least one listener"); } public static MockitoException invocationListenerThrewException(InvocationListener listener, Throwable listenerThrowable) { diff --git a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java index b6a80c64a3..11f2c43fc7 100644 --- a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java +++ b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java @@ -5,8 +5,8 @@ package org.mockito.internal.junit; import org.mockito.internal.exceptions.Reporter; -import org.mockito.internal.listeners.StubbingLookupEvent; -import org.mockito.internal.listeners.StubbingLookupListener; +import org.mockito.listeners.StubbingLookupEvent; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.internal.stubbing.UnusedStubbingReporting; import org.mockito.invocation.Invocation; import org.mockito.quality.Strictness; diff --git a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java index 9e60c31356..227c8c42e2 100644 --- a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java +++ b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java @@ -4,7 +4,6 @@ */ package org.mockito.internal.junit; -import org.mockito.internal.creation.settings.CreationSettings; import org.mockito.mock.MockCreationSettings; import org.mockito.quality.Strictness; @@ -23,7 +22,6 @@ public void onMockCreated(Object mock, MockCreationSettings settings) { //It is not ideal that we modify the state of MockCreationSettings object //MockCreationSettings is intended to be an immutable view of the creation settings //In future, we should start passing MockSettings object to the creation listener - //TODO #793 - when completed, we should be able to get rid of the CreationSettings casting below - ((CreationSettings) settings).getStubbingLookupListeners().add(stubbingLookupListener); + settings.getStubbingLookupListeners().add(stubbingLookupListener); } } diff --git a/src/main/java/org/mockito/internal/listeners/StubbingLookupNotifier.java b/src/main/java/org/mockito/internal/listeners/StubbingLookupNotifier.java index 3e550013a3..a0de0c37b5 100644 --- a/src/main/java/org/mockito/internal/listeners/StubbingLookupNotifier.java +++ b/src/main/java/org/mockito/internal/listeners/StubbingLookupNotifier.java @@ -6,6 +6,8 @@ import org.mockito.internal.creation.settings.CreationSettings; import org.mockito.invocation.Invocation; +import org.mockito.listeners.StubbingLookupEvent; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.mock.MockCreationSettings; import org.mockito.stubbing.Stubbing; diff --git a/src/main/java/org/mockito/internal/listeners/StubbingLookupEvent.java b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java similarity index 95% rename from src/main/java/org/mockito/internal/listeners/StubbingLookupEvent.java rename to src/main/java/org/mockito/listeners/StubbingLookupEvent.java index 0d45edd345..a5d78b341b 100644 --- a/src/main/java/org/mockito/internal/listeners/StubbingLookupEvent.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java @@ -2,7 +2,7 @@ * Copyright (c) 2018 Mockito contributors * This program is made available under the terms of the MIT License. */ -package org.mockito.internal.listeners; +package org.mockito.listeners; import org.mockito.invocation.Invocation; import org.mockito.mock.MockCreationSettings; diff --git a/src/main/java/org/mockito/internal/listeners/StubbingLookupListener.java b/src/main/java/org/mockito/listeners/StubbingLookupListener.java similarity index 62% rename from src/main/java/org/mockito/internal/listeners/StubbingLookupListener.java rename to src/main/java/org/mockito/listeners/StubbingLookupListener.java index 6fa37a159d..e725aa4af9 100644 --- a/src/main/java/org/mockito/internal/listeners/StubbingLookupListener.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupListener.java @@ -2,10 +2,17 @@ * Copyright (c) 2017 Mockito contributors * This program is made available under the terms of the MIT License. */ -package org.mockito.internal.listeners; +package org.mockito.listeners; + +import org.mockito.MockSettings; /** - * Listens to attempts to look up stubbing answer for given mocks. This class is internal for now. + * This listener can be notified of looking up stubbing answer for a given mock. + * + * For this to happen, it must be registered using {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. + * + * TODO x use case, mutability + * *

* How does it work? * When method is called on the mock object, Mockito looks for any answer (stubbing) declared on that mock. @@ -13,11 +20,6 @@ * If the answer is not found (e.g. that invocation was not stubbed on the mock), mock's default answer is used. * This listener implementation is notified when Mockito looked up an answer for invocation on a mock. *

- * If we make this interface a part of public API (and we should): - * - make the implementation unified with InvocationListener (for example: common parent, marker interface MockObjectListener - * single method for adding listeners so long they inherit from the parent) - * - make the error handling strict - * so that Mockito provides decent message when listener fails due to poor implementation. */ public interface StubbingLookupListener { @@ -25,6 +27,8 @@ public interface StubbingLookupListener { * Called by the framework when Mockito looked up an answer for invocation on a mock. * * @param stubbingLookupEvent - Information about the looked up stubbing + * + * @see StubbingLookupEvent */ void onStubbingLookup(StubbingLookupEvent stubbingLookupEvent); } diff --git a/src/main/java/org/mockito/mock/MockCreationSettings.java b/src/main/java/org/mockito/mock/MockCreationSettings.java index 7e74be8a90..4460b0881a 100644 --- a/src/main/java/org/mockito/mock/MockCreationSettings.java +++ b/src/main/java/org/mockito/mock/MockCreationSettings.java @@ -8,6 +8,7 @@ import org.mockito.Incubating; import org.mockito.MockSettings; import org.mockito.NotExtensible; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.listeners.InvocationListener; import org.mockito.listeners.VerificationStartedListener; import org.mockito.quality.Strictness; @@ -70,7 +71,13 @@ public interface MockCreationSettings { boolean isStripAnnotations(); /** - * {@link InvocationListener} instances attached to this mock, see {@link org.mockito.MockSettings#invocationListeners}. + * TODO x document mutability + * {@link StubbingLookupListener} instances attached to this mock via {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. + */ + List getStubbingLookupListeners(); + + /** + * {@link InvocationListener} instances attached to this mock, see {@link org.mockito.MockSettings#addListeners}. */ List getInvocationListeners(); diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 18911e4ddf..c9f781f1af 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -10,6 +10,7 @@ import org.mockito.exceptions.base.MockitoException; import org.mockito.internal.debugging.VerboseMockInvocationLogger; import org.mockito.listeners.InvocationListener; +import org.mockito.listeners.StubbingLookupListener; import org.mockitoutil.TestBase; import java.util.LinkedList; @@ -24,6 +25,7 @@ public class MockSettingsImplTest extends TestBase { private MockSettingsImpl mockSettingsImpl = new MockSettingsImpl(); @Mock private InvocationListener invocationListener; + @Mock private StubbingLookupListener stubbingLookupListener; @Test(expected=MockitoException.class) @SuppressWarnings("unchecked") @@ -163,4 +165,70 @@ public void shouldReportErrorWhenAddingANullInvocationListener() throws Exceptio Assertions.assertThat(e.getMessage()).contains("does not accept null"); } } + + @Test + public void addListeners_shouldNotAllowNullListener() { + try { + mockSettingsImpl.stubbingLookupListeners((StubbingLookupListener[])null); + fail(); + } catch (MockitoException e) { + assertThat(e.getMessage()).contains("null vararg array"); + } + } + + @Test + public void addListeners_shouldNotAllowEmptyListener() { + try { + mockSettingsImpl.stubbingLookupListeners(); + fail(); + } catch (MockitoException e) { + assertThat(e.getMessage()).contains("at least one listener"); + } + } + + @Test + public void addListeners_shouldReportErrorWhenAddingANullListener() { + try { + mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener, null); + fail(); + } catch (Exception e) { + assertThat(e.getMessage()).contains("does not accept null"); + } + } + + @Test + public void addListeners_shouldAddMockObjectListeners() { + //given + assertTrue(mockSettingsImpl.getInvocationListeners().isEmpty()); + assertTrue(mockSettingsImpl.getStubbingLookupListeners().isEmpty()); + + //when + mockSettingsImpl.invocationListeners(invocationListener); + mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener); + + //then + assertThat(mockSettingsImpl.getInvocationListeners()).contains(invocationListener); + assertThat(mockSettingsImpl.getStubbingLookupListeners()).contains(stubbingLookupListener); + + //TODO x add test that we can remove listeners + } + + @Test + public void addListeners_canAddDuplicateMockObjectListeners_ItsNotOurBusinessThere() { + //given + assertTrue(mockSettingsImpl.getInvocationListeners().isEmpty()); + assertTrue(mockSettingsImpl.getStubbingLookupListeners().isEmpty()); + + //when + mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener) + .stubbingLookupListeners(stubbingLookupListener) + .invocationListeners(invocationListener) + .invocationListeners(invocationListener); + + //then + assertThat(mockSettingsImpl.getInvocationListeners()) + .containsSequence(invocationListener, invocationListener); + assertThat(mockSettingsImpl.getStubbingLookupListeners()) + .containsSequence(stubbingLookupListener, stubbingLookupListener); + } } diff --git a/src/test/java/org/mockito/internal/listeners/StubbingLookupNotifierTest.java b/src/test/java/org/mockito/internal/listeners/StubbingLookupNotifierTest.java index ac963c4855..40e5396920 100644 --- a/src/test/java/org/mockito/internal/listeners/StubbingLookupNotifierTest.java +++ b/src/test/java/org/mockito/internal/listeners/StubbingLookupNotifierTest.java @@ -9,6 +9,7 @@ import org.mockito.ArgumentMatcher; import org.mockito.internal.creation.settings.CreationSettings; import org.mockito.invocation.Invocation; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.stubbing.Stubbing; import org.mockitoutil.TestBase; diff --git a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java new file mode 100644 index 0000000000..81117985ac --- /dev/null +++ b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java @@ -0,0 +1,153 @@ +/* + * Copyright (c) 2018 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockitousage.debugging; + +import org.junit.Test; +import org.mockito.ArgumentMatcher; +import org.mockito.InOrder; +import org.mockito.invocation.Invocation; +import org.mockito.listeners.StubbingLookupEvent; +import org.mockito.listeners.StubbingLookupListener; +import org.mockito.mock.MockCreationSettings; +import org.mockito.stubbing.Stubbing; +import org.mockitoutil.TestBase; + +import java.util.Collection; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +public class StubbingLookupListenerCallbackTest extends TestBase { + + @Test + public void should_call_listener_when_mock_return_normally_with_stubbed_answer() { + // given + final AnswerListener listener = spy(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); + doReturn("coke").when(foo).giveMeSomeString("soda"); + doReturn("java").when(foo).giveMeSomeString("coffee"); + + // when + foo.giveMeSomeString("soda"); + + // then + verify(listener).onStubbingLookup(argThat(new ArgumentMatcher() { + @Override + public boolean matches(StubbingLookupEvent argument) { + assertEquals("soda", argument.getInvocation().getArgument(0)); + assertEquals("foo", argument.getMockSettings().getMockName().toString()); + assertEquals(2, argument.getAllStubbings().size()); + assertNotNull(argument.getStubbingFound()); + return true; + } + })); + } + + @Test + public void should_call_listener_when_mock_return_normally_with_default_answer() { + // given + AnswerListener listener = spy(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); + doReturn("java").when(foo).giveMeSomeString("coffee"); + + // when + foo.giveMeSomeString("soda"); + + // then + verify(listener).onStubbingLookup(argThat(new ArgumentMatcher() { + @Override + public boolean matches(StubbingLookupEvent argument) { + assertEquals("soda", argument.getInvocation().getArgument(0)); + assertEquals("foo", argument.getMockSettings().getMockName().toString()); + assertEquals(1, argument.getAllStubbings().size()); + assertNull(argument.getStubbingFound()); + return true; + } + })); + } + + @Test + public void should_not_call_listener_when_mock_is_not_called() { + // given + AnswerListener listener = spy(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); + doReturn("java").when(foo).giveMeSomeString("coffee"); + + // when nothing + + // then + verifyZeroInteractions(listener); + } + + @Test + public void should_allow_same_listener() { + // given + AnswerListener listener = mock(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener)); + + // when + foo.giveMeSomeString("tea"); + foo.giveMeSomeString("coke"); + + // then each listener was notified 2 times (notified 4 times in total) + verify(listener, times(4)).onStubbingLookup(any(StubbingLookupEvent.class)); + } + + @Test + public void should_call_all_listeners_in_order() { + // given + AnswerListener listener1 = mock(AnswerListener.class); + AnswerListener listener2 = mock(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + doReturn("sprite").when(foo).giveMeSomeString("soda"); + + // when + foo.giveMeSomeString("soda"); + + // then + InOrder inOrder = inOrder(listener1, listener2); + inOrder.verify(listener1).onStubbingLookup(any(StubbingLookupEvent.class)); + inOrder.verify(listener2).onStubbingLookup(any(StubbingLookupEvent.class)); + } + + @Test + public void should_call_all_listeners_when_mock_throws_exception() { + // given + AnswerListener listener1 = mock(AnswerListener.class); + AnswerListener listener2 = mock(AnswerListener.class); + Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + doThrow(new NoWater()).when(foo).giveMeSomeString("tea"); + + // when + try { + foo.giveMeSomeString("tea"); + fail(); + } catch (NoWater e) { + // then + verify(listener1).onStubbingLookup(any(StubbingLookupEvent.class)); + verify(listener2).onStubbingLookup(any(StubbingLookupEvent.class)); + } + } + + //TODO x deleted listener + + private static class AnswerListener implements StubbingLookupListener { + private Invocation invocation; + private Stubbing stubbing; + private Collection allStubbings; + private MockCreationSettings mockSettings; + + public AnswerListener() {} + + public void onStubbingLookup(StubbingLookupEvent event) { + this.invocation = event.getInvocation(); + this.stubbing = event.getStubbingFound(); + this.allStubbings = event.getAllStubbings(); + this.mockSettings = event.getMockSettings(); + } + } + + private static class NoWater extends RuntimeException {} +} From bbcbf3cf2b2f4d098a8bd064e90bd422eeaed264 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 09:03:16 -0700 Subject: [PATCH 18/42] Tidy-up --- .../mockito/internal/creation/settings/CreationSettings.java | 1 + .../mockito/internal/junit/StrictStubsRunnerTestListener.java | 1 - src/main/java/org/mockito/listeners/StubbingLookupListener.java | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-) 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 acae4a2f9f..3909a9e77b 100644 --- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java +++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java @@ -46,6 +46,7 @@ public CreationSettings() {} @SuppressWarnings("unchecked") public CreationSettings(CreationSettings copy) { + //TODO x can we have a reflection test here? this.typeToMock = copy.typeToMock; this.extraInterfaces = copy.extraInterfaces; this.name = copy.name; diff --git a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java index 227c8c42e2..4e01fc1090 100644 --- a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java +++ b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java @@ -21,7 +21,6 @@ public void testFinished(TestFinishedEvent event) {} public void onMockCreated(Object mock, MockCreationSettings settings) { //It is not ideal that we modify the state of MockCreationSettings object //MockCreationSettings is intended to be an immutable view of the creation settings - //In future, we should start passing MockSettings object to the creation listener settings.getStubbingLookupListeners().add(stubbingLookupListener); } } diff --git a/src/main/java/org/mockito/listeners/StubbingLookupListener.java b/src/main/java/org/mockito/listeners/StubbingLookupListener.java index e725aa4af9..11011eb8ea 100644 --- a/src/main/java/org/mockito/listeners/StubbingLookupListener.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupListener.java @@ -27,8 +27,6 @@ public interface StubbingLookupListener { * Called by the framework when Mockito looked up an answer for invocation on a mock. * * @param stubbingLookupEvent - Information about the looked up stubbing - * - * @see StubbingLookupEvent */ void onStubbingLookup(StubbingLookupEvent stubbingLookupEvent); } From 2a812b4eecb9e76ff39155fa9957af32f65d63e8 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 09:04:46 -0700 Subject: [PATCH 19/42] Reduced duplication, simplified tests --- .../internal/creation/MockSettingsImplTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index c9f781f1af..0f341b0091 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -197,11 +197,13 @@ public void addListeners_shouldReportErrorWhenAddingANullListener() { } @Test - public void addListeners_shouldAddMockObjectListeners() { - //given + public void addListeners_has_empty_listeners_by_default() { assertTrue(mockSettingsImpl.getInvocationListeners().isEmpty()); assertTrue(mockSettingsImpl.getStubbingLookupListeners().isEmpty()); + } + @Test + public void addListeners_shouldAddMockObjectListeners() { //when mockSettingsImpl.invocationListeners(invocationListener); mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener); @@ -215,10 +217,6 @@ public void addListeners_shouldAddMockObjectListeners() { @Test public void addListeners_canAddDuplicateMockObjectListeners_ItsNotOurBusinessThere() { - //given - assertTrue(mockSettingsImpl.getInvocationListeners().isEmpty()); - assertTrue(mockSettingsImpl.getStubbingLookupListeners().isEmpty()); - //when mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener) .stubbingLookupListeners(stubbingLookupListener) From 5367e88d5dc2e0819c93cf67ca7b7e5ff774657b Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 09:18:37 -0700 Subject: [PATCH 20/42] Simplified test, removed duplicated TODO --- .../creation/MockSettingsImplTest.java | 2 - .../StubbingLookupListenerCallbackTest.java | 77 ++++++------------- 2 files changed, 25 insertions(+), 54 deletions(-) diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 0f341b0091..0cd7b54617 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -211,8 +211,6 @@ public void addListeners_shouldAddMockObjectListeners() { //then assertThat(mockSettingsImpl.getInvocationListeners()).contains(invocationListener); assertThat(mockSettingsImpl.getStubbingLookupListeners()).contains(stubbingLookupListener); - - //TODO x add test that we can remove listeners } @Test diff --git a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java index 81117985ac..e43d765597 100644 --- a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java +++ b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java @@ -7,37 +7,33 @@ import org.junit.Test; import org.mockito.ArgumentMatcher; import org.mockito.InOrder; -import org.mockito.invocation.Invocation; import org.mockito.listeners.StubbingLookupEvent; import org.mockito.listeners.StubbingLookupListener; -import org.mockito.mock.MockCreationSettings; -import org.mockito.stubbing.Stubbing; import org.mockitoutil.TestBase; -import java.util.Collection; - import static org.junit.Assert.*; import static org.mockito.Mockito.*; public class StubbingLookupListenerCallbackTest extends TestBase { + final StubbingLookupListener listener = mock(StubbingLookupListener.class); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); + @Test public void should_call_listener_when_mock_return_normally_with_stubbed_answer() { // given - final AnswerListener listener = spy(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); - doReturn("coke").when(foo).giveMeSomeString("soda"); - doReturn("java").when(foo).giveMeSomeString("coffee"); + doReturn("coke").when(mock).giveMeSomeString("soda"); + doReturn("java").when(mock).giveMeSomeString("coffee"); // when - foo.giveMeSomeString("soda"); + mock.giveMeSomeString("soda"); // then verify(listener).onStubbingLookup(argThat(new ArgumentMatcher() { @Override public boolean matches(StubbingLookupEvent argument) { assertEquals("soda", argument.getInvocation().getArgument(0)); - assertEquals("foo", argument.getMockSettings().getMockName().toString()); + assertEquals("mock", argument.getMockSettings().getMockName().toString()); assertEquals(2, argument.getAllStubbings().size()); assertNotNull(argument.getStubbingFound()); return true; @@ -48,19 +44,17 @@ public boolean matches(StubbingLookupEvent argument) { @Test public void should_call_listener_when_mock_return_normally_with_default_answer() { // given - AnswerListener listener = spy(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); - doReturn("java").when(foo).giveMeSomeString("coffee"); + doReturn("java").when(mock).giveMeSomeString("coffee"); // when - foo.giveMeSomeString("soda"); + mock.giveMeSomeString("soda"); // then verify(listener).onStubbingLookup(argThat(new ArgumentMatcher() { @Override public boolean matches(StubbingLookupEvent argument) { assertEquals("soda", argument.getInvocation().getArgument(0)); - assertEquals("foo", argument.getMockSettings().getMockName().toString()); + assertEquals("mock", argument.getMockSettings().getMockName().toString()); assertEquals(1, argument.getAllStubbings().size()); assertNull(argument.getStubbingFound()); return true; @@ -70,12 +64,8 @@ public boolean matches(StubbingLookupEvent argument) { @Test public void should_not_call_listener_when_mock_is_not_called() { - // given - AnswerListener listener = spy(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); - doReturn("java").when(foo).giveMeSomeString("coffee"); - - // when nothing + // when stubbing is recorded + doReturn("java").when(mock).giveMeSomeString("coffee"); // then verifyZeroInteractions(listener); @@ -84,12 +74,11 @@ public void should_not_call_listener_when_mock_is_not_called() { @Test public void should_allow_same_listener() { // given - AnswerListener listener = mock(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener)); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener)); // when - foo.giveMeSomeString("tea"); - foo.giveMeSomeString("coke"); + mock.giveMeSomeString("tea"); + mock.giveMeSomeString("coke"); // then each listener was notified 2 times (notified 4 times in total) verify(listener, times(4)).onStubbingLookup(any(StubbingLookupEvent.class)); @@ -98,13 +87,13 @@ public void should_allow_same_listener() { @Test public void should_call_all_listeners_in_order() { // given - AnswerListener listener1 = mock(AnswerListener.class); - AnswerListener listener2 = mock(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); - doReturn("sprite").when(foo).giveMeSomeString("soda"); + StubbingLookupListener listener1 = mock(StubbingLookupListener.class); + StubbingLookupListener listener2 = mock(StubbingLookupListener.class); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + doReturn("sprite").when(mock).giveMeSomeString("soda"); // when - foo.giveMeSomeString("soda"); + mock.giveMeSomeString("soda"); // then InOrder inOrder = inOrder(listener1, listener2); @@ -115,14 +104,14 @@ public void should_call_all_listeners_in_order() { @Test public void should_call_all_listeners_when_mock_throws_exception() { // given - AnswerListener listener1 = mock(AnswerListener.class); - AnswerListener listener2 = mock(AnswerListener.class); - Foo foo = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); - doThrow(new NoWater()).when(foo).giveMeSomeString("tea"); + StubbingLookupListener listener1 = mock(StubbingLookupListener.class); + StubbingLookupListener listener2 = mock(StubbingLookupListener.class); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + doThrow(new NoWater()).when(mock).giveMeSomeString("tea"); // when try { - foo.giveMeSomeString("tea"); + mock.giveMeSomeString("tea"); fail(); } catch (NoWater e) { // then @@ -133,21 +122,5 @@ public void should_call_all_listeners_when_mock_throws_exception() { //TODO x deleted listener - private static class AnswerListener implements StubbingLookupListener { - private Invocation invocation; - private Stubbing stubbing; - private Collection allStubbings; - private MockCreationSettings mockSettings; - - public AnswerListener() {} - - public void onStubbingLookup(StubbingLookupEvent event) { - this.invocation = event.getInvocation(); - this.stubbing = event.getStubbingFound(); - this.allStubbings = event.getAllStubbings(); - this.mockSettings = event.getMockSettings(); - } - } - private static class NoWater extends RuntimeException {} } From dcddad903c3015c6faca83b16071de84aa6dce93 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 09:22:49 -0700 Subject: [PATCH 21/42] Added missing coverage --- .../StubbingLookupListenerCallbackTest.java | 45 ++++++++++++++----- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java index e43d765597..aa9f62f952 100644 --- a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java +++ b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java @@ -16,7 +16,8 @@ public class StubbingLookupListenerCallbackTest extends TestBase { - final StubbingLookupListener listener = mock(StubbingLookupListener.class); + StubbingLookupListener listener = mock(StubbingLookupListener.class); + StubbingLookupListener listener2 = mock(StubbingLookupListener.class); Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener)); @Test @@ -87,26 +88,22 @@ public void should_allow_same_listener() { @Test public void should_call_all_listeners_in_order() { // given - StubbingLookupListener listener1 = mock(StubbingLookupListener.class); - StubbingLookupListener listener2 = mock(StubbingLookupListener.class); - Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener2)); doReturn("sprite").when(mock).giveMeSomeString("soda"); // when mock.giveMeSomeString("soda"); // then - InOrder inOrder = inOrder(listener1, listener2); - inOrder.verify(listener1).onStubbingLookup(any(StubbingLookupEvent.class)); + InOrder inOrder = inOrder(listener, listener2); + inOrder.verify(listener).onStubbingLookup(any(StubbingLookupEvent.class)); inOrder.verify(listener2).onStubbingLookup(any(StubbingLookupEvent.class)); } @Test public void should_call_all_listeners_when_mock_throws_exception() { // given - StubbingLookupListener listener1 = mock(StubbingLookupListener.class); - StubbingLookupListener listener2 = mock(StubbingLookupListener.class); - Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener1, listener2)); + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener2)); doThrow(new NoWater()).when(mock).giveMeSomeString("tea"); // when @@ -115,12 +112,38 @@ public void should_call_all_listeners_when_mock_throws_exception() { fail(); } catch (NoWater e) { // then - verify(listener1).onStubbingLookup(any(StubbingLookupEvent.class)); + verify(listener).onStubbingLookup(any(StubbingLookupEvent.class)); verify(listener2).onStubbingLookup(any(StubbingLookupEvent.class)); } } - //TODO x deleted listener + @Test + public void should_delete_listener() { + // given + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener2)); + + // when + mock.doSomething("1"); + mockingDetails(mock).getMockCreationSettings().getStubbingLookupListeners().remove(listener2); + mock.doSomething("2"); + + // then + verify(listener, times(2)).onStubbingLookup(any(StubbingLookupEvent.class)); + verify(listener2, times(1)).onStubbingLookup(any(StubbingLookupEvent.class)); + } + + @Test + public void should_clear_listeners() { + // given + Foo mock = mock(Foo.class, withSettings().stubbingLookupListeners(listener, listener2)); + + // when + mockingDetails(mock).getMockCreationSettings().getStubbingLookupListeners().clear(); + mock.doSomething("foo"); + + // then + verifyZeroInteractions(listener, listener2); + } private static class NoWater extends RuntimeException {} } From c018fe9866f91a079dacb00c576c3d880cc4bd7b Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 16:26:40 -0700 Subject: [PATCH 22/42] Added explicit coverage for one of the methods --- .../internal/creation/MockSettingsImpl.java | 3 +-- .../creation/MockSettingsImplTest.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java index d842e3e8e3..50134b4458 100644 --- a/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java +++ b/src/main/java/org/mockito/internal/creation/MockSettingsImpl.java @@ -184,8 +184,7 @@ public MockSettings stubbingLookupListeners(StubbingLookupListener... listeners) return this; } - //TODO X dedicated unit tests - private static void addListeners(T[] listeners, List container, String method) { + static void addListeners(T[] listeners, List container, String method) { if (listeners == null) { throw methodDoesNotAcceptParameter(method, "null vararg array."); } diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 0cd7b54617..6fc554a743 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -5,6 +5,7 @@ package org.mockito.internal.creation; import org.assertj.core.api.Assertions; +import org.assertj.core.api.ThrowableAssert; import org.junit.Test; import org.mockito.Mock; import org.mockito.exceptions.base.MockitoException; @@ -166,6 +167,27 @@ public void shouldReportErrorWhenAddingANullInvocationListener() throws Exceptio } } + @Test + public void validates_listeners() { + Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.addListeners(new Object[] {}, new LinkedList(), "myListeners"); + } + }).hasMessageContaining("myListeners() requires at least one listener"); + + Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.addListeners(null, new LinkedList(), "myListeners"); + } + }).hasMessageContaining("myListeners() does not accept null vararg array"); + + Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.addListeners(new Object[] {null}, new LinkedList(), "myListeners"); + } + }).hasMessageContaining("myListeners() does not accept null listeners"); + } + @Test public void addListeners_shouldNotAllowNullListener() { try { From adadd0fdd2d42491c1ee5f7751d63b69ac4373ec Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 16:28:50 -0700 Subject: [PATCH 23/42] Refactored tests for cleanness --- .../creation/MockSettingsImplTest.java | 49 ++++++++----------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 6fc554a743..5fc5b0781d 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.Set; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.*; import static org.assertj.core.api.Assertions.assertThat; @@ -169,53 +170,45 @@ public void shouldReportErrorWhenAddingANullInvocationListener() throws Exceptio @Test public void validates_listeners() { - Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { public void call() { mockSettingsImpl.addListeners(new Object[] {}, new LinkedList(), "myListeners"); } }).hasMessageContaining("myListeners() requires at least one listener"); - Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { public void call() { mockSettingsImpl.addListeners(null, new LinkedList(), "myListeners"); } }).hasMessageContaining("myListeners() does not accept null vararg array"); - Assertions.assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { public void call() { mockSettingsImpl.addListeners(new Object[] {null}, new LinkedList(), "myListeners"); } }).hasMessageContaining("myListeners() does not accept null listeners"); } - @Test - public void addListeners_shouldNotAllowNullListener() { - try { - mockSettingsImpl.stubbingLookupListeners((StubbingLookupListener[])null); - fail(); - } catch (MockitoException e) { - assertThat(e.getMessage()).contains("null vararg array"); - } - } @Test - public void addListeners_shouldNotAllowEmptyListener() { - try { - mockSettingsImpl.stubbingLookupListeners(); - fail(); - } catch (MockitoException e) { - assertThat(e.getMessage()).contains("at least one listener"); - } - } + public void validates_stubbing_lookup_listeners() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.stubbingLookupListeners(new StubbingLookupListener[] {}); + } + }).hasMessageContaining("stubbingLookupListeners() requires at least one listener"); - @Test - public void addListeners_shouldReportErrorWhenAddingANullListener() { - try { - mockSettingsImpl.stubbingLookupListeners(stubbingLookupListener, null); - fail(); - } catch (Exception e) { - assertThat(e.getMessage()).contains("does not accept null"); - } + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.stubbingLookupListeners(null); + } + }).hasMessageContaining("stubbingLookupListeners() does not accept null vararg array"); + + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.stubbingLookupListeners(new StubbingLookupListener[] {null}); + } + }).hasMessageContaining("stubbingLookupListeners() does not accept null listeners"); } @Test From dec573266dd7258d94ad244556a0583ef50b570e Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 16:30:43 -0700 Subject: [PATCH 24/42] Refactored tests Testing of the similar logic should be standardized in a given test class so that it is easier to read/maintain. --- .../creation/MockSettingsImplTest.java | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 5fc5b0781d..4e4b9a88e4 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -116,12 +116,6 @@ public void shouldAddVerboseLoggingListenerOnlyOnce() { Assertions.assertThat(mockSettingsImpl.getInvocationListeners()).hasSize(1); } - @SuppressWarnings("unchecked") - @Test(expected=MockitoException.class) - public void shouldNotAllowNullListener() { - mockSettingsImpl.invocationListeners((InvocationListener[])null); - } - @Test @SuppressWarnings("unchecked") public void shouldAddInvocationListener() { @@ -148,26 +142,6 @@ public void canAddDuplicateInvocationListeners_ItsNotOurBusinessThere() { Assertions.assertThat(mockSettingsImpl.getInvocationListeners()).containsSequence(invocationListener, invocationListener, invocationListener); } - @Test - public void shouldReportErrorWhenAddingNoInvocationListeners() throws Exception { - try { - mockSettingsImpl.invocationListeners(); - fail(); - } catch (Exception e) { - Assertions.assertThat(e.getMessage()).contains("at least one listener"); - } - } - - @Test - public void shouldReportErrorWhenAddingANullInvocationListener() throws Exception { - try { - mockSettingsImpl.invocationListeners(invocationListener, null); - fail(); - } catch (Exception e) { - Assertions.assertThat(e.getMessage()).contains("does not accept null"); - } - } - @Test public void validates_listeners() { assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { @@ -211,6 +185,27 @@ public void call() { }).hasMessageContaining("stubbingLookupListeners() does not accept null listeners"); } + @Test + public void validates_invocation_listeners() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.invocationListeners(new InvocationListener[] {}); + } + }).hasMessageContaining("invocationListeners() requires at least one listener"); + + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.invocationListeners(null); + } + }).hasMessageContaining("invocationListeners() does not accept null vararg array"); + + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + public void call() { + mockSettingsImpl.invocationListeners(new InvocationListener[] {null}); + } + }).hasMessageContaining("invocationListeners() does not accept null listeners"); + } + @Test public void addListeners_has_empty_listeners_by_default() { assertTrue(mockSettingsImpl.getInvocationListeners().isEmpty()); From eedbb36c338d3d08857ed9c78f4b4804f1fdf475 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sun, 16 Sep 2018 16:47:36 -0700 Subject: [PATCH 25/42] Concurrent scenario for stubbing lookup listeners --- .../creation/settings/CreationSettings.java | 6 ++-- .../mockito/mock/MockCreationSettings.java | 2 +- .../StubbingLookupListenerCallbackTest.java | 33 +++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) 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 3909a9e77b..89ddc7c406 100644 --- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java +++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java @@ -18,6 +18,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; public class CreationSettings implements MockCreationSettings, Serializable { private static final long serialVersionUID = -6789800638070123629L; @@ -31,8 +32,9 @@ public class CreationSettings implements MockCreationSettings, Serializabl protected SerializableMode serializableMode = SerializableMode.NONE; protected List invocationListeners = new ArrayList(); - //TODO X - concurrent scenario? - protected List stubbingLookupListeners = new ArrayList(); + //Other listeners in this class may also need concurrency-safe implementation + //However, no issue was reported yet, so we only protect stubbing lookup listeners (new code) + protected List stubbingLookupListeners = new CopyOnWriteArrayList(); protected List verificationStartedListeners = new LinkedList(); protected boolean stubOnly; diff --git a/src/main/java/org/mockito/mock/MockCreationSettings.java b/src/main/java/org/mockito/mock/MockCreationSettings.java index 4460b0881a..66deddf9bd 100644 --- a/src/main/java/org/mockito/mock/MockCreationSettings.java +++ b/src/main/java/org/mockito/mock/MockCreationSettings.java @@ -77,7 +77,7 @@ public interface MockCreationSettings { List getStubbingLookupListeners(); /** - * {@link InvocationListener} instances attached to this mock, see {@link org.mockito.MockSettings#addListeners}. + * {@link InvocationListener} instances attached to this mock, see {@link org.mockito.MockSettings#invocationListeners(InvocationListener...)}. */ List getInvocationListeners(); diff --git a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java index aa9f62f952..fce9e1dd26 100644 --- a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java +++ b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java @@ -9,8 +9,14 @@ import org.mockito.InOrder; import org.mockito.listeners.StubbingLookupEvent; import org.mockito.listeners.StubbingLookupListener; +import org.mockito.mock.MockCreationSettings; +import org.mockitousage.IMethods; +import org.mockitoutil.ConcurrentTesting; import org.mockitoutil.TestBase; +import java.util.LinkedList; +import java.util.List; + import static org.junit.Assert.*; import static org.mockito.Mockito.*; @@ -145,5 +151,32 @@ public void should_clear_listeners() { verifyZeroInteractions(listener, listener2); } + @Test + public void add_listeners_concurrently_sanity_check() throws Exception { + //given + final IMethods mock = mock(IMethods.class); + final MockCreationSettings settings = mockingDetails(mock).getMockCreationSettings(); + + List runnables = new LinkedList(); + for (int i = 0; i < 50; i++) { + runnables.add(new Runnable() { + public void run() { + StubbingLookupListener listener1 = mock(StubbingLookupListener.class); + StubbingLookupListener listener2 = mock(StubbingLookupListener.class); + settings.getStubbingLookupListeners().add(listener1); + settings.getStubbingLookupListeners().add(listener2); + settings.getStubbingLookupListeners().remove(listener1); + } + }); + } + + //when + ConcurrentTesting.concurrently(runnables.toArray(new Runnable[runnables.size()])); + + //then + //This assertion may be flaky. If it is let's fix it or remove the test. For now, I'm keeping the test. + assertEquals(50, settings.getStubbingLookupListeners().size()); + } + private static class NoWater extends RuntimeException {} } From 45bbfe8b4ea342b182aff5b24c138cd499b86452 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 24 Nov 2018 08:16:31 -0800 Subject: [PATCH 26/42] Since tags and documentation --- src/main/java/org/mockito/MockSettings.java | 2 +- .../creation/settings/CreationSettings.java | 2 +- .../junit/StrictStubsRunnerTestListener.java | 2 ++ .../listeners/StubbingLookupEvent.java | 7 +++++++ .../listeners/StubbingLookupListener.java | 21 +++++++++++-------- .../mockito/mock/MockCreationSettings.java | 10 ++++++--- 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/mockito/MockSettings.java b/src/main/java/org/mockito/MockSettings.java index bf98c2af6e..e5cf8d0a6a 100644 --- a/src/main/java/org/mockito/MockSettings.java +++ b/src/main/java/org/mockito/MockSettings.java @@ -218,7 +218,7 @@ public interface MockSettings extends Serializable { * * @param listeners The stubbing lookup listeners to add. May not be null. * @return settings instance so that you can fluently specify other settings - * @since TODO x here and everywhere else + * @since 2.23.5 */ MockSettings stubbingLookupListeners(StubbingLookupListener... listeners); 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 89ddc7c406..b2c15e1c13 100644 --- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java +++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java @@ -48,7 +48,7 @@ public CreationSettings() {} @SuppressWarnings("unchecked") public CreationSettings(CreationSettings copy) { - //TODO x can we have a reflection test here? + //TODO can we have a reflection test here? We had a couple of bugs here in the past. this.typeToMock = copy.typeToMock; this.extraInterfaces = copy.extraInterfaces; this.name = copy.name; diff --git a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java index 4e01fc1090..d23431bae1 100644 --- a/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java +++ b/src/main/java/org/mockito/internal/junit/StrictStubsRunnerTestListener.java @@ -21,6 +21,8 @@ public void testFinished(TestFinishedEvent event) {} public void onMockCreated(Object mock, MockCreationSettings settings) { //It is not ideal that we modify the state of MockCreationSettings object //MockCreationSettings is intended to be an immutable view of the creation settings + //However, we our previous listeners work this way and it hasn't backfired. + //Since it is simple and pragmatic, we'll keep it for now. settings.getStubbingLookupListeners().add(stubbingLookupListener); } } diff --git a/src/main/java/org/mockito/listeners/StubbingLookupEvent.java b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java index a5d78b341b..e18f1f407c 100644 --- a/src/main/java/org/mockito/listeners/StubbingLookupEvent.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java @@ -12,25 +12,32 @@ /** * Represent an information about the looked up stubbing + * + * @since 2.23.5 */ public interface StubbingLookupEvent { + /** * @return The invocation that causes stubbing lookup + * @since 2.23.5 */ Invocation getInvocation(); /** * @return Looked up stubbing. It can be null, which indicates that the invocation was not stubbed + * @since 2.23.5 */ Stubbing getStubbingFound(); /** * @return All stubbings declared on the mock object that we are invoking + * @since 2.23.5 */ Collection getAllStubbings(); /** * @return Settings of the mock object that we are invoking + * @since 2.23.5 */ MockCreationSettings getMockSettings(); } diff --git a/src/main/java/org/mockito/listeners/StubbingLookupListener.java b/src/main/java/org/mockito/listeners/StubbingLookupListener.java index 11011eb8ea..7b9991ab31 100644 --- a/src/main/java/org/mockito/listeners/StubbingLookupListener.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupListener.java @@ -5,21 +5,23 @@ package org.mockito.listeners; import org.mockito.MockSettings; +import org.mockito.mock.MockCreationSettings; /** - * This listener can be notified of looking up stubbing answer for a given mock. - * - * For this to happen, it must be registered using {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. - * - * TODO x use case, mutability - * + * When a method is called on a mock object Mockito looks up any stubbings recorded on that mock. + * This listener gets notified on stubbing lookup. + * Register listener via {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. + * This API is used by Mockito to implement {@link org.mockito.exceptions.misusing.PotentialStubbingProblem} + * (part of Mockito {@link org.mockito.quality.Strictness}. *

- * How does it work? - * When method is called on the mock object, Mockito looks for any answer (stubbing) declared on that mock. + * Details: When method is called on the mock object, Mockito looks for any answer (stubbing) declared on that mock. * If the stubbed answer is found, that answer is invoked (value returned, thrown exception, etc.). * If the answer is not found (e.g. that invocation was not stubbed on the mock), mock's default answer is used. - * This listener implementation is notified when Mockito looked up an answer for invocation on a mock. + * This listener implementation is notified when Mockito attempted to find an answer for invocation on a mock. *

+ * The listeners can be accessed via {@link MockCreationSettings#getStubbingLookupListeners()}. + * + * @since 2.23.5 */ public interface StubbingLookupListener { @@ -27,6 +29,7 @@ public interface StubbingLookupListener { * Called by the framework when Mockito looked up an answer for invocation on a mock. * * @param stubbingLookupEvent - Information about the looked up stubbing + * @since 2.23.5 */ void onStubbingLookup(StubbingLookupEvent stubbingLookupEvent); } diff --git a/src/main/java/org/mockito/mock/MockCreationSettings.java b/src/main/java/org/mockito/mock/MockCreationSettings.java index 66deddf9bd..0ca38a5b24 100644 --- a/src/main/java/org/mockito/mock/MockCreationSettings.java +++ b/src/main/java/org/mockito/mock/MockCreationSettings.java @@ -8,8 +8,8 @@ import org.mockito.Incubating; import org.mockito.MockSettings; import org.mockito.NotExtensible; -import org.mockito.listeners.StubbingLookupListener; import org.mockito.listeners.InvocationListener; +import org.mockito.listeners.StubbingLookupListener; import org.mockito.listeners.VerificationStartedListener; import org.mockito.quality.Strictness; import org.mockito.stubbing.Answer; @@ -71,8 +71,12 @@ public interface MockCreationSettings { boolean isStripAnnotations(); /** - * TODO x document mutability - * {@link StubbingLookupListener} instances attached to this mock via {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. + * Returns {@link StubbingLookupListener} instances attached to this mock via {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}. + * The resulting list is mutable, you can add/remove listeners even after the mock was created. + *

+ * For more details see {@link StubbingLookupListener}. + * + * @since 2.23.5 */ List getStubbingLookupListeners(); From 2797d35cec861b3b56d8a0efe5923274d9661107 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 24 Nov 2018 08:30:25 -0800 Subject: [PATCH 27/42] Final documentation tweaks --- src/main/java/org/mockito/MockSettings.java | 4 ++-- .../internal/creation/settings/CreationSettings.java | 4 ++-- .../internal/junit/DefaultStubbingLookupListener.java | 4 ++-- .../java/org/mockito/listeners/StubbingLookupListener.java | 7 ++++--- .../mockito/internal/creation/MockSettingsImplTest.java | 6 ++++-- .../debugging/StubbingLookupListenerCallbackTest.java | 5 ++++- 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/mockito/MockSettings.java b/src/main/java/org/mockito/MockSettings.java index e5cf8d0a6a..7828ee05d6 100644 --- a/src/main/java/org/mockito/MockSettings.java +++ b/src/main/java/org/mockito/MockSettings.java @@ -207,9 +207,9 @@ public interface MockSettings extends Serializable { /** * Add stubbing lookup listener to the mock object. * - * Multiple listeners may be added and they will be notified in the order they were supplied. + * Multiple listeners may be added and they will be notified orderly. * - * For use cases and more info see {@link StubbingLookupListener} + * For use cases and more info see {@link StubbingLookupListener}. * * Example: *


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 b2c15e1c13..3b45592c72 100644
--- a/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java
+++ b/src/main/java/org/mockito/internal/creation/settings/CreationSettings.java
@@ -32,8 +32,8 @@ public class CreationSettings implements MockCreationSettings, Serializabl
     protected SerializableMode serializableMode = SerializableMode.NONE;
     protected List invocationListeners = new ArrayList();
 
-    //Other listeners in this class may also need concurrency-safe implementation
-    //However, no issue was reported yet, so we only protect stubbing lookup listeners (new code)
+    //Other listeners in this class may also need concurrency-safe implementation. However, no issue was reported about it.
+    // If we do it, we need to understand usage patterns and choose the right concurrent implementation.
     protected List stubbingLookupListeners = new CopyOnWriteArrayList();
 
     protected List verificationStartedListeners = new LinkedList();
diff --git a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java
index 11f2c43fc7..b5d2022b39 100644
--- a/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java
+++ b/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java
@@ -5,10 +5,10 @@
 package org.mockito.internal.junit;
 
 import org.mockito.internal.exceptions.Reporter;
-import org.mockito.listeners.StubbingLookupEvent;
-import org.mockito.listeners.StubbingLookupListener;
 import org.mockito.internal.stubbing.UnusedStubbingReporting;
 import org.mockito.invocation.Invocation;
+import org.mockito.listeners.StubbingLookupEvent;
+import org.mockito.listeners.StubbingLookupListener;
 import org.mockito.quality.Strictness;
 import org.mockito.stubbing.Stubbing;
 
diff --git a/src/main/java/org/mockito/listeners/StubbingLookupListener.java b/src/main/java/org/mockito/listeners/StubbingLookupListener.java
index 7b9991ab31..aa4e49ba88 100644
--- a/src/main/java/org/mockito/listeners/StubbingLookupListener.java
+++ b/src/main/java/org/mockito/listeners/StubbingLookupListener.java
@@ -12,12 +12,12 @@
  * This listener gets notified on stubbing lookup.
  * Register listener via {@link MockSettings#stubbingLookupListeners(StubbingLookupListener...)}.
  * This API is used by Mockito to implement {@link org.mockito.exceptions.misusing.PotentialStubbingProblem}
- * (part of Mockito {@link org.mockito.quality.Strictness}.
+ * (part of Mockito {@link org.mockito.quality.Strictness}).
  * 

* Details: When method is called on the mock object, Mockito looks for any answer (stubbing) declared on that mock. - * If the stubbed answer is found, that answer is invoked (value returned, thrown exception, etc.). + * If the stubbed answer is found, that answer is then invoked (value returned, thrown exception, etc.). * If the answer is not found (e.g. that invocation was not stubbed on the mock), mock's default answer is used. - * This listener implementation is notified when Mockito attempted to find an answer for invocation on a mock. + * This listener implementation is notified when Mockito attempts to find an answer for invocation on a mock. *

* The listeners can be accessed via {@link MockCreationSettings#getStubbingLookupListeners()}. * @@ -27,6 +27,7 @@ public interface StubbingLookupListener { /** * Called by the framework when Mockito looked up an answer for invocation on a mock. + * For details, see {@link StubbingLookupListener}. * * @param stubbingLookupEvent - Information about the looked up stubbing * @since 2.23.5 diff --git a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java index 4e4b9a88e4..63cb1e7218 100644 --- a/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java +++ b/src/test/java/org/mockito/internal/creation/MockSettingsImplTest.java @@ -18,9 +18,11 @@ import java.util.List; import java.util.Set; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.Assert.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class MockSettingsImplTest extends TestBase { diff --git a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java index fce9e1dd26..280fc61962 100644 --- a/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java +++ b/src/test/java/org/mockitousage/debugging/StubbingLookupListenerCallbackTest.java @@ -17,7 +17,10 @@ import java.util.LinkedList; import java.util.List; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import static org.mockito.Mockito.*; public class StubbingLookupListenerCallbackTest extends TestBase { From 49a1d85d5684e1a1884e8ba00655a2ceb3d69414 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Wed, 27 Feb 2019 09:37:22 -0800 Subject: [PATCH 28/42] Updated since tags --- src/main/java/org/mockito/MockSettings.java | 2 +- src/main/java/org/mockito/invocation/Location.java | 2 +- .../org/mockito/listeners/StubbingLookupEvent.java | 10 +++++----- .../org/mockito/listeners/StubbingLookupListener.java | 4 ++-- .../java/org/mockito/mock/MockCreationSettings.java | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/mockito/MockSettings.java b/src/main/java/org/mockito/MockSettings.java index 7828ee05d6..95fcfe3236 100644 --- a/src/main/java/org/mockito/MockSettings.java +++ b/src/main/java/org/mockito/MockSettings.java @@ -218,7 +218,7 @@ public interface MockSettings extends Serializable { * * @param listeners The stubbing lookup listeners to add. May not be null. * @return settings instance so that you can fluently specify other settings - * @since 2.23.5 + * @since 2.24.6 */ MockSettings stubbingLookupListeners(StubbingLookupListener... listeners); diff --git a/src/main/java/org/mockito/invocation/Location.java b/src/main/java/org/mockito/invocation/Location.java index 9337668283..43b48324d8 100644 --- a/src/main/java/org/mockito/invocation/Location.java +++ b/src/main/java/org/mockito/invocation/Location.java @@ -23,7 +23,7 @@ public interface Location { * Source file of this location * * @return source file - * @since 2.23.5 + * @since 2.24.6 */ String getSourceFile(); } diff --git a/src/main/java/org/mockito/listeners/StubbingLookupEvent.java b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java index e18f1f407c..a03fb594ea 100644 --- a/src/main/java/org/mockito/listeners/StubbingLookupEvent.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupEvent.java @@ -13,31 +13,31 @@ /** * Represent an information about the looked up stubbing * - * @since 2.23.5 + * @since 2.24.6 */ public interface StubbingLookupEvent { /** * @return The invocation that causes stubbing lookup - * @since 2.23.5 + * @since 2.24.6 */ Invocation getInvocation(); /** * @return Looked up stubbing. It can be null, which indicates that the invocation was not stubbed - * @since 2.23.5 + * @since 2.24.6 */ Stubbing getStubbingFound(); /** * @return All stubbings declared on the mock object that we are invoking - * @since 2.23.5 + * @since 2.24.6 */ Collection getAllStubbings(); /** * @return Settings of the mock object that we are invoking - * @since 2.23.5 + * @since 2.24.6 */ MockCreationSettings getMockSettings(); } diff --git a/src/main/java/org/mockito/listeners/StubbingLookupListener.java b/src/main/java/org/mockito/listeners/StubbingLookupListener.java index aa4e49ba88..b33e063462 100644 --- a/src/main/java/org/mockito/listeners/StubbingLookupListener.java +++ b/src/main/java/org/mockito/listeners/StubbingLookupListener.java @@ -21,7 +21,7 @@ *

* The listeners can be accessed via {@link MockCreationSettings#getStubbingLookupListeners()}. * - * @since 2.23.5 + * @since 2.24.6 */ public interface StubbingLookupListener { @@ -30,7 +30,7 @@ public interface StubbingLookupListener { * For details, see {@link StubbingLookupListener}. * * @param stubbingLookupEvent - Information about the looked up stubbing - * @since 2.23.5 + * @since 2.24.6 */ void onStubbingLookup(StubbingLookupEvent stubbingLookupEvent); } diff --git a/src/main/java/org/mockito/mock/MockCreationSettings.java b/src/main/java/org/mockito/mock/MockCreationSettings.java index 0ca38a5b24..2343b0da64 100644 --- a/src/main/java/org/mockito/mock/MockCreationSettings.java +++ b/src/main/java/org/mockito/mock/MockCreationSettings.java @@ -76,7 +76,7 @@ public interface MockCreationSettings { *

* For more details see {@link StubbingLookupListener}. * - * @since 2.23.5 + * @since 2.24.6 */ List getStubbingLookupListeners(); From 9e43580bffaf48681fca3efc380b9113229005ac Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Wed, 27 Feb 2019 18:12:12 +0000 Subject: [PATCH 29/42] 2.24.6 release (previous 2.24.5) + release notes updated by CI build 3904 [ci skip-release] --- doc/release-notes/official.md | 6 ++++++ version.properties | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index d15b1d6d24..547a79cbc7 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,11 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.6 + - 2019-02-27 - [16 commits](https://github.com/mockito/mockito/compare/v2.24.5...v2.24.6) by [Szczepan Faber](https://github.com/mockitoguy) (15), [Marcin Stachniuk](https://github.com/mstachniuk) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.6-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.6) + - [New features] Expose StubbingLookupListener publicly and create listener distinction [(#793)](https://github.com/mockito/mockito/issues/793) + - Make use of Shipkit v2.1.3 [(#1626)](https://github.com/mockito/mockito/pull/1626) + - Exposed new API - StubbingLookupListener [(#1543)](https://github.com/mockito/mockito/pull/1543) + #### 2.24.5 - 2019-02-18 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.4...v2.24.5) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.5-green.svg)](https://bintray.com/mockito/maven/mockito/2.24.5) - No pull requests referenced in commit messages. diff --git a/version.properties b/version.properties index ddbd9bc93a..f59388fa57 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.6 +version=2.24.7 #Previous version used to generate release notes delta -previousVersion=2.24.5 +previousVersion=2.24.6 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From d41ad6cb1c1018b334769ae595214c6c025f67a4 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Wed, 27 Feb 2019 17:19:18 -0800 Subject: [PATCH 30/42] Removed redundancy in release note [ci skip] --- doc/release-notes/official.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index 547a79cbc7..43b0cf01e8 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -2,7 +2,6 @@ #### 2.24.6 - 2019-02-27 - [16 commits](https://github.com/mockito/mockito/compare/v2.24.5...v2.24.6) by [Szczepan Faber](https://github.com/mockitoguy) (15), [Marcin Stachniuk](https://github.com/mstachniuk) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.6-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.6) - - [New features] Expose StubbingLookupListener publicly and create listener distinction [(#793)](https://github.com/mockito/mockito/issues/793) - Make use of Shipkit v2.1.3 [(#1626)](https://github.com/mockito/mockito/pull/1626) - Exposed new API - StubbingLookupListener [(#1543)](https://github.com/mockito/mockito/pull/1543) From 4a99af3fc920fa2dd8379da7f9a24dd61a7ee39e Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Thu, 28 Feb 2019 14:25:03 +0000 Subject: [PATCH 31/42] Fix handling of generics in ReturnsMocks (#1635) ReturnsMocks was exhibiting the same problems as we previously had with ReturnsSmartNulls. Extract that common behavior into a separate class and thus fix the issues with ReturnsMocks. --- .../RetrieveGenericsForDefaultAnswers.java | 126 ++++++++++++++++++ .../stubbing/defaultanswers/ReturnsMocks.java | 39 +++--- .../defaultanswers/ReturnsSmartNulls.java | 111 ++------------- .../defaultanswers/ReturnsMocksTest.java | 30 ++++- 4 files changed, 184 insertions(+), 122 deletions(-) create mode 100644 src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java new file mode 100644 index 0000000000..951abf9bc5 --- /dev/null +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2007 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.internal.stubbing.defaultanswers; + +import java.lang.reflect.GenericArrayType; +import java.lang.reflect.Modifier; +import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; +import org.mockito.internal.util.MockUtil; +import org.mockito.internal.util.reflection.GenericMetadataSupport; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.mock.MockCreationSettings; + +class RetrieveGenericsForDefaultAnswers { + + static Object returnTypeForMockWithCorrectGenerics( + InvocationOnMock invocation, AnswerCallback answerCallback) { + Class type = invocation.getMethod().getReturnType(); + + final Type returnType = invocation.getMethod().getGenericReturnType(); + + Object defaultReturnValue = null; + + if (returnType instanceof TypeVariable) { + type = findTypeFromGeneric(invocation, (TypeVariable) returnType); + if (type != null) { + defaultReturnValue = delegateChains(type); + } + } + + if (defaultReturnValue != null) { + return defaultReturnValue; + } + + if (type != null && !type.isPrimitive() && !Modifier.isFinal(type.getModifiers())) { + return answerCallback.apply(type); + } + + return answerCallback.apply(null); + } + + /** + * Try to resolve the result value using {@link ReturnsEmptyValues} and {@link ReturnsMoreEmptyValues}. + * + * This will try to use all parent class (superclass & interfaces) to retrieve the value.. + * + * @param type the return type of the method + * @return a non-null instance if the type has been resolve. Null otherwise. + */ + private static Object delegateChains(final Class type) { + final ReturnsEmptyValues returnsEmptyValues = new ReturnsEmptyValues(); + Object result = returnsEmptyValues.returnValueFor(type); + + if (result == null) { + Class emptyValueForClass = type; + while (emptyValueForClass != null && result == null) { + final Class[] classes = emptyValueForClass.getInterfaces(); + for (Class clazz : classes) { + result = returnsEmptyValues.returnValueFor(clazz); + if (result != null) { + break; + } + } + emptyValueForClass = emptyValueForClass.getSuperclass(); + } + } + + if (result == null) { + result = new ReturnsMoreEmptyValues().returnValueFor(type); + } + + return result; + } + + /** + * Retrieve the expected type when it came from a primitive. If the type cannot be retrieve, return null. + * + * @param invocation the current invocation + * @param returnType the expected return type + * @return the type or null if not found + */ + private static Class findTypeFromGeneric(final InvocationOnMock invocation, final TypeVariable returnType) { + // Class level + final MockCreationSettings mockSettings = MockUtil.getMockHandler(invocation.getMock()).getMockSettings(); + final GenericMetadataSupport returnTypeSupport = GenericMetadataSupport + .inferFrom(mockSettings.getTypeToMock()) + .resolveGenericReturnType(invocation.getMethod()); + final Class rawType = returnTypeSupport.rawType(); + + // Method level + if (rawType == Object.class) { + return findTypeFromGenericInArguments(invocation, returnType); + } + return rawType; + } + + /** + * Find a return type using generic arguments provided by the calling method. + * + * @param invocation the current invocation + * @param returnType the expected return type + * @return the return type or null if the return type cannot be found + */ + private static Class findTypeFromGenericInArguments(final InvocationOnMock invocation, final TypeVariable returnType) { + final Type[] parameterTypes = invocation.getMethod().getGenericParameterTypes(); + for (int i = 0; i < parameterTypes.length; i++) { + Type argType = parameterTypes[i]; + if (returnType.equals(argType)) { + return invocation.getArgument(i).getClass(); + } + if (argType instanceof GenericArrayType) { + argType = ((GenericArrayType) argType).getGenericComponentType(); + if (returnType.equals(argType)) { + return invocation.getArgument(i).getClass(); + } + } + } + return null; + } + + interface AnswerCallback { + Object apply(Class type); + } +} diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java index 302e4df092..2aaff72e3a 100755 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java @@ -4,33 +4,42 @@ */ package org.mockito.internal.stubbing.defaultanswers; +import java.io.Serializable; +import org.mockito.Mockito; import org.mockito.internal.MockitoCore; import org.mockito.internal.creation.MockSettingsImpl; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import java.io.Serializable; - public class ReturnsMocks implements Answer, Serializable { private static final long serialVersionUID = -6755257986994634579L; - private final MockitoCore mockitoCore = new MockitoCore(); private final Answer delegate = new ReturnsMoreEmptyValues(); + private final MockitoCore mockitoCore = new MockitoCore(); - public Object answer(InvocationOnMock invocation) throws Throwable { - Object ret = delegate.answer(invocation); - if (ret != null) { - return ret; - } - - return returnValueFor(invocation.getMethod().getReturnType()); - } + @Override + public Object answer(final InvocationOnMock invocation) throws Throwable { + Object defaultReturnValue = delegate.answer(invocation); - Object returnValueFor(Class clazz) { - if (!mockitoCore.isTypeMockable(clazz)) { - return null; + if (defaultReturnValue != null) { + return defaultReturnValue; } - return mockitoCore.mock(clazz, new MockSettingsImpl().defaultAnswer(this)); + return RetrieveGenericsForDefaultAnswers.returnTypeForMockWithCorrectGenerics(invocation, + new RetrieveGenericsForDefaultAnswers.AnswerCallback() { + @Override + public Object apply(Class type) { + if (type == null) { + type = invocation.getMethod().getReturnType(); + + if (!mockitoCore.isTypeMockable(type)) { + return null; + } + } + + return Mockito + .mock(type, new MockSettingsImpl().defaultAnswer(ReturnsMocks.this)); + } + }); } } diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java index ff6b7942d1..6cf50c7d39 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsSmartNulls.java @@ -8,18 +8,10 @@ import static org.mockito.internal.util.ObjectMethodsGuru.isToStringMethod; import java.io.Serializable; -import java.lang.reflect.GenericArrayType; -import java.lang.reflect.Modifier; -import java.lang.reflect.Type; -import java.lang.reflect.TypeVariable; - import org.mockito.Mockito; import org.mockito.internal.debugging.LocationImpl; -import org.mockito.internal.util.MockUtil; -import org.mockito.internal.util.reflection.GenericMetadataSupport; import org.mockito.invocation.InvocationOnMock; import org.mockito.invocation.Location; -import org.mockito.mock.MockCreationSettings; import org.mockito.stubbing.Answer; /** @@ -46,108 +38,25 @@ public class ReturnsSmartNulls implements Answer, Serializable { private final Answer delegate = new ReturnsMoreEmptyValues(); + @Override public Object answer(final InvocationOnMock invocation) throws Throwable { Object defaultReturnValue = delegate.answer(invocation); - if (defaultReturnValue != null) { - return defaultReturnValue; - } - Class type = invocation.getMethod().getReturnType(); - final Type returnType = invocation.getMethod().getGenericReturnType(); - if (returnType instanceof TypeVariable) { - type = findTypeFromGeneric(invocation, (TypeVariable) returnType); - if (type != null) { - defaultReturnValue = delegateChains(type); - } - } if (defaultReturnValue != null) { return defaultReturnValue; } - if (type != null && !type.isPrimitive() && !Modifier.isFinal(type.getModifiers())) { - final Location location = new LocationImpl(); - return Mockito.mock(type, new ThrowsSmartNullPointer(invocation, location)); - } - return null; - } - - /** - * Try to resolve the result value using {@link ReturnsEmptyValues} and {@link ReturnsMoreEmptyValues}. - * - * This will try to use all parent class (superclass & interfaces) to retrieve the value.. - * - * @param type the return type of the method - * @return a non-null instance if the type has been resolve. Null otherwise. - */ - private Object delegateChains(final Class type) { - final ReturnsEmptyValues returnsEmptyValues = new ReturnsEmptyValues(); - Object result = returnsEmptyValues.returnValueFor(type); - - if (result == null) { - Class emptyValueForClass = type; - while (emptyValueForClass != null && result == null) { - final Class[] classes = emptyValueForClass.getInterfaces(); - for (Class clazz : classes) { - result = returnsEmptyValues.returnValueFor(clazz); - if (result != null) { - break; + return RetrieveGenericsForDefaultAnswers.returnTypeForMockWithCorrectGenerics(invocation, + new RetrieveGenericsForDefaultAnswers.AnswerCallback() { + @Override + public Object apply(Class type) { + if (type == null) { + return null; } - } - emptyValueForClass = emptyValueForClass.getSuperclass(); - } - } - - if (result == null) { - result = new ReturnsMoreEmptyValues().returnValueFor(type); - } - - return result; - } - - /** - * Retrieve the expected type when it came from a primitive. If the type cannot be retrieve, return null. - * - * @param invocation the current invocation - * @param returnType the expected return type - * @return the type or null if not found - */ - private Class findTypeFromGeneric(final InvocationOnMock invocation, final TypeVariable returnType) { - // Class level - final MockCreationSettings mockSettings = MockUtil.getMockHandler(invocation.getMock()).getMockSettings(); - final GenericMetadataSupport returnTypeSupport = GenericMetadataSupport - .inferFrom(mockSettings.getTypeToMock()) - .resolveGenericReturnType(invocation.getMethod()); - final Class rawType = returnTypeSupport.rawType(); - // Method level - if (rawType == Object.class) { - return findTypeFromGenericInArguments(invocation, returnType); - } - return rawType; - } - - /** - * Find a return type using generic arguments provided by the calling method. - * - * @param invocation the current invocation - * @param returnType the expected return type - * @return the return type or null if the return type cannot be found - */ - private Class findTypeFromGenericInArguments(final InvocationOnMock invocation, final TypeVariable returnType) { - final Type[] parameterTypes = invocation.getMethod().getGenericParameterTypes(); - for (int i = 0; i < parameterTypes.length; i++) { - Type argType = parameterTypes[i]; - if (returnType.equals(argType)) { - return invocation.getArgument(i).getClass(); - } - if (argType instanceof GenericArrayType) { - argType = ((GenericArrayType) argType).getGenericComponentType(); - if (returnType.equals(argType)) { - return invocation.getArgument(i).getClass(); + return Mockito.mock(type, new ThrowsSmartNullPointer(invocation, new LocationImpl())); } - } - } - return null; + }); } private static class ThrowsSmartNullPointer implements Answer { @@ -156,7 +65,7 @@ private static class ThrowsSmartNullPointer implements Answer { private final Location location; - public ThrowsSmartNullPointer(InvocationOnMock unstubbedInvocation, Location location) { + ThrowsSmartNullPointer(InvocationOnMock unstubbedInvocation, Location location) { this.unstubbedInvocation = unstubbedInvocation; this.location = location; } diff --git a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocksTest.java b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocksTest.java index 7de160c09e..ba5b822d30 100755 --- a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocksTest.java +++ b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocksTest.java @@ -6,15 +6,24 @@ import org.junit.Test; import org.mockito.internal.configuration.plugins.Plugins; +import org.mockito.internal.stubbing.defaultanswers.ReturnsGenericDeepStubsTest.WithGenerics; import org.mockito.internal.util.MockUtil; import org.mockitoutil.TestBase; import static org.junit.Assert.*; import static org.junit.Assume.assumeFalse; +import static org.mockito.Mockito.when; public class ReturnsMocksTest extends TestBase { private ReturnsMocks values = new ReturnsMocks(); + interface AllInterface { + FooInterface getInterface(); + BarClass getNormalClass(); + Baz getFinalClass(); + WithGenerics withGenerics(); + } + interface FooInterface { } @@ -25,21 +34,30 @@ final class Baz { } @Test - public void should_return_mock_value_for_interface() throws Exception { - Object interfaceMock = values.returnValueFor(FooInterface.class); + public void should_return_mock_value_for_interface() throws Throwable { + Object interfaceMock = values.answer(invocationOf(AllInterface.class, "getInterface")); assertTrue(MockUtil.isMock(interfaceMock)); } @Test - public void should_return_mock_value_for_class() throws Exception { - Object classMock = values.returnValueFor(BarClass.class); + public void should_return_mock_value_for_class() throws Throwable { + Object classMock = values.answer(invocationOf(AllInterface.class, "getNormalClass")); + assertTrue(MockUtil.isMock(classMock)); + } + + @SuppressWarnings("unchecked") + @Test + public void should_return_mock_value_for_generic_class() throws Throwable { + WithGenerics classMock = (WithGenerics) values.answer(invocationOf(AllInterface.class, "withGenerics")); assertTrue(MockUtil.isMock(classMock)); + when(classMock.execute()).thenReturn("return"); + assertEquals("return", classMock.execute()); } @Test - public void should_return_null_for_final_class_if_unsupported() throws Exception { + public void should_return_null_for_final_class_if_unsupported() throws Throwable { assumeFalse(Plugins.getMockMaker().isTypeMockable(Baz.class).mockable()); - assertNull(values.returnValueFor(Baz.class)); + assertNull(values.answer(invocationOf(AllInterface.class, "getFinalClass"))); } @Test From ab569c0ca4db1d0ffd98ea5251f08c825b5b4c19 Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Thu, 28 Feb 2019 14:30:45 +0000 Subject: [PATCH 32/42] 2.24.7 release (previous 2.24.6) + release notes updated by CI build 3921 [ci skip-release] --- doc/release-notes/official.md | 4 ++++ version.properties | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index 43b0cf01e8..fb9e03e427 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,9 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.7 + - 2019-02-28 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.6...v2.24.7) by [shipkit-org](https://github.com/shipkit-org) (1), [Tim van der Lippe](https://github.com/TimvdLippe) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.7-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.7) + - Fix handling of generics in ReturnsMocks [(#1635)](https://github.com/mockito/mockito/pull/1635) + #### 2.24.6 - 2019-02-27 - [16 commits](https://github.com/mockito/mockito/compare/v2.24.5...v2.24.6) by [Szczepan Faber](https://github.com/mockitoguy) (15), [Marcin Stachniuk](https://github.com/mstachniuk) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.6-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.6) - Make use of Shipkit v2.1.3 [(#1626)](https://github.com/mockito/mockito/pull/1626) diff --git a/version.properties b/version.properties index f59388fa57..5c5d37b0ac 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.7 +version=2.24.8 #Previous version used to generate release notes delta -previousVersion=2.24.6 +previousVersion=2.24.7 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From 1df5e49043fb032106f5c0fc0eec97ca9251241f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Pamu=C5=82a?= Date: Fri, 1 Mar 2019 22:11:23 +0900 Subject: [PATCH 33/42] Fixes #1638: Removes inaccessible links from javadocs in Mockito.java (#1639) The monkeyisland.pl domain is no longer available. List of inaccessible articles: http://monkeyisland.pl/2009/01/13/subclass-and-override-vs-partial-mocking-vs-refactoring http://monkeyisland.pl/2008/07/12/should-i-worry-about-the-unexpected http://monkeyisland.pl/2008/04/26/asking-and-telling http://monkeyisland.pl/2009/01/13/subclass-and-override-vs-partial-mocking-vs-refactoring --- src/main/java/org/mockito/Mockito.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/mockito/Mockito.java b/src/main/java/org/mockito/Mockito.java index e2a0ade374..afbe082d9e 100644 --- a/src/main/java/org/mockito/Mockito.java +++ b/src/main/java/org/mockito/Mockito.java @@ -207,7 +207,7 @@ * * //Although it is possible to verify a stubbed invocation, usually it's just redundant * //If your code cares what get(0) returns, then something else breaks (often even before verify() gets executed). - * //If your code doesn't care what get(0) returns, then it should not be stubbed. Not convinced? See here. + * //If your code doesn't care what get(0) returns, then it should not be stubbed. * verify(mockedList).get(0); * * @@ -426,8 +426,7 @@ * Some users who did a lot of classic, expect-run-verify mocking tend to use verifyNoMoreInteractions() very often, even in every test method. * verifyNoMoreInteractions() is not recommended to use in every test method. * verifyNoMoreInteractions() is a handy assertion from the interaction testing toolkit. Use it only when it's relevant. - * Abusing it leads to overspecified, less maintainable tests. You can find further reading - * here. + * Abusing it leads to overspecified, less maintainable tests. * *

* See also {@link Mockito#never()} - it is more explicit and @@ -598,8 +597,7 @@ * Before the release 1.8, Mockito spies were not real partial mocks. * The reason was we thought partial mock is a code smell. * At some point we found legitimate use cases for partial mocks - * (3rd party interfaces, interim refactoring of legacy code, the full article is - * here) + * (3rd party interfaces, interim refactoring of legacy code). *

* *


@@ -708,8 +706,7 @@
  * 

16. Real partial mocks (Since 1.8.0)

* * Finally, after many internal debates & discussions on the mailing list, partial mock support was added to Mockito. - * Previously we considered partial mocks as code smells. However, we found a legitimate use case for partial mocks - more reading: - * here + * Previously we considered partial mocks as code smells. However, we found a legitimate use case for partial mocks. *

* Before release 1.8 spy() was not producing real partial mocks and it was confusing for some users. * Read more about spying: here or in javadoc for {@link Mockito#spy(Object)} method. @@ -2070,7 +2067,6 @@ public static T spy(Class classToSpy) { * Let's say you've stubbed foo.bar(). * If your code cares what foo.bar() returns then something else breaks(often before even verify() gets executed). * If your code doesn't care what get(0) returns then it should not be stubbed. - * Not convinced? See here. * *

* See examples in javadoc for {@link Mockito} class @@ -2102,7 +2098,6 @@ public static OngoingStubbing when(T methodCall) { * Let's say you've stubbed foo.bar(). * If your code cares what foo.bar() returns then something else breaks(often before even verify() gets executed). * If your code doesn't care what get(0) returns then it should not be stubbed. - * Not convinced? See here. * *

* See examples in javadoc for {@link Mockito} class @@ -2204,8 +2199,7 @@ public static void clearInvocations(T ... mocks) { * Some users who did a lot of classic, expect-run-verify mocking tend to use verifyNoMoreInteractions() very often, even in every test method. * verifyNoMoreInteractions() is not recommended to use in every test method. * verifyNoMoreInteractions() is a handy assertion from the interaction testing toolkit. Use it only when it's relevant. - * Abusing it leads to overspecified, less maintainable tests. You can find further reading - * here. + * Abusing it leads to overspecified, less maintainable tests. *

* This method will also detect unverified invocations that occurred before the test method, * for example: in setUp(), @Before method or in constructor. From b745b5d48e9c110fa419fd007b3159ff612b8643 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Fri, 1 Mar 2019 13:13:09 +0000 Subject: [PATCH 34/42] Fix returns mocks for final classes (#1641) The guard for final mocking was incorrect. It should have passed it on to MockitoCore. Since we have the InlineMockMaker, we can actually mock, so this check was incorrect. --- .../RetrieveGenericsForDefaultAnswers.java | 10 ++++++++-- .../internal/stubbing/defaultanswers/ReturnsMocks.java | 6 ------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java index 951abf9bc5..021283dc65 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java @@ -5,9 +5,9 @@ package org.mockito.internal.stubbing.defaultanswers; import java.lang.reflect.GenericArrayType; -import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import org.mockito.internal.MockitoCore; import org.mockito.internal.util.MockUtil; import org.mockito.internal.util.reflection.GenericMetadataSupport; import org.mockito.invocation.InvocationOnMock; @@ -15,6 +15,8 @@ class RetrieveGenericsForDefaultAnswers { + private static final MockitoCore MOCKITO_CORE = new MockitoCore(); + static Object returnTypeForMockWithCorrectGenerics( InvocationOnMock invocation, AnswerCallback answerCallback) { Class type = invocation.getMethod().getReturnType(); @@ -34,7 +36,11 @@ static Object returnTypeForMockWithCorrectGenerics( return defaultReturnValue; } - if (type != null && !type.isPrimitive() && !Modifier.isFinal(type.getModifiers())) { + if (type != null) { + if (!MOCKITO_CORE.isTypeMockable(type)) { + return null; + } + return answerCallback.apply(type); } diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java index 2aaff72e3a..5d4befe60e 100755 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsMocks.java @@ -6,7 +6,6 @@ import java.io.Serializable; import org.mockito.Mockito; -import org.mockito.internal.MockitoCore; import org.mockito.internal.creation.MockSettingsImpl; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -15,7 +14,6 @@ public class ReturnsMocks implements Answer, Serializable { private static final long serialVersionUID = -6755257986994634579L; private final Answer delegate = new ReturnsMoreEmptyValues(); - private final MockitoCore mockitoCore = new MockitoCore(); @Override public Object answer(final InvocationOnMock invocation) throws Throwable { @@ -31,10 +29,6 @@ public Object answer(final InvocationOnMock invocation) throws Throwable { public Object apply(Class type) { if (type == null) { type = invocation.getMethod().getReturnType(); - - if (!mockitoCore.isTypeMockable(type)) { - return null; - } } return Mockito From 73f1d17e5b0ec1410e54a72334639da077e984f4 Mon Sep 17 00:00:00 2001 From: Fr Jeremy Krieg Date: Mon, 4 Mar 2019 20:00:48 +1030 Subject: [PATCH 35/42] VerificationCollector to handle non-matching args and other assertions (#1644) * Rename assertAtLeastOneFailure() to assertExactlyOneFailure() New method name is a more accurate description of what it does. * Change Assert.fail() to AssertJ's assertBecauseExceptionWasNotThrown() Gives better diagnositics in the event of a test failure. * Enhance VerificationCollectorImpl to collect argument-matching failures Fixes #1642. --- .../junit/VerificationCollectorImpl.java | 4 +- .../VerificationCollectorImplTest.java | 69 ++++++++++++++++--- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/mockito/internal/junit/VerificationCollectorImpl.java b/src/main/java/org/mockito/internal/junit/VerificationCollectorImpl.java index 7c19d35652..e54d47864e 100644 --- a/src/main/java/org/mockito/internal/junit/VerificationCollectorImpl.java +++ b/src/main/java/org/mockito/internal/junit/VerificationCollectorImpl.java @@ -75,7 +75,7 @@ private void append(String message) { this.numberOfFailures++; this.builder.append('\n') .append(this.numberOfFailures).append(". ") - .append(message.substring(1, message.length())); + .append(message.trim()).append('\n'); } private class VerificationWrapper implements VerificationMode { @@ -89,7 +89,7 @@ private VerificationWrapper(VerificationMode delegate) { public void verify(VerificationData data) { try { this.delegate.verify(data); - } catch (MockitoAssertionError error) { + } catch (AssertionError error) { VerificationCollectorImpl.this.append(error.getMessage()); } } diff --git a/src/test/java/org/mockitousage/junitrule/VerificationCollectorImplTest.java b/src/test/java/org/mockitousage/junitrule/VerificationCollectorImplTest.java index 8f4c420eca..9b4cc1ec1c 100644 --- a/src/test/java/org/mockitousage/junitrule/VerificationCollectorImplTest.java +++ b/src/test/java/org/mockitousage/junitrule/VerificationCollectorImplTest.java @@ -8,13 +8,14 @@ import org.junit.Test; import org.junit.runner.JUnitCore; import org.junit.runner.Result; +import org.mockito.ArgumentMatcher; import org.mockito.exceptions.base.MockitoAssertionError; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.VerificationCollector; import org.mockitousage.IMethods; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static org.mockito.Mockito.*; public class VerificationCollectorImplTest { @@ -46,14 +47,54 @@ public void should_collect_multiple_verification_failures() { IMethods methods = mock(IMethods.class); + methods.intArgumentMethod(6); + verify(methods).simpleMethod(); verify(methods).byteReturningMethod(); + verify(methods).intArgumentMethod(8); + verify(methods).longArg(8L); try { collector.collectAndReport(); - fail(); + failBecauseExceptionWasNotThrown(MockitoAssertionError.class); } catch (MockitoAssertionError error) { assertThat(error).hasMessageContaining("1. Wanted but not invoked:"); assertThat(error).hasMessageContaining("2. Wanted but not invoked:"); + assertThat(error).hasMessageContaining("3. Argument(s) are different! Wanted:"); + assertThat(error).hasMessageContaining("4. Wanted but not invoked:"); + } + } + + @Test + public void should_collect_matching_error_from_non_matching_arguments() { + VerificationCollector collector = MockitoJUnit.collector().assertLazily(); + + IMethods methods = mock(IMethods.class); + + methods.intArgumentMethod(6); + methods.longArg(8L); + methods.forShort((short)6); + + verify(methods).intArgumentMethod(8); + verify(methods).longArg(longThat(new ArgumentMatcher() { + @Override + public boolean matches(Long argument) { + throw new AssertionError("custom error message"); + } + })); + verify(methods).forShort(shortThat(new ArgumentMatcher() { + @Override + public boolean matches(Short argument) { + return false; + } + })); + + try { + collector.collectAndReport(); + failBecauseExceptionWasNotThrown(MockitoAssertionError.class); + } catch (MockitoAssertionError error) { + assertThat(error).hasMessageContaining("1. Argument(s) are different! Wanted:"); + assertThat(error).hasMessageContaining("2. custom error message"); + assertThat(error).hasMessageContaining("3. Argument(s) are different! Wanted:"); } } @@ -66,7 +107,7 @@ public void should_only_collect_failures_ignore_successful_verifications() { verify(methods, never()).simpleMethod(); verify(methods).byteReturningMethod(); - this.assertAtLeastOneFailure(collector); + this.assertExactlyOneFailure(collector); } @Test @@ -79,13 +120,13 @@ public void should_continue_collecting_after_failing_verification() { verify(methods).byteReturningMethod(); verify(methods).simpleMethod(); - this.assertAtLeastOneFailure(collector); + this.assertExactlyOneFailure(collector); } - private void assertAtLeastOneFailure(VerificationCollector collector) { + private void assertExactlyOneFailure(VerificationCollector collector) { try { collector.collectAndReport(); - fail(); + failBecauseExceptionWasNotThrown(MockitoAssertionError.class); } catch (MockitoAssertionError error) { assertThat(error).hasMessageContaining("1. Wanted but not invoked:"); assertThat(error.getMessage()).doesNotContain("2."); @@ -97,8 +138,11 @@ public void should_invoke_collector_rule_after_test() { JUnitCore runner = new JUnitCore(); Result result = runner.run(VerificationCollectorRuleInner.class); - assertThat(result.getFailureCount()).isEqualTo(1); - assertThat(result.getFailures().get(0).getMessage()).contains("1. Wanted but not invoked:"); + assertThat(result.getFailureCount()).as("failureCount").isEqualTo(2); + assertThat(result.getFailures().get(0).getMessage()).as("failure1").contains("1. Wanted but not invoked:"); + assertThat(result.getFailures().get(1).getMessage()).as("failure2") + .contains("1. Argument(s) are different! Wanted:") + .contains("2. Wanted but not invoked:"); } // This class is picked up when running a test suite using an IDE. It fails on purpose. @@ -121,5 +165,14 @@ public void should_not_fail() { verify(methods).simpleMethod(); } + + @Test + public void should_fail_with_args() { + IMethods methods = mock(IMethods.class); + methods.intArgumentMethod(8); + + verify(methods).intArgumentMethod(9); + verify(methods).byteReturningMethod(); + } } } From 3f84cbfc1434b17fb84c1ae7579c3b03749a2454 Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Mon, 4 Mar 2019 15:45:14 +0100 Subject: [PATCH 36/42] Reintroduces the guard that returns null instead of a mock of Object While the generics metadata extractor has been improved, not every possible scenarios are covered even Javac in version 8 do not follow through, in order to avoid a stealth return of the bugs spot in #357 this change improves the guard by making sure extracted generic data does not have extra-interfaces as well. --- src/main/java/org/mockito/Mockito.java | 6 +- .../defaultanswers/ReturnsDeepStubs.java | 44 +++++++---- .../reflection/GenericMetadataSupport.java | 10 ++- .../ReturnsGenericDeepStubsTest.java | 73 +++++++++++++++++-- .../DeepStubsSerializableTest.java | 4 +- 5 files changed, 108 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/mockito/Mockito.java b/src/main/java/org/mockito/Mockito.java index e2a0ade374..0d85e672c8 100644 --- a/src/main/java/org/mockito/Mockito.java +++ b/src/main/java/org/mockito/Mockito.java @@ -1685,6 +1685,7 @@ public class Mockito extends ArgumentMatchers { /** * Optional Answer to be used with {@link Mockito#mock(Class, Answer)} + * *

* {@link Answer} can be used to define the return values of unstubbed invocations. *

@@ -1716,8 +1717,11 @@ public class Mockito extends ArgumentMatchers { * * *

- * Note: Stubbing partial mocks using when(mock.getSomething()).thenReturn(fakeValue) + * Note 1: Stubbing partial mocks using when(mock.getSomething()).thenReturn(fakeValue) * syntax will call the real method. For partial mock it's recommended to use doReturn syntax. + *

+ * Note 2: If the mock is serialized then deserialized, then this answer will not be able to understand + * generics metadata. */ public static final Answer CALLS_REAL_METHODS = Answers.CALLS_REAL_METHODS; diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java index cc963679b2..356b2e629f 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java @@ -25,7 +25,7 @@ /** * Returning deep stub implementation. * - * Will return previously created mock if the invocation matches. + *

Will return previously created mock if the invocation matches. * *

Supports nested generic information, with this answer you can write code like this : * @@ -37,6 +37,8 @@ * *

* + *

However this answer does not support generics information when the mock has been deserialized. + * * @see org.mockito.Mockito#RETURNS_DEEP_STUBS * @see org.mockito.Answers#RETURNS_DEEP_STUBS */ @@ -46,21 +48,21 @@ public class ReturnsDeepStubs implements Answer, Serializable { public Object answer(InvocationOnMock invocation) throws Throwable { GenericMetadataSupport returnTypeGenericMetadata = - actualParameterizedType(invocation.getMock()).resolveGenericReturnType(invocation.getMethod()); + actualParameterizedType(invocation.getMock()).resolveGenericReturnType(invocation.getMethod()); Class rawType = returnTypeGenericMetadata.rawType(); if (!mockitoCore().isTypeMockable(rawType)) { return delegate().returnValueFor(rawType); } - // When dealing with erasured generics, we only receive the Object type as rawType. At this + // When dealing with erased generics, we only receive the Object type as rawType. At this // point, there is nothing to salvage for Mockito. Instead of trying to be smart and generate // a mock that would potentially match the return signature, instead return `null`. This // is valid per the CheckCast JVM instruction and is better than causing a ClassCastException // on runtime. -// if (rawType.equals(Object.class)) { -// return null; -// } + if (rawType.equals(Object.class) && !returnTypeGenericMetadata.hasRawExtraInterfaces()) { + return null; + } return deepStub(invocation, returnTypeGenericMetadata); } @@ -78,9 +80,9 @@ private Object deepStub(InvocationOnMock invocation, GenericMetadataSupport retu // record deep stub answer StubbedInvocationMatcher stubbing = recordDeepStubAnswer( - newDeepStubMock(returnTypeGenericMetadata, invocation.getMock()), - container - ); + newDeepStubMock(returnTypeGenericMetadata, invocation.getMock()), + container + ); // deep stubbing creates a stubbing and immediately uses it // so the stubbing is actually used by the same invocation @@ -97,24 +99,24 @@ private Object deepStub(InvocationOnMock invocation, GenericMetadataSupport retu * {@link ReturnsDeepStubs} answer in which we will store the returned type generic metadata. * * @param returnTypeGenericMetadata The metadata to use to create the new mock. - * @param parentMock The parent of the current deep stub mock. + * @param parentMock The parent of the current deep stub mock. * @return The mock */ private Object newDeepStubMock(GenericMetadataSupport returnTypeGenericMetadata, Object parentMock) { MockCreationSettings parentMockSettings = MockUtil.getMockSettings(parentMock); return mockitoCore().mock( - returnTypeGenericMetadata.rawType(), - withSettingsUsing(returnTypeGenericMetadata, parentMockSettings) - ); + returnTypeGenericMetadata.rawType(), + withSettingsUsing(returnTypeGenericMetadata, parentMockSettings) + ); } private MockSettings withSettingsUsing(GenericMetadataSupport returnTypeGenericMetadata, MockCreationSettings parentMockSettings) { MockSettings mockSettings = returnTypeGenericMetadata.hasRawExtraInterfaces() ? - withSettings().extraInterfaces(returnTypeGenericMetadata.rawExtraInterfaces()) - : withSettings(); + withSettings().extraInterfaces(returnTypeGenericMetadata.rawExtraInterfaces()) + : withSettings(); return propagateSerializationSettings(mockSettings, parentMockSettings) - .defaultAnswer(returnsDeepStubsAnswerUsing(returnTypeGenericMetadata)); + .defaultAnswer(returnsDeepStubsAnswerUsing(returnTypeGenericMetadata)); } private MockSettings propagateSerializationSettings(MockSettings mockSettings, MockCreationSettings parentMockSettings) { @@ -148,6 +150,15 @@ public ReturnsDeepStubsSerializationFallback(GenericMetadataSupport returnTypeGe protected GenericMetadataSupport actualParameterizedType(Object mock) { return returnTypeGenericMetadata; } + + /** + * Generics support and serialization with deep stubs don't work together. + *

+ * The issue is that GenericMetadataSupport is not serializable because + * the type elements inferred via reflection are not serializable. Supporting + * serialization would require to replace all types coming from the Java reflection + * with our own and still managing type equality with the JDK ones. + */ private Object writeReplace() throws IOException { return Mockito.RETURNS_DEEP_STUBS; } @@ -161,6 +172,7 @@ private static class DeeplyStubbedAnswer implements Answer, Serializable DeeplyStubbedAnswer(Object mock) { this.mock = mock; } + public Object answer(InvocationOnMock invocation) throws Throwable { return mock; } diff --git a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java index 3c519c1a51..80cbf65be3 100644 --- a/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java +++ b/src/main/java/org/mockito/internal/util/reflection/GenericMetadataSupport.java @@ -418,6 +418,7 @@ private static class TypeVariableReturnType extends GenericMetadataSupport { private final TypeVariable typeVariable; private final TypeVariable[] typeParameters; private Class rawType; + private List extraInterfaces; public TypeVariableReturnType(GenericMetadataSupport source, TypeVariable[] typeParameters, TypeVariable typeVariable) { this.typeParameters = typeParameters; @@ -450,15 +451,18 @@ public Class rawType() { @Override public List extraInterfaces() { + if (extraInterfaces != null) { + return extraInterfaces; + } Type type = extractActualBoundedTypeOf(typeVariable); if (type instanceof BoundedType) { - return Arrays.asList(((BoundedType) type).interfaceBounds()); + return extraInterfaces = Arrays.asList(((BoundedType) type).interfaceBounds()); } if (type instanceof ParameterizedType) { - return Collections.singletonList(type); + return extraInterfaces = Collections.singletonList(type); } if (type instanceof Class) { - return Collections.emptyList(); + return extraInterfaces = Collections.emptyList(); } throw new MockitoException("Cannot extract extra-interfaces from '" + typeVariable + "' : '" + type + "'"); } diff --git a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java index 40b9ecb575..b051e124a8 100644 --- a/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java +++ b/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsGenericDeepStubsTest.java @@ -5,11 +5,14 @@ package org.mockito.internal.stubbing.defaultanswers; import org.junit.Test; +import org.mockitousage.examples.use.Article; +import java.io.Closeable; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; @@ -18,18 +21,27 @@ @SuppressWarnings("unused") public class ReturnsGenericDeepStubsTest { - interface ListOfInteger extends List {} + interface ListOfInteger extends List { + } - interface AnotherListOfInteger extends ListOfInteger {} + interface AnotherListOfInteger extends ListOfInteger { + } interface GenericsNest & Cloneable> extends Map> { Set remove(Object key); // override with fixed ParameterizedType + List returningWildcard(); + Map returningNonMockableNestedGeneric(); + K returningK(); + List paramTypeWithTypeParams(); + T twoTypeParams(S s); + O typeVarWithTypeParams(); + Number returnsNormalType(); } @@ -38,7 +50,7 @@ public void generic_deep_mock_frenzy__look_at_these_chained_calls() { GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Set>> entries = mock.entrySet(); - Iterator>> entriesIterator = mock.entrySet().iterator(); + Iterator>> entriesIterator = mock.entrySet().iterator(); Map.Entry> nextEntry = mock.entrySet().iterator().next(); Cloneable cloneableKey = mock.entrySet().iterator().next().getKey(); @@ -54,9 +66,9 @@ public void can_create_mock_from_multiple_type_variable_bounds_when_return_type_ GenericsNest mock = mock(GenericsNest.class, RETURNS_DEEP_STUBS); Cloneable cloneable_bound_that_is_declared_on_typevar_K_in_the_class_which_is_referenced_by_typevar_O_declared_on_the_method = - mock.paramTypeWithTypeParams().get(0); + mock.paramTypeWithTypeParams().get(0); Comparable comparable_bound_that_is_declared_on_typevar_K_in_the_class_which_is_referenced_by_typevar_O_declared_on_the_method = - mock.paramTypeWithTypeParams().get(0); + mock.paramTypeWithTypeParams().get(0); } @Test @@ -109,7 +121,7 @@ public void as_expected_fail_with_a_CCE_on_call_site_when_erasure_takes_place_fo // following assignment needed to create a ClassCastException on the call site (i.e. : here) StringBuilder stringBuilder_assignment_that_should_throw_a_CCE = - mock.twoTypeParams(new StringBuilder()).append(2).append(3); + mock.twoTypeParams(new StringBuilder()).append(2).append(3); } class WithGenerics { @@ -117,7 +129,9 @@ T execute() { throw new IllegalArgumentException(); } } - class SubClass extends WithGenerics {} + + class SubClass extends WithGenerics { + } class UserOfSubClass { SubClass generate() { @@ -133,4 +147,49 @@ public void can_handle_deep_stubs_with_generics_at_end_of_deep_invocation() { assertThat(mock.generate().execute()).isEqualTo("sub"); } + + public interface TopInterface { + T generic(); + } + + public interface MiddleInterface extends TopInterface { + } + + public class OwningClassWithDeclaredUpperBounds & Callable
& Closeable> { + public abstract class AbstractInner implements MiddleInterface { + } + } + + @Test + public void cannot_handle_deep_stubs_with_generics_declared_upper_bounds_at_end_of_deep_invocation() throws Exception { + OwningClassWithDeclaredUpperBounds.AbstractInner mock = + mock(OwningClassWithDeclaredUpperBounds.AbstractInner.class, RETURNS_DEEP_STUBS); + + // It seems that while the syntax used on OwningClassWithDeclaredUpperBounds.AbstractInner + // appear to be legal, the javac compiler does not follow through + // hence we need casting, this may also explain why GenericMetadataSupport has trouble to + // extract matching data as well. + + assertThat(mock.generic()) + .describedAs("mock should implement first bound : 'Iterable'") + .isInstanceOf(Iterable.class); + assertThat(((Iterable
) mock.generic()).iterator()) + .describedAs("Iterable returns Iterator").isInstanceOf(Iterator.class); + assertThat(((Iterable
) mock.generic()).iterator().next()) + .describedAs("Cannot yet extract Type argument 'Article' so return null instead of a mock " + + "of type Object (which would raise CCE on the call-site)") + .isNull(); + + assertThat(mock.generic()) + .describedAs("mock should implement second interface bound : 'Callable'") + .isInstanceOf(Callable.class); + assertThat(((Callable
) mock.generic()).call()) + .describedAs("Cannot yet extract Type argument 'Article' so return null instead of a mock " + + "of type Object (which would raise CCE on the call-site)") + .isNull(); + + assertThat(mock.generic()) + .describedAs("mock should implement third interface bound : 'Closeable'") + .isInstanceOf(Closeable.class); + } } diff --git a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java index e3891c6047..c26e8d8b57 100644 --- a/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java +++ b/src/test/java/org/mockitousage/serialization/DeepStubsSerializableTest.java @@ -62,8 +62,8 @@ public void should_discard_generics_metadata_when_serialized_then_disabling_deep // then revert to the default RETURNS_DEEP_STUBS and the code will raise a ClassCastException when(deserialized_deep_stub.iterator().next().get(42)).thenReturn("no"); fail("Expected an exception to be thrown as deep stubs and serialization does not play well together"); - } catch (ClassCastException e) { - assertThat(e).hasMessageContaining("java.util.List"); + } catch (NullPointerException e) { + assertThat(e).hasMessage(null); } } From f377d0e27ffe189554367c53dd5f00a09452d2b6 Mon Sep 17 00:00:00 2001 From: epeee Date: Mon, 4 Mar 2019 18:32:03 +0100 Subject: [PATCH 37/42] Update shipkit plugin (v2.1.6) and increase the version since we already have a tag for 2.24.8 --- build.gradle | 2 +- version.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index c8142db658..4285ff16ff 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ buildscript { classpath 'net.ltgt.gradle:gradle-errorprone-plugin:0.6' //Using buildscript.classpath so that we can resolve shipkit from maven local, during local testing - classpath 'org.shipkit:shipkit:2.1.3' + classpath 'org.shipkit:shipkit:2.1.6' } } diff --git a/version.properties b/version.properties index 5c5d37b0ac..19b0042628 100644 --- a/version.properties +++ b/version.properties @@ -1,5 +1,5 @@ #Currently building Mockito version -version=2.24.8 +version=2.24.9 #Previous version used to generate release notes delta previousVersion=2.24.7 From 0cf38225c58fcf7dfe49780c987516ff53048c6e Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Mon, 4 Mar 2019 18:01:24 +0000 Subject: [PATCH 38/42] 2.24.9 release (previous 2.24.7) + release notes updated by CI build 3944 [ci skip-release] --- doc/release-notes/official.md | 14 ++++++++++++++ version.properties | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index fb9e03e427..e627ceeab5 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,19 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.9 + - 2019-03-04 - [12 commits](https://github.com/mockito/mockito/compare/v2.24.7...v2.24.9) by 6 authors - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.9-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.9) + - Commits: [Brice Dutheil](https://github.com/bric3) (5), [Tim van der Lippe](https://github.com/TimvdLippe) (3), [epeee](https://github.com/epeee) (1), [Fr Jeremy Krieg](https://github.com/kriegfrj) (1), [Paweł Pamuła](https://github.com/PawelPamula) (1), shipkit-org (1) + - [Java 9 support] ClassCastExceptions with JDK9 javac [(#357)](https://github.com/mockito/mockito/issues/357) + - [Bugfixes] RETURNS_DEEP_STUBS causes "Raw extraction not supported for : 'null'" in some cases [(#1621)](https://github.com/mockito/mockito/issues/1621) + - Update shipkit plugin (v2.1.6) [(#1647)](https://github.com/mockito/mockito/pull/1647) + - VerificationCollector to handle non-matching args and other assertions [(#1644)](https://github.com/mockito/mockito/pull/1644) + - VerificationCollector doesn't work for invocations with non-matching args [(#1642)](https://github.com/mockito/mockito/issues/1642) + - Fix returns mocks for final classes [(#1641)](https://github.com/mockito/mockito/pull/1641) + - Removes inaccessible links from javadocs in Mockito.java [(#1639)](https://github.com/mockito/mockito/pull/1639) + - Mockito.java contains inaccessible links to articles. [(#1638)](https://github.com/mockito/mockito/issues/1638) + - Handle terminal type var with bounds [(#1624)](https://github.com/mockito/mockito/pull/1624) + - Return null instead of causing a CCE [(#1612)](https://github.com/mockito/mockito/pull/1612) + #### 2.24.7 - 2019-02-28 - [2 commits](https://github.com/mockito/mockito/compare/v2.24.6...v2.24.7) by [shipkit-org](https://github.com/shipkit-org) (1), [Tim van der Lippe](https://github.com/TimvdLippe) (1) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.7-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.7) - Fix handling of generics in ReturnsMocks [(#1635)](https://github.com/mockito/mockito/pull/1635) diff --git a/version.properties b/version.properties index 19b0042628..3fa30ed621 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.9 +version=2.24.10 #Previous version used to generate release notes delta -previousVersion=2.24.7 +previousVersion=2.24.9 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0 From b5e9400b01f8bfbda436c79ce5f857b3dc613657 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 4 Mar 2019 18:22:03 +0000 Subject: [PATCH 39/42] Prevent NPE in findTypeFromGenericInArguments There was only a single test failing on this. I think the issue was RETURNS_SMART_NULLS in combination with an ArgumentCaptor, but couldn't figure that part out. At least this fixes prevented the NPE. --- .../defaultanswers/RetrieveGenericsForDefaultAnswers.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java index 021283dc65..09fb182b9d 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java @@ -114,7 +114,13 @@ private static Class findTypeFromGenericInArguments(final InvocationOnMock in for (int i = 0; i < parameterTypes.length; i++) { Type argType = parameterTypes[i]; if (returnType.equals(argType)) { - return invocation.getArgument(i).getClass(); + Object argument = invocation.getArgument(i); + + if (argument == null) { + return null; + } + + return argument.getClass(); } if (argType instanceof GenericArrayType) { argType = ((GenericArrayType) argType).getGenericComponentType(); From 2d21a5abc744084646d9939d5ed9d1825695af9a Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 5 Mar 2019 14:49:36 +0000 Subject: [PATCH 40/42] Add regression test --- .../mockitousage/stubbing/SmartNullsStubbingTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/java/org/mockitousage/stubbing/SmartNullsStubbingTest.java b/src/test/java/org/mockitousage/stubbing/SmartNullsStubbingTest.java index 9b3dc868d1..8e27f73100 100644 --- a/src/test/java/org/mockitousage/stubbing/SmartNullsStubbingTest.java +++ b/src/test/java/org/mockitousage/stubbing/SmartNullsStubbingTest.java @@ -42,6 +42,15 @@ public void shouldSmartNPEPointToUnstubbedCall() throws Exception { } } + @Test + public void should_not_throw_NPE_when_verifying_with_returns_smart_nulls() { + Foo mock = mock(Foo.class, RETURNS_SMART_NULLS); + + when(mock.returnsFromArg(null)).thenReturn("Does not fail."); + + assertThat((Object) mock.returnsFromArg(null)).isEqualTo("Does not fail."); + } + interface Bar { void boo(); } @@ -59,6 +68,8 @@ Bar getBarWithParams(int x, String y) { return null; } + T returnsFromArg(T arg) { return arg; } + void boo() {} } From 2894895746950be644af81055bf7543ff4e9f832 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 5 Mar 2019 14:52:30 +0000 Subject: [PATCH 41/42] Fix checkstyle warnings --- .../defaultanswers/RetrieveGenericsForDefaultAnswers.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java index 09fb182b9d..979c8f7813 100644 --- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java +++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/RetrieveGenericsForDefaultAnswers.java @@ -115,11 +115,11 @@ private static Class findTypeFromGenericInArguments(final InvocationOnMock in Type argType = parameterTypes[i]; if (returnType.equals(argType)) { Object argument = invocation.getArgument(i); - + if (argument == null) { return null; } - + return argument.getClass(); } if (argType instanceof GenericArrayType) { From af2d33be7fbfecad172cef4da2127b8d9a6fbe31 Mon Sep 17 00:00:00 2001 From: shipkit-org Date: Tue, 5 Mar 2019 15:49:22 +0000 Subject: [PATCH 42/42] 2.24.10 release (previous 2.24.9) + release notes updated by CI build 3953 [ci skip-release] --- doc/release-notes/official.md | 4 ++++ version.properties | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/official.md b/doc/release-notes/official.md index e627ceeab5..752360b3c8 100644 --- a/doc/release-notes/official.md +++ b/doc/release-notes/official.md @@ -1,5 +1,9 @@ *Release notes were automatically generated by [Shipkit](http://shipkit.org/)* +#### 2.24.10 + - 2019-03-05 - [4 commits](https://github.com/mockito/mockito/compare/v2.24.9...v2.24.10) by [Tim van der Lippe](https://github.com/TimvdLippe) - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.10-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.10) + - Prevent NPE in findTypeFromGenericInArguments [(#1648)](https://github.com/mockito/mockito/pull/1648) + #### 2.24.9 - 2019-03-04 - [12 commits](https://github.com/mockito/mockito/compare/v2.24.7...v2.24.9) by 6 authors - published to [![Bintray](https://img.shields.io/badge/Bintray-2.24.9-green.svg)](https://bintray.com/mockito/maven/mockito-development/2.24.9) - Commits: [Brice Dutheil](https://github.com/bric3) (5), [Tim van der Lippe](https://github.com/TimvdLippe) (3), [epeee](https://github.com/epeee) (1), [Fr Jeremy Krieg](https://github.com/kriegfrj) (1), [Paweł Pamuła](https://github.com/PawelPamula) (1), shipkit-org (1) diff --git a/version.properties b/version.properties index 3fa30ed621..61a6e09815 100644 --- a/version.properties +++ b/version.properties @@ -1,8 +1,8 @@ #Currently building Mockito version -version=2.24.10 +version=2.24.11 #Previous version used to generate release notes delta -previousVersion=2.24.9 +previousVersion=2.24.10 #Not published currently, see https://github.com/mockito/mockito/issues/962 mockito.testng.version=1.0