Skip to content

Commit

Permalink
Switch the default mockmaker to the inline mockmaker on JDK 17+. Fixes
Browse files Browse the repository at this point in the history
…#2589

Signed-off-by: Andriy Redko <drreta@gmail.com>
  • Loading branch information
reta committed Jan 2, 2023
1 parent ffb294c commit 1b35460
Show file tree
Hide file tree
Showing 13 changed files with 117 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ public class DefaultMockitoPlugins implements MockitoPlugins {
static final String SUBCLASS_ALIAS = MockMakers.SUBCLASS;
public static final Set<String> MOCK_MAKER_ALIASES = new HashSet<>();
static final String MODULE_ALIAS = "member-accessor-module";
static final String REFLECTION_ALIAS = "member-accessor-reflection";

static {
// Keep the mapping: plugin interface name -> plugin implementation class name
DEFAULT_PLUGINS.put(PluginSwitch.class.getName(), DefaultPluginSwitch.class.getName());
DEFAULT_PLUGINS.put(
MockMaker.class.getName(),
"org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker");
"org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker");
DEFAULT_PLUGINS.put(
StackTraceCleanerProvider.class.getName(),
"org.mockito.internal.exceptions.stacktrace.DefaultStackTraceCleanerProvider");
Expand All @@ -53,9 +54,11 @@ public class DefaultMockitoPlugins implements MockitoPlugins {
MockitoLogger.class.getName(), "org.mockito.internal.util.ConsoleMockitoLogger");
DEFAULT_PLUGINS.put(
MemberAccessor.class.getName(),
"org.mockito.internal.util.reflection.ReflectionMemberAccessor");
"org.mockito.internal.util.reflection.ModuleMemberAccessor");
DEFAULT_PLUGINS.put(
MODULE_ALIAS, "org.mockito.internal.util.reflection.ModuleMemberAccessor");
DEFAULT_PLUGINS.put(
REFLECTION_ALIAS, "org.mockito.internal.util.reflection.ReflectionMemberAccessor");
DEFAULT_PLUGINS.put(
DoNotMockEnforcer.class.getName(),
"org.mockito.internal.configuration.DefaultDoNotMockEnforcer");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public <T> Class<? extends T> mockClass(MockFeatures<T> features) {
boolean subclassingRequired =
!features.interfaces.isEmpty()
|| features.serializableMode != SerializableMode.NONE
|| features.stripAnnotations
|| Modifier.isAbstract(features.mockedType.getModifiers());

checkSupportedCombination(subclassingRequired, features);
Expand Down Expand Up @@ -416,6 +417,7 @@ public synchronized void clearAllCaches() {
}
mocked.clear();
flatMocked.clear();
subclassEngine.clearAllCaches();
try {
instrumentation.retransformClasses(types.toArray(new Class<?>[0]));
} catch (UnmodifiableClassException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,16 @@ public Object newInstance(
onConstruction.invoke(
() -> {
try {
return DISPATCHER.invokeWithArguments(handle, arguments);
// Use handle.asFixedArity() to handle varargs bindings properly
// (as by default Java will create method handle with
// asVarargsCollector), fe:
//
// private static class Varargs {
// Varargs(String whatever, Observer... observers) {
// }
// }
return DISPATCHER.invokeWithArguments(
handle.asFixedArity(), arguments);
} catch (Throwable throwable) {
thrown.set(true);
return throwable;
Expand Down Expand Up @@ -372,17 +381,23 @@ private static void assureArguments(
throw new IllegalArgumentException("Cannot access " + target + " on " + owner);
}
}
if (types.length != values.length) {

Object[] args = values;
if (args == null) {
args = new Object[0];
}

if (types.length != args.length) {
throw new IllegalArgumentException(
"Incorrect number of arguments for "
+ target
+ ": expected "
+ types.length
+ " but recevied "
+ values.length);
+ args.length);
}
for (int index = 0; index < values.length; index++) {
if (values[index] == null) {
for (int index = 0; index < args.length; index++) {
if (args[index] == null) {
if (types[index].isPrimitive()) {
throw new IllegalArgumentException(
"Cannot assign null to primitive type "
Expand All @@ -394,10 +409,10 @@ private static void assureArguments(
}
} else {
Class<?> resolved = WRAPPERS.getOrDefault(types[index], types[index]);
if (!resolved.isAssignableFrom(values[index].getClass())) {
if (!resolved.isAssignableFrom(args[index].getClass())) {
throw new IllegalArgumentException(
"Cannot assign value of type "
+ values[index].getClass()
+ args[index].getClass()
+ " to "
+ resolved
+ " for "
Expand Down
23 changes: 23 additions & 0 deletions src/test/java/org/mockito/MockitoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,21 @@
import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.not;
import static org.mockito.Mockito.times;
import static org.mockito.internal.progress.ThreadSafeMockingProgress.mockingProgress;

import java.util.List;

import org.junit.Assume;
import org.junit.Test;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.exceptions.misusing.NotAMockException;
import org.mockito.exceptions.misusing.NullInsteadOfMockException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.creation.MockSettingsImpl;
import org.mockito.plugins.InlineMockMaker;

@SuppressWarnings("unchecked")
public class MockitoTest {
Expand Down Expand Up @@ -99,6 +104,8 @@ public void shouldValidateMockWhenCreatingInOrderObject() {
@SuppressWarnings({"CheckReturnValue", "MockitoUsage"})
@Test
public void shouldGiveExplanationOnStaticMockingWithoutInlineMockMaker() {
Assume.assumeThat(Plugins.getMockMaker(), not(instanceOf(InlineMockMaker.class)));

assertThatThrownBy(
() -> {
Mockito.mockStatic(Object.class);
Expand All @@ -114,6 +121,8 @@ public void shouldGiveExplanationOnStaticMockingWithoutInlineMockMaker() {
@SuppressWarnings({"CheckReturnValue", "MockitoUsage"})
@Test
public void shouldGiveExplanationOnConstructionMockingWithoutInlineMockMaker() {
Assume.assumeThat(Plugins.getMockMaker(), not(instanceOf(InlineMockMaker.class)));

assertThatThrownBy(
() -> {
Mockito.mockConstruction(Object.class);
Expand All @@ -126,6 +135,20 @@ public void shouldGiveExplanationOnConstructionMockingWithoutInlineMockMaker() {
"Note that Mockito's inline mock maker is not supported on Android.");
}

@SuppressWarnings({"CheckReturnValue", "MockitoUsage"})
@Test
public void shouldGiveExplanationOnConstructionMockingWithInlineMockMaker() {
Assume.assumeThat(Plugins.getMockMaker(), instanceOf(InlineMockMaker.class));

assertThatThrownBy(
() -> {
Mockito.mockConstruction(Object.class);
})
.isInstanceOf(MockitoException.class)
.hasMessageContainingAll(
"It is not possible to mock construction of the Object class to avoid inference with default object constructor chains");
}

@Test
public void shouldStartingMockSettingsContainDefaultBehavior() {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import static org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.SUBCLASS_ALIAS;

import org.junit.Test;
import org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker;
import org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker;
import org.mockito.internal.util.ConsoleMockitoLogger;
import org.mockito.plugins.InstantiatorProvider2;
Expand All @@ -35,7 +34,8 @@ public void provides_plugins() throws Exception {
"org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker",
DefaultMockitoPlugins.getDefaultPluginClass(SUBCLASS_ALIAS));
assertEquals(
ByteBuddyMockMaker.class, plugins.getDefaultPlugin(MockMaker.class).getClass());
InlineByteBuddyMockMaker.class,
plugins.getDefaultPlugin(MockMaker.class).getClass());
assertNotNull(plugins.getDefaultPlugin(InstantiatorProvider2.class));
assertEquals(
ConsoleMockitoLogger.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
*/
package org.mockito.internal.runners;

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

import org.junit.Assume;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;
Expand All @@ -18,9 +21,11 @@
import org.junit.runner.notification.RunNotifier;
import org.junit.runners.model.Statement;
import org.mockito.Mock;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.junit.MockitoTestListener;
import org.mockito.internal.junit.TestFinishedEvent;
import org.mockito.internal.util.Supplier;
import org.mockito.plugins.InlineMockMaker;

public class DefaultInternalRunnerTest {

Expand All @@ -42,6 +47,9 @@ public void does_not_fail_when_tests_succeeds() throws Exception {

@Test
public void does_not_fail_second_test_when_first_test_fail() throws Exception {
// The TestFailOnInitialization is initialized properly by inline mock maker
Assume.assumeThat(Plugins.getMockMaker(), not(instanceOf(InlineMockMaker.class)));

new DefaultInternalRunner(TestFailOnInitialization.class, supplier)
.run(newNotifier(runListener));

Expand Down
24 changes: 23 additions & 1 deletion src/test/java/org/mockitousage/annotation/SpyAnnotationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
Expand All @@ -22,6 +24,7 @@
import java.util.LinkedList;
import java.util.List;

import org.junit.Assume;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Expand All @@ -30,6 +33,8 @@
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.plugins.InlineMockMaker;
import org.mockitoutil.TestBase;

@SuppressWarnings("unused")
Expand Down Expand Up @@ -188,8 +193,21 @@ class WithSpy {
}
}

@Test
public void should_spy_private_inner() throws Exception {
Assume.assumeThat(Plugins.getMockMaker(), instanceOf(InlineMockMaker.class));

WithInnerPrivate inner = new WithInnerPrivate();
MockitoAnnotations.openMocks(inner);

when(inner.spy_field.lenght()).thenReturn(10);
assertEquals(10, inner.spy_field.lenght());
}

@Test
public void should_report_private_inner_not_supported() throws Exception {
Assume.assumeThat(Plugins.getMockMaker(), not(instanceOf(InlineMockMaker.class)));

try {
MockitoAnnotations.openMocks(new WithInnerPrivate());
fail();
Expand Down Expand Up @@ -287,7 +305,11 @@ private class InnerPrivateConcrete extends InnerPrivateAbstract {}
static class WithInnerPrivate {
@Spy private InnerPrivate spy_field;

private class InnerPrivate {}
private class InnerPrivate {
int lenght() {
return 0;
}
}

private class InnerPrivateSub extends InnerPrivate {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ private static SimplePerRealmReloadingClassLoader.ReloadClassPredicate reloadMoc
return new SimplePerRealmReloadingClassLoader.ReloadClassPredicate() {
public boolean acceptReloadOf(String qualifiedName) {
return (!qualifiedName.contains("net.bytebuddy")
&& qualifiedName.contains("org.mockito"));
&& qualifiedName.contains("org.mockito")
&& !qualifiedName.contains(
"org.mockito.internal.creation.bytebuddy.inject"));
}
};
}
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/org/mockitousage/misuse/InvalidUsageTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,25 @@
*/
package org.mockitousage.misuse;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.not;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import org.junit.After;
import org.junit.Assume;
import org.junit.Test;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.exceptions.misusing.MissingMethodInvocationException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.plugins.InlineMockMaker;
import org.mockitousage.IMethods;
import org.mockitoutil.TestBase;

Expand Down Expand Up @@ -155,6 +161,8 @@ final class FinalClass {}

@Test
public void shouldNotAllowMockingFinalClassesIfDisabled() {
Assume.assumeThat(Plugins.getMockMaker(), not(instanceOf(InlineMockMaker.class)));

assertThatThrownBy(
() -> {
mock(FinalClass.class);
Expand All @@ -166,6 +174,12 @@ public void shouldNotAllowMockingFinalClassesIfDisabled() {
" - final class");
}

@Test
public void shouldAllowMockingFinalClassesIfEnabled() {
Assume.assumeThat(Plugins.getMockMaker(), instanceOf(InlineMockMaker.class));
assertThat(mock(FinalClass.class)).isInstanceOf(FinalClass.class);
}

@SuppressWarnings({"CheckReturnValue", "MockitoUsage"})
@Test
public void shouldNotAllowMockingPrimitives() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
member-accessor-reflection
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,9 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
protected URL findResource(String moduleName, String name) {
return Mockito.class.getResource("/" + name);
}

@Override
public InputStream getResourceAsStream(String name) {
return Mockito.class.getResourceAsStream("/" + name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,25 @@

import static org.junit.Assert.assertEquals;

import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockMakers;
import org.mockito.Mockito;
import org.mockitoutil.TestBase;
import org.mockito.MockitoAnnotations;

public class ProgrammaticMockMakerAnnotationTest extends TestBase {
public class ProgrammaticMockMakerAnnotationTest {
@Mock(mockMaker = MockMakers.INLINE)
ClassWithFinalMethod inlineMock;

@Mock(mockMaker = MockMakers.SUBCLASS)
ClassWithFinalMethod subclassMock;

@Before
public void init() {
MockitoAnnotations.openMocks(this);
}

@Test
public void test_mock_uses_given_mock_maker() {
Mockito.when(inlineMock.finalMethodCallingNonFinal()).thenReturn("MOCKED");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
member-accessor-reflection

0 comments on commit 1b35460

Please sign in to comment.