From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sun, 17 Mar 2019 23:04:30 +0000 Subject: [PATCH] Test changes - convert to mockito for mocking of types - Allow use of TYPE_USE annotations - Ignore package-private methods for nullability annotations - Add excludes for classes which don't pass Co-authored-by: Riley Park Co-authored-by: Jake Potrebic diff --git a/build.gradle.kts b/build.gradle.kts index 34117c1178e04885ff9be5b73c7c2547e77ed4d5..c174d9d6534382ae9d002b883bde929d46e32789 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { compileOnlyApi(checkerQual) testCompileOnly(checkerQual) // Paper end + testImplementation("org.mockito:mockito-core:4.9.0") // Paper - add mockito testImplementation("org.apache.commons:commons-lang3:3.12.0") testImplementation("junit:junit:4.13.2") diff --git a/src/test/java/io/papermc/paper/testing/EmptyRegistry.java b/src/test/java/io/papermc/paper/testing/EmptyRegistry.java new file mode 100644 index 0000000000000000000000000000000000000000..ba9ddce87a9f385e729a5c2cf7c5eec120e388a7 --- /dev/null +++ b/src/test/java/io/papermc/paper/testing/EmptyRegistry.java @@ -0,0 +1,23 @@ +package io.papermc.paper.testing; + +import java.util.Collections; +import java.util.Iterator; +import org.bukkit.Keyed; +import org.bukkit.NamespacedKey; +import org.bukkit.Registry; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public record EmptyRegistry() implements Registry { + + @NotNull + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } + + @Override + public @Nullable Keyed get(@NotNull final NamespacedKey key) { + return null; + } +} diff --git a/src/test/java/io/papermc/paper/testing/EmptyTag.java b/src/test/java/io/papermc/paper/testing/EmptyTag.java new file mode 100644 index 0000000000000000000000000000000000000000..77154095cfb8b259bdb318e8ff40cb6f559ebc18 --- /dev/null +++ b/src/test/java/io/papermc/paper/testing/EmptyTag.java @@ -0,0 +1,31 @@ +package io.papermc.paper.testing; + +import java.util.Collections; +import java.util.Set; +import org.bukkit.Keyed; +import org.bukkit.NamespacedKey; +import org.bukkit.Tag; +import org.jetbrains.annotations.NotNull; + +public record EmptyTag(NamespacedKey key) implements Tag { + + @SuppressWarnings("deprecation") + public EmptyTag() { + this(NamespacedKey.randomKey()); + } + + @Override + public @NotNull NamespacedKey getKey() { + return this.key; + } + + @Override + public boolean isTagged(@NotNull final Keyed item) { + return false; + } + + @Override + public @NotNull Set getValues() { + return Collections.emptySet(); + } +} diff --git a/src/test/java/io/papermc/paper/testing/TestServer.java b/src/test/java/io/papermc/paper/testing/TestServer.java new file mode 100644 index 0000000000000000000000000000000000000000..756acf231b1b076b08046d86992ba7ce7f62a94f --- /dev/null +++ b/src/test/java/io/papermc/paper/testing/TestServer.java @@ -0,0 +1,48 @@ +package io.papermc.paper.testing; + +import java.util.logging.Logger; +import org.bukkit.Bukkit; +import org.bukkit.NamespacedKey; +import org.bukkit.Server; +import org.bukkit.command.SimpleCommandMap; +import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.SimplePluginManager; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public final class TestServer { + + @SuppressWarnings("removal") + public static void setup() { + //noinspection ConstantValue + if (Bukkit.getServer() != null) { + return; + } + + final Server dummyServer = mock(Server.class); + + final Logger logger = Logger.getLogger(TestServer.class.getCanonicalName()); + when(dummyServer.getLogger()).thenReturn(logger); + when(dummyServer.getName()).thenReturn(TestServer.class.getSimpleName()); + when(dummyServer.getVersion()).thenReturn("Version_" + TestServer.class.getPackage().getImplementationVersion()); + when(dummyServer.getBukkitVersion()).thenReturn("BukkitVersion_" + TestServer.class.getPackage().getImplementationVersion()); + + + final Thread currentThread = Thread.currentThread(); + when(dummyServer.isPrimaryThread()).thenAnswer(ignored -> Thread.currentThread().equals(currentThread)); + + when(dummyServer.getTag(anyString(), any(NamespacedKey.class), any())).thenAnswer(ignored -> new EmptyTag()); + + final PluginManager pluginManager = new SimplePluginManager(dummyServer, new SimpleCommandMap(dummyServer)); + when(dummyServer.getPluginManager()).thenReturn(pluginManager); + + when(dummyServer.getRegistry(any())).thenAnswer(ignored -> new EmptyRegistry()); + when(dummyServer.getScoreboardCriteria(anyString())).thenReturn(null); + + Bukkit.setServer(dummyServer); + } + +} diff --git a/src/test/java/org/bukkit/AnnotationTest.java b/src/test/java/org/bukkit/AnnotationTest.java index 4ac3dd977e75cd8464163351d306e037ee32cb48..c26ea217927ba77611e6ae93f8df50a83bceb3dd 100644 --- a/src/test/java/org/bukkit/AnnotationTest.java +++ b/src/test/java/org/bukkit/AnnotationTest.java @@ -29,7 +29,13 @@ public class AnnotationTest { "Lorg/jetbrains/annotations/Nullable;", "Lorg/jetbrains/annotations/NotNull;", "Lorg/jetbrains/annotations/Contract;", - "Lorg/bukkit/UndefinedNullability;" + "Lorg/bukkit/UndefinedNullability;", + // Paper start + "Lorg/checkerframework/checker/nullness/qual/MonotonicNonNull;", + "Lorg/checkerframework/checker/nullness/qual/NonNull;", + "Lorg/checkerframework/checker/nullness/qual/Nullable;", + "Lorg/checkerframework/checker/nullness/qual/PolyNull;", + // Paper end }; private static final String[] EXCLUDED_CLASSES = { @@ -40,7 +46,17 @@ public class AnnotationTest { "org/bukkit/util/io/Wrapper", "org/bukkit/plugin/java/PluginClassLoader", // Generic functional interface - "org/bukkit/util/Consumer" + "org/bukkit/util/Consumer", + // Paper start + // Timings history is broken in terms of nullability due to guavas Function defining that the param is NonNull + "co/aikar/timings/TimingHistory$2", + "co/aikar/timings/TimingHistory$2$1", + "co/aikar/timings/TimingHistory$2$1$1", + "co/aikar/timings/TimingHistory$2$1$2", + "co/aikar/timings/TimingHistory$3", + "co/aikar/timings/TimingHistory$4", + "co/aikar/timings/TimingHistoryEntry$1" + // Paper end }; @Test @@ -67,14 +83,40 @@ public class AnnotationTest { } if (mustBeAnnotated(Type.getReturnType(method.desc)) && !isWellAnnotated(method.invisibleAnnotations)) { + // Paper start - Allow use of TYPE_USE annotations + boolean warn = true; + if (isWellAnnotated(method.visibleTypeAnnotations)) { + warn = false; + } else if (method.invisibleTypeAnnotations != null) { + dance: for (final org.objectweb.asm.tree.TypeAnnotationNode invisibleTypeAnnotation : method.invisibleTypeAnnotations) { + final org.objectweb.asm.TypeReference ref = new org.objectweb.asm.TypeReference(invisibleTypeAnnotation.typeRef); + if (ref.getSort() == org.objectweb.asm.TypeReference.METHOD_RETURN && java.util.Arrays.asList(ACCEPTED_ANNOTATIONS).contains(invisibleTypeAnnotation.desc)) { + warn = false; + break dance; // cha cha real smooth + } + } + } + if (warn) + // Paper end warn(errors, clazz, method, "return value"); } Type[] paramTypes = Type.getArgumentTypes(method.desc); List parameters = method.parameters; + dancing: // Paper for (int i = 0; i < paramTypes.length; i++) { if (mustBeAnnotated(paramTypes[i]) ^ isWellAnnotated(method.invisibleParameterAnnotations == null ? null : method.invisibleParameterAnnotations[i])) { + // Paper start + if (method.invisibleTypeAnnotations != null) { + for (final org.objectweb.asm.tree.TypeAnnotationNode invisibleTypeAnnotation : method.invisibleTypeAnnotations) { + final org.objectweb.asm.TypeReference ref = new org.objectweb.asm.TypeReference(invisibleTypeAnnotation.typeRef); + if (ref.getSort() == org.objectweb.asm.TypeReference.METHOD_FORMAL_PARAMETER && ref.getTypeParameterIndex() == i && java.util.Arrays.asList(ACCEPTED_ANNOTATIONS).contains(invisibleTypeAnnotation.desc)) { + continue dancing; + } + } + } + // Paper end - Allow use of TYPE_USE annotations ParameterNode paramNode = parameters == null ? null : parameters.get(i); String paramName = paramNode == null ? null : paramNode.name; @@ -91,13 +133,18 @@ public class AnnotationTest { Collections.sort(errors); - System.out.println(errors.size() + " missing annotation(s):"); + StringBuilder builder = new StringBuilder() + .append("There ") + .append(errors.size() != 1 ? "are " : "is ") + .append(errors.size()) + .append(" missing annotation") + .append(errors.size() != 1 ? "s:\n" : ":\n"); + for (String message : errors) { - System.out.print("\t"); - System.out.println(message); + builder.append("\t").append(message).append("\n"); } - Assert.fail("There " + errors.size() + " are missing annotation(s)"); + Assert.fail(builder.toString()); } private static void collectClasses(@NotNull File from, @NotNull Map to) throws IOException { @@ -140,6 +187,11 @@ public class AnnotationTest { // Exceptions are excluded return false; } + // Paper start + if (isInternal(clazz.invisibleAnnotations)) { + return false; + } + // Paper end for (String excludedClass : EXCLUDED_CLASSES) { if (excludedClass.equals(clazz.name)) { @@ -152,7 +204,7 @@ public class AnnotationTest { private static boolean isMethodIncluded(@NotNull ClassNode clazz, @NotNull MethodNode method, @NotNull Map allClasses) { // Exclude private, synthetic and deprecated methods - if ((method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_DEPRECATED)) != 0) { + if ((method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_DEPRECATED)) != 0 || (method.access & (Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED | Opcodes.ACC_PUBLIC)) == 0) { // Paper - ignore package-private return false; } @@ -170,11 +222,30 @@ public class AnnotationTest { if ("".equals(method.name) && isAnonymous(clazz)) { return false; } + // Paper start + if (isInternal(method.invisibleAnnotations)) { + return false; + } + // Paper end return true; } + // Paper start + private static boolean isInternal(List annotations) { + if (annotations == null) { + return false; + } + for (AnnotationNode node : annotations) { + if (node.desc.equals("Lorg/jetbrains/annotations/ApiStatus$Internal;")) { + return true; + } + } + + return false; + } + // Paper end - private static boolean isWellAnnotated(@Nullable List annotations) { + private static boolean isWellAnnotated(@Nullable List annotations) { // Paper if (annotations == null) { return false; } diff --git a/src/test/java/org/bukkit/TestServer.java b/src/test/java/org/bukkit/TestServer.java deleted file mode 100644 index 701a17c10f31cd345238a3c568264178ce372faa..0000000000000000000000000000000000000000 --- a/src/test/java/org/bukkit/TestServer.java +++ /dev/null @@ -1,139 +0,0 @@ -package org.bukkit; - -import com.google.common.collect.ImmutableMap; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; -import java.util.Iterator; -import java.util.Map; -import java.util.logging.Logger; -import org.bukkit.command.SimpleCommandMap; -import org.bukkit.plugin.PluginManager; -import org.bukkit.plugin.SimplePluginManager; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -public final class TestServer implements InvocationHandler { - private static interface MethodHandler { - Object handle(TestServer server, Object[] args); - } - - private static final Map methods; - - static { - try { - ImmutableMap.Builder methodMap = ImmutableMap.builder(); - methodMap.put( - Server.class.getMethod("isPrimaryThread"), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return Thread.currentThread().equals(server.creatingThread); - } - } - ); - methodMap.put( - Server.class.getMethod("getPluginManager"), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return server.pluginManager; - } - } - ); - methodMap.put( - Server.class.getMethod("getLogger"), - new MethodHandler() { - final Logger logger = Logger.getLogger(TestServer.class.getCanonicalName()); - @Override - public Object handle(TestServer server, Object[] args) { - return logger; - } - } - ); - methodMap.put( - Server.class.getMethod("getName"), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return TestServer.class.getSimpleName(); - } - } - ); - methodMap.put( - Server.class.getMethod("getVersion"), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return "Version_" + TestServer.class.getPackage().getImplementationVersion(); - } - } - ); - methodMap.put( - Server.class.getMethod("getBukkitVersion"), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return "BukkitVersion_" + TestServer.class.getPackage().getImplementationVersion(); - } - } - ); - methodMap.put( - Server.class.getMethod("getRegistry", Class.class), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - return new Registry() { - @NotNull - @Override - public Iterator iterator() { - return null; - } - - @Nullable - @Override - public Keyed get(@NotNull NamespacedKey key) { - return null; - } - }; - } - } - ); - methodMap.put( - Server.class.getMethod("getScoreboardCriteria", String.class), - new MethodHandler() { - @Override - public Object handle(TestServer server, Object[] args) { - // Does not need to return anything. Exists solely to test CriteriaTest which has static init fields - return null; - } - } - ); - methods = methodMap.build(); - - TestServer server = new TestServer(); - Server instance = Proxy.getProxyClass(Server.class.getClassLoader(), Server.class).asSubclass(Server.class).getConstructor(InvocationHandler.class).newInstance(server); - Bukkit.setServer(instance); - server.pluginManager = new SimplePluginManager(instance, new SimpleCommandMap(instance)); - } catch (Throwable t) { - throw new Error(t); - } - } - - private Thread creatingThread = Thread.currentThread(); - private PluginManager pluginManager; - private TestServer() {}; - - public static Server getInstance() { - return Bukkit.getServer(); - } - - @Override - public Object invoke(Object proxy, Method method, Object[] args) { - MethodHandler handler = methods.get(method); - if (handler != null) { - return handler.handle(this, args); - } - throw new UnsupportedOperationException(String.valueOf(method)); - } -} diff --git a/src/test/java/org/bukkit/TestWorld.java b/src/test/java/org/bukkit/TestWorld.java index ab34f1199921d415fa2ca6e281a8125c9e6d7173..f64d024f5bbf9482aaddb56597b23b04c66f21bf 100644 --- a/src/test/java/org/bukkit/TestWorld.java +++ b/src/test/java/org/bukkit/TestWorld.java @@ -18,7 +18,7 @@ public final class TestWorld implements InvocationHandler { static { try { - TestServer.getInstance(); + io.papermc.paper.testing.TestServer.setup(); // Paper ImmutableMap.Builder methodMap = ImmutableMap.builder(); methodMap.put( diff --git a/src/test/java/org/bukkit/event/SyntheticEventTest.java b/src/test/java/org/bukkit/event/SyntheticEventTest.java index d402cb59f508205ebe9ee450594826b04cecb90b..09886568ae6167141b463b6262565fa212af3385 100644 --- a/src/test/java/org/bukkit/event/SyntheticEventTest.java +++ b/src/test/java/org/bukkit/event/SyntheticEventTest.java @@ -1,6 +1,5 @@ package org.bukkit.event; -import org.bukkit.TestServer; import org.bukkit.plugin.PluginLoader; import org.bukkit.plugin.SimplePluginManager; import org.bukkit.plugin.TestPlugin; @@ -12,14 +11,15 @@ public class SyntheticEventTest { @SuppressWarnings("deprecation") @Test public void test() { - final JavaPluginLoader loader = new JavaPluginLoader(TestServer.getInstance()); + io.papermc.paper.testing.TestServer.setup(); // Paper + final JavaPluginLoader loader = new JavaPluginLoader(org.bukkit.Bukkit.getServer()); // Paper TestPlugin plugin = new TestPlugin(getClass().getName()) { @Override public PluginLoader getPluginLoader() { return loader; } }; - SimplePluginManager pluginManager = new SimplePluginManager(TestServer.getInstance(), null); + SimplePluginManager pluginManager = new SimplePluginManager(org.bukkit.Bukkit.getServer(), null); // Paper TestEvent event = new TestEvent(false); Impl impl = new Impl(); diff --git a/src/test/java/org/bukkit/plugin/PluginManagerTest.java b/src/test/java/org/bukkit/plugin/PluginManagerTest.java index f188cd4f3b07027c30d41f1162db77a506b7b6bb..c46ed2acb82db814d660459b705dd49e6d44240f 100644 --- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java +++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java @@ -2,7 +2,6 @@ package org.bukkit.plugin; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; -import org.bukkit.TestServer; import org.bukkit.event.Event; import org.bukkit.event.TestEvent; import org.bukkit.permissions.Permission; @@ -14,7 +13,7 @@ public class PluginManagerTest { volatile Object value = null; } - private static final PluginManager pm = TestServer.getInstance().getPluginManager(); + private static final PluginManager pm = org.bukkit.Bukkit.getServer().getPluginManager(); // Paper private final MutableObject store = new MutableObject(); diff --git a/src/test/java/org/bukkit/scoreboard/CriteriaTest.java b/src/test/java/org/bukkit/scoreboard/CriteriaTest.java index eb94b6f4d58cd9f66b07791c57af7e359992e28c..a93f28e2f987a36e2c7e4f7d31506b750bdb222b 100644 --- a/src/test/java/org/bukkit/scoreboard/CriteriaTest.java +++ b/src/test/java/org/bukkit/scoreboard/CriteriaTest.java @@ -2,7 +2,6 @@ package org.bukkit.scoreboard; import org.bukkit.Material; import org.bukkit.Statistic; -import org.bukkit.TestServer; import org.bukkit.entity.EntityType; import org.junit.Assert; import org.junit.Test; @@ -11,7 +10,7 @@ public class CriteriaTest { @Test public void testStatistic() { - TestServer.getInstance(); + io.papermc.paper.testing.TestServer.setup(); // Paper Assert.assertThrows(IllegalArgumentException.class, () -> Criteria.statistic(Statistic.AVIATE_ONE_CM, Material.STONE)); // Generic statistic with block Assert.assertThrows(IllegalArgumentException.class, () -> Criteria.statistic(Statistic.AVIATE_ONE_CM, EntityType.CREEPER)); // Generic statistic with entity type