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
…mockito#2589

Signed-off-by: Andriy Redko <drreta@gmail.com>
  • Loading branch information
reta committed Jan 5, 2023
1 parent e910bcc commit 733b5c8
Show file tree
Hide file tree
Showing 19 changed files with 201 additions and 19 deletions.
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
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
Expand Up @@ -24,13 +24,20 @@ public boolean isIn(StackFrameMetadata e) {
private boolean isIn(String className) {
if (isFromMockitoRunner(className) || isFromMockitoRule(className)) {
return true;
} else if (isMockDispatcher(className) || isFromMockito(className)) {
} else if (isMockDispatcher(className)
|| isFromMockito(className)
|| isMethodHandle(className)) {
return false;
} else {
return true;
}
}

/* Some mock makers (like inline) use java.lang.invoke.MethodHandle to dispatch calls */
private static boolean isMethodHandle(String className) {
return className.startsWith("java.lang.invoke.MethodHandle");
}

private static boolean isMockDispatcher(String className) {
return (className.contains("$$EnhancerByMockitoWithCGLIB$$")
|| className.contains("$MockitoMock$"));
Expand Down
Expand Up @@ -8,9 +8,12 @@
import static org.mockito.internal.util.ObjectMethodsGuru.isToStringMethod;

import java.io.Serializable;
import java.lang.reflect.Method;
import java.util.Arrays;

import org.mockito.Mockito;
import org.mockito.internal.creation.MockSettingsImpl;
import org.mockito.internal.creation.bytebuddy.MockAccess;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.internal.util.MockUtil;
import org.mockito.invocation.InvocationOnMock;
Expand Down Expand Up @@ -89,9 +92,28 @@ public Object answer(InvocationOnMock currentInvocation) throws Throwable {
if (isToStringMethod(currentInvocation.getMethod())) {
return "SmartNull returned by this unstubbed method call on a mock:\n"
+ unstubbedInvocation;
} else if (isMethodOf(
MockAccess.class, currentInvocation.getMock(), currentInvocation.getMethod())) {
/* The MockAccess methods should be called directly */
return currentInvocation.callRealMethod();
}

throw smartNullPointerException(unstubbedInvocation.toString(), location);
}

private static boolean isMethodOf(Class<?> clazz, Object instance, Method method) {
if (!clazz.isInstance(instance)) {
return false;
}

for (Method m : clazz.getDeclaredMethods()) {
if (m.getName().equalsIgnoreCase(method.getName())
&& Arrays.equals(m.getParameterTypes(), method.getParameterTypes())) {
return true;
}
}

return false;
}
}
}
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
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
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
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
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
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

0 comments on commit 733b5c8

Please sign in to comment.