Skip to content

Commit ebc1685

Browse files
TimvdLippeTimvdLippe
authored
Add annotation to mark a type as DoNotMock (#1833)
Mocking types that users not own [1] or are severely complicating test logic [2] leads to brittle or wrong tests. In particular, the StackOverflow answer is wrong, as the contract of java.util.Map is violated. When a new key is added to the Map, the stubbed return would be wrong. In Google we have used the DoNotMock annotation via ErrorProne [3] to annotate these types, as well as an internal list of types that can't be mocked (this includes several java.util types). We are using a custom Mockmaker to enforce this on run-time. Based on our successful experience with DoNotMock (we have seen a large reduction in bad/broken tests for types involved), we are proposing to open source this into Mockito itself. The DoNotMock annotation can be added to any type, e.g. classes and interfaces. If, in the type hierarchy of the class-to-be-mocked, there is a type that is annotated with DoNotMock, Mockito will throw a DoNotMockException. This would help preventing issues such as #1827 and #1734 which is in-line with the guidance on our wiki [1]. A follow-up change would allow us to define external types (like the java.util types) that can't be mocked. (We can't add the annotation to the types, as they live in the JDK instead.) This PR also introduces the DoNotMockEnforcer interface which users can override to implement their special handling of types annotated with DoNotMock. [1]: https://github.com/mockito/mockito/wiki/How-to-write-good-tests#dont-mock-a-type-you-dont-own [2]: https://stackoverflow.com/a/15820143 [3]: https://errorprone.info/api/latest/com/google/errorprone/annotations/DoNotMock.html
1 parent 102cc38 commit ebc1685

File tree

14 files changed

+405
-1
lines changed

14 files changed

+405
-1
lines changed

‎src/main/java/org/mockito/DoNotMock.java

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright (c) 2019 Mockito contributors
3+
* This program is made available under the terms of the MIT License.
4+
*/
5+
package org.mockito;
6+
7+
import static java.lang.annotation.ElementType.TYPE;
8+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
9+
10+
import java.lang.annotation.Documented;
11+
import java.lang.annotation.Retention;
12+
import java.lang.annotation.Target;
13+
14+
/**
15+
* Annotation representing a type that should not be mocked.
16+
* <p>When marking a type {@code @DoNotMock}, you should always point to alternative testing
17+
* solutions such as standard fakes or other testing utilities.
18+
*
19+
* Mockito enforces {@code @DoNotMock} with the {@link org.mockito.plugins.DoNotMockEnforcer}.
20+
*
21+
* If you want to use a custom {@code @DoNotMock} annotation, the {@link org.mockito.plugins.DoNotMockEnforcer}
22+
* will match on annotations with a type ending in "org.mockito.DoNotMock". You can thus place
23+
* your custom annotation in {@code com.my.package.org.mockito.DoNotMock} and Mockito will enforce
24+
* that types annotated by {@code @com.my.package.org.mockito.DoNotMock} can not be mocked.
25+
*
26+
* <pre class="code"><code class="java">
27+
* &#064;DoNotMock(reason = "Use a real instance instead")
28+
* class DoNotMockMe {}
29+
* </code></pre>
30+
*/
31+
@Target({TYPE})
32+
@Retention(RUNTIME)
33+
@Documented
34+
public @interface DoNotMock {
35+
/**
36+
* The reason why the annotated type should not be mocked.
37+
*
38+
* <p>This should suggest alternative APIs to use for testing objects of this type.
39+
*/
40+
String reason() default "Create a real instance instead.";
41+
}

‎src/main/java/org/mockito/exceptions/misusing/DoNotMockException.java

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/*
2+
* Copyright (c) 2019 Mockito contributors
3+
* This program is made available under the terms of the MIT License.
4+
*/
5+
package org.mockito.exceptions.misusing;
6+
7+
import org.mockito.exceptions.base.MockitoException;
8+
9+
/**
10+
* Thrown when attempting to mock a class that is annotated with {@link org.mockito.DoNotMock}.
11+
*/
12+
public class DoNotMockException extends MockitoException {
13+
public DoNotMockException(String message) {
14+
super(message);
15+
}
16+
}

‎src/main/java/org/mockito/internal/MockitoCore.java

+43
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,19 @@
2727
import static org.mockito.internal.verification.VerificationModeFactory.noMoreInteractions;
2828

2929
import java.util.Arrays;
30+
import java.util.Collections;
31+
import java.util.HashSet;
3032
import java.util.List;
33+
import java.util.Set;
34+
3135
import org.mockito.InOrder;
3236
import org.mockito.MockSettings;
3337
import org.mockito.MockedConstruction;
3438
import org.mockito.MockedStatic;
3539
import org.mockito.MockingDetails;
40+
import org.mockito.exceptions.misusing.DoNotMockException;
3641
import org.mockito.exceptions.misusing.NotAMockException;
42+
import org.mockito.internal.configuration.plugins.Plugins;
3743
import org.mockito.internal.creation.MockSettingsImpl;
3844
import org.mockito.internal.invocation.finder.VerifiableInvocationsFinder;
3945
import org.mockito.internal.listeners.VerificationStartedNotifier;
@@ -53,6 +59,7 @@
5359
import org.mockito.invocation.Invocation;
5460
import org.mockito.invocation.MockHandler;
5561
import org.mockito.mock.MockCreationSettings;
62+
import org.mockito.plugins.DoNotMockEnforcer;
5663
import org.mockito.plugins.MockMaker;
5764
import org.mockito.quality.Strictness;
5865
import org.mockito.stubbing.LenientStubber;
@@ -67,6 +74,10 @@
6774
@SuppressWarnings("unchecked")
6875
public class MockitoCore {
6976

77+
private static final DoNotMockEnforcer DO_NOT_MOCK_ENFORCER = Plugins.getDoNotMockEnforcer();
78+
private static final Set<Class<?>> MOCKABLE_CLASSES =
79+
Collections.synchronizedSet(new HashSet<>());
80+
7081
public boolean isTypeMockable(Class<?> typeToMock) {
7182
return typeMockabilityOf(typeToMock).mockable();
7283
}
@@ -81,11 +92,43 @@ public <T> T mock(Class<T> typeToMock, MockSettings settings) {
8192
}
8293
MockSettingsImpl impl = (MockSettingsImpl) settings;
8394
MockCreationSettings<T> creationSettings = impl.build(typeToMock);
95+
checkDoNotMockAnnotation(creationSettings.getTypeToMock(), creationSettings);
8496
T mock = createMock(creationSettings);
8597
mockingProgress().mockingStarted(mock, creationSettings);
8698
return mock;
8799
}
88100

101+
private void checkDoNotMockAnnotation(
102+
Class<?> typeToMock, MockCreationSettings<?> creationSettings) {
103+
checkDoNotMockAnnotationForType(typeToMock);
104+
for (Class<?> aClass : creationSettings.getExtraInterfaces()) {
105+
checkDoNotMockAnnotationForType(aClass);
106+
}
107+
}
108+
109+
private static void checkDoNotMockAnnotationForType(Class<?> type) {
110+
// Object and interfaces do not have a super class
111+
if (type == null) {
112+
return;
113+
}
114+
115+
if (MOCKABLE_CLASSES.contains(type)) {
116+
return;
117+
}
118+
119+
String warning = DO_NOT_MOCK_ENFORCER.checkTypeForDoNotMockViolation(type);
120+
if (warning != null) {
121+
throw new DoNotMockException(warning);
122+
}
123+
124+
checkDoNotMockAnnotationForType(type.getSuperclass());
125+
for (Class<?> aClass : type.getInterfaces()) {
126+
checkDoNotMockAnnotationForType(aClass);
127+
}
128+
129+
MOCKABLE_CLASSES.add(type);
130+
}
131+
89132
public <T> MockedStatic<T> mockStatic(Class<T> classToMock, MockSettings settings) {
90133
if (!MockSettingsImpl.class.isInstance(settings)) {
91134
throw new IllegalArgumentException(

‎src/main/java/org/mockito/internal/configuration/DefaultDoNotMockEnforcer.java

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) 2019 Mockito contributors
3+
* This program is made available under the terms of the MIT License.
4+
*/
5+
package org.mockito.internal.configuration;
6+
7+
import java.lang.annotation.Annotation;
8+
9+
import org.mockito.DoNotMock;
10+
import org.mockito.plugins.DoNotMockEnforcer;
11+
12+
public class DefaultDoNotMockEnforcer implements DoNotMockEnforcer {
13+
14+
@Override
15+
public String checkTypeForDoNotMockViolation(Class<?> type) {
16+
for (Annotation annotation : type.getAnnotations()) {
17+
if (annotation.annotationType().getName().endsWith("org.mockito.DoNotMock")) {
18+
String exceptionMessage =
19+
type + " is annotated with @org.mockito.DoNoMock and can't be mocked.";
20+
if (DoNotMock.class.equals(annotation.annotationType())) {
21+
exceptionMessage += " " + type.getAnnotation(DoNotMock.class).reason();
22+
}
23+
24+
return exceptionMessage;
25+
}
26+
}
27+
28+
return null;
29+
}
30+
}

‎src/main/java/org/mockito/internal/configuration/plugins/DefaultMockitoPlugins.java

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.HashMap;
88
import java.util.Map;
99
import org.mockito.plugins.AnnotationEngine;
10+
import org.mockito.plugins.DoNotMockEnforcer;
1011
import org.mockito.plugins.InstantiatorProvider2;
1112
import org.mockito.plugins.MemberAccessor;
1213
import org.mockito.plugins.MockMaker;
@@ -47,6 +48,9 @@ class DefaultMockitoPlugins implements MockitoPlugins {
4748
"org.mockito.internal.util.reflection.ReflectionMemberAccessor");
4849
DEFAULT_PLUGINS.put(
4950
MODULE_ALIAS, "org.mockito.internal.util.reflection.ModuleMemberAccessor");
51+
DEFAULT_PLUGINS.put(
52+
DoNotMockEnforcer.class.getName(),
53+
"org.mockito.internal.configuration.DefaultDoNotMockEnforcer");
5054
}
5155

5256
@Override

‎src/main/java/org/mockito/internal/configuration/plugins/PluginRegistry.java

+14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import java.util.List;
88
import org.mockito.plugins.AnnotationEngine;
9+
import org.mockito.plugins.DoNotMockEnforcer;
910
import org.mockito.plugins.InstantiatorProvider2;
1011
import org.mockito.plugins.MemberAccessor;
1112
import org.mockito.plugins.MockMaker;
@@ -44,6 +45,9 @@ class PluginRegistry {
4445
private final List<MockResolver> mockResolvers =
4546
new PluginLoader(pluginSwitch).loadPlugins(MockResolver.class);
4647

48+
private final DoNotMockEnforcer doNotMockEnforcer =
49+
new PluginLoader(pluginSwitch).loadPlugin(DoNotMockEnforcer.class);
50+
4751
PluginRegistry() {
4852
instantiatorProvider =
4953
new PluginLoader(pluginSwitch).loadPlugin(InstantiatorProvider2.class);
@@ -108,6 +112,16 @@ MockitoLogger getMockitoLogger() {
108112
return mockitoLogger;
109113
}
110114

115+
/**
116+
* Returns the DoNotMock enforce for the current runtime.
117+
*
118+
* <p> Returns {@link org.mockito.internal.configuration.DefaultDoNotMockEnforcer} if no
119+
* {@link DoNotMockEnforcer} extension exists or is visible in the current classpath.</p>
120+
*/
121+
DoNotMockEnforcer getDoNotMockEnforcer() {
122+
return doNotMockEnforcer;
123+
}
124+
111125
/**
112126
* Returns a list of available mock resolvers if any.
113127
*

‎src/main/java/org/mockito/internal/configuration/plugins/Plugins.java

+12
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
*/
55
package org.mockito.internal.configuration.plugins;
66

7+
import org.mockito.DoNotMock;
78
import java.util.List;
89
import org.mockito.plugins.AnnotationEngine;
10+
import org.mockito.plugins.DoNotMockEnforcer;
911
import org.mockito.plugins.InstantiatorProvider2;
1012
import org.mockito.plugins.MemberAccessor;
1113
import org.mockito.plugins.MockMaker;
@@ -93,5 +95,15 @@ public static MockitoPlugins getPlugins() {
9395
return new DefaultMockitoPlugins();
9496
}
9597

98+
/**
99+
* Returns the {@link DoNotMock} enforcer available for the current runtime.
100+
*
101+
* <p> Returns {@link org.mockito.internal.configuration.DefaultDoNotMockEnforcer} if no
102+
* {@link DoNotMockEnforcer} extension exists or is visible in the current classpath.</p>
103+
*/
104+
public static DoNotMockEnforcer getDoNotMockEnforcer() {
105+
return registry.getDoNotMockEnforcer();
106+
}
107+
96108
private Plugins() {}
97109
}

‎src/main/java/org/mockito/plugins/DoNotMockEnforcer.java

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright (c) 2019 Mockito contributors
3+
* This program is made available under the terms of the MIT License.
4+
*/
5+
package org.mockito.plugins;
6+
7+
/**
8+
* Enforcer that is applied to every type in the type hierarchy of the class-to-be-mocked.
9+
*/
10+
public interface DoNotMockEnforcer {
11+
12+
/**
13+
* If this type is allowed to be mocked. Return an empty optional if the enforcer allows
14+
* this type to be mocked. Return a message if there is a reason this type can not be mocked.
15+
*
16+
* Note that Mockito performs traversal of the type hierarchy. Implementations of this class
17+
* should therefore not perform type traversal themselves but rely on Mockito.
18+
*
19+
* @param type The type to check
20+
* @return Optional message if this type can not be mocked, or an empty optional if type can be mocked
21+
*/
22+
String checkTypeForDoNotMockViolation(Class<?> type);
23+
}

0 commit comments

Comments
 (0)