From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jake Potrebic Date: Mon, 13 Feb 2023 14:14:56 -0800 Subject: [PATCH] Test changes Co-authored-by: yannnicklamprecht diff --git a/build.gradle.kts b/build.gradle.kts index f276414e9e81abf8f1f80991ebd5ab43472e07b1..7a0f2391a464eeebc5e57856300bc000b8d35e52 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -22,6 +22,7 @@ dependencies { testImplementation("org.hamcrest:hamcrest:2.2") testImplementation("org.mockito:mockito-core:5.11.0") testImplementation("org.ow2.asm:asm-tree:9.7") + testImplementation("org.junit-pioneer:junit-pioneer:2.2.0") // Paper - CartesianTest } paperweight { @@ -55,6 +56,12 @@ tasks.jar { } } +// Paper start - compile tests with -parameters for better junit parameterized test names +tasks.compileTestJava { + options.compilerArgs.add("-parameters") +} +// Paper end + publishing { publications.create("maven") { } diff --git a/src/test/java/io/papermc/paper/registry/RegistryKeyTest.java b/src/test/java/io/papermc/paper/registry/RegistryKeyTest.java new file mode 100644 index 0000000000000000000000000000000000000000..e1c14886064cde56be7fcd8f22a6ecb2d222a762 --- /dev/null +++ b/src/test/java/io/papermc/paper/registry/RegistryKeyTest.java @@ -0,0 +1,33 @@ +package io.papermc.paper.registry; + +import java.util.Optional; +import java.util.stream.Stream; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import org.bukkit.support.AbstractTestingBase; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +class RegistryKeyTest extends AbstractTestingBase { + + @BeforeAll + static void before() throws ClassNotFoundException { + Class.forName(RegistryKey.class.getName()); // load all keys so they are found for the test + } + + static Stream> data() { + return RegistryKeyImpl.REGISTRY_KEYS.stream(); + } + + @ParameterizedTest + @MethodSource("data") + void testApiRegistryKeysExist(final RegistryKey key) { + final Optional> registry = AbstractTestingBase.REGISTRY_CUSTOM.registry(ResourceKey.createRegistryKey(ResourceLocation.parse(key.key().asString()))); + assertTrue(registry.isPresent(), "Missing vanilla registry for " + key.key().asString()); + + } +} diff --git a/src/test/java/io/papermc/paper/util/EmptyTag.java b/src/test/java/io/papermc/paper/util/EmptyTag.java new file mode 100644 index 0000000000000000000000000000000000000000..6eb95a5e2534974c0e52e2b78b04e7c2b2f28525 --- /dev/null +++ b/src/test/java/io/papermc/paper/util/EmptyTag.java @@ -0,0 +1,31 @@ +package io.papermc.paper.util; + +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/util/MethodParameterProvider.java b/src/test/java/io/papermc/paper/util/MethodParameterProvider.java new file mode 100644 index 0000000000000000000000000000000000000000..3f58ef36df34cd15fcb72189eeff057654adf0c6 --- /dev/null +++ b/src/test/java/io/papermc/paper/util/MethodParameterProvider.java @@ -0,0 +1,206 @@ +/* + * Copyright 2015-2023 the original author or authors of https://github.com/junit-team/junit5/blob/6593317c15fb556febbde11914fa7afe00abf8cd/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package io.papermc.paper.util; + +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.support.AnnotationConsumer; +import org.junit.platform.commons.JUnitException; +import org.junit.platform.commons.PreconditionViolationException; +import org.junit.platform.commons.util.ClassLoaderUtils; +import org.junit.platform.commons.util.CollectionUtils; +import org.junit.platform.commons.util.Preconditions; +import org.junit.platform.commons.util.ReflectionUtils; +import org.junit.platform.commons.util.StringUtils; +import org.junitpioneer.jupiter.cartesian.CartesianParameterArgumentsProvider; + +import static java.lang.String.format; +import static java.util.Arrays.stream; +import static java.util.stream.Collectors.toList; +import static org.junit.platform.commons.util.AnnotationUtils.isAnnotated; +import static org.junit.platform.commons.util.CollectionUtils.isConvertibleToStream; + +public class MethodParameterProvider implements CartesianParameterArgumentsProvider, AnnotationConsumer { + private MethodParameterSource source; + + MethodParameterProvider() { + } + + @Override + public void accept(final MethodParameterSource source) { + this.source = source; + } + + @Override + public Stream provideArguments(ExtensionContext context, Parameter parameter) { + return this.provideArguments(context, this.source); + } + + // Below is mostly copied from MethodArgumentsProvider + + private static final Predicate isFactoryMethod = // + method -> isConvertibleToStream(method.getReturnType()) && !isTestMethod(method); + + protected Stream provideArguments(ExtensionContext context, MethodParameterSource methodSource) { + Class testClass = context.getRequiredTestClass(); + Method testMethod = context.getRequiredTestMethod(); + Object testInstance = context.getTestInstance().orElse(null); + String[] methodNames = methodSource.value(); + // @formatter:off + return stream(methodNames) + .map(factoryMethodName -> findFactoryMethod(testClass, testMethod, factoryMethodName)) + .map(factoryMethod -> validateFactoryMethod(factoryMethod, testInstance)) + .map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance)) + .flatMap(CollectionUtils::toStream); + // @formatter:on + } + + private static Method findFactoryMethod(Class testClass, Method testMethod, String factoryMethodName) { + String originalFactoryMethodName = factoryMethodName; + + // If the user did not provide a factory method name, find a "default" local + // factory method with the same name as the parameterized test method. + if (StringUtils.isBlank(factoryMethodName)) { + factoryMethodName = testMethod.getName(); + return findFactoryMethodBySimpleName(testClass, testMethod, factoryMethodName); + } + + // Convert local factory method name to fully-qualified method name. + if (!looksLikeAFullyQualifiedMethodName(factoryMethodName)) { + factoryMethodName = testClass.getName() + "#" + factoryMethodName; + } + + // Find factory method using fully-qualified name. + Method factoryMethod = findFactoryMethodByFullyQualifiedName(testClass, testMethod, factoryMethodName); + + // Ensure factory method has a valid return type and is not a test method. + Preconditions.condition(isFactoryMethod.test(factoryMethod), () -> format( + "Could not find valid factory method [%s] for test class [%s] but found the following invalid candidate: %s", + originalFactoryMethodName, testClass.getName(), factoryMethod)); + + return factoryMethod; + } + + private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodName) { + if (factoryMethodName.contains("#")) { + return true; + } + int indexOfFirstDot = factoryMethodName.indexOf('.'); + if (indexOfFirstDot == -1) { + return false; + } + int indexOfLastOpeningParenthesis = factoryMethodName.lastIndexOf('('); + if (indexOfLastOpeningParenthesis > 0) { + // Exclude simple/local method names with parameters + return indexOfFirstDot < indexOfLastOpeningParenthesis; + } + // If we get this far, we conclude the supplied factory method name "looks" + // like it was intended to be a fully-qualified method name, even if the + // syntax is invalid. We do this in order to provide better diagnostics for + // the user when a fully-qualified method name is in fact invalid. + return true; + } + + // package-private for testing + static Method findFactoryMethodByFullyQualifiedName( + Class testClass, Method testMethod, + String fullyQualifiedMethodName + ) { + String[] methodParts = ReflectionUtils.parseFullyQualifiedMethodName(fullyQualifiedMethodName); + String className = methodParts[0]; + String methodName = methodParts[1]; + String methodParameters = methodParts[2]; + ClassLoader classLoader = ClassLoaderUtils.getClassLoader(testClass); + Class clazz = loadRequiredClass(className, classLoader); + + // Attempt to find an exact match first. + Method factoryMethod = ReflectionUtils.findMethod(clazz, methodName, methodParameters).orElse(null); + if (factoryMethod != null) { + return factoryMethod; + } + + boolean explicitParameterListSpecified = // + StringUtils.isNotBlank(methodParameters) || fullyQualifiedMethodName.endsWith("()"); + + // If we didn't find an exact match but an explicit parameter list was specified, + // that's a user configuration error. + Preconditions.condition(!explicitParameterListSpecified, + () -> format("Could not find factory method [%s(%s)] in class [%s]", methodName, methodParameters, + className)); + + // Otherwise, fall back to the same lenient search semantics that are used + // to locate a "default" local factory method. + return findFactoryMethodBySimpleName(clazz, testMethod, methodName); + } + + /** + * Find the factory method by searching for all methods in the given {@code clazz} + * with the desired {@code factoryMethodName} which have return types that can be + * converted to a {@link Stream}, ignoring the {@code testMethod} itself as well + * as any {@code @Test}, {@code @TestTemplate}, or {@code @TestFactory} methods + * with the same name. + * + * @return the single factory method matching the search criteria + * @throws PreconditionViolationException if the factory method was not found or + * multiple competing factory methods with the same name were found + */ + private static Method findFactoryMethodBySimpleName(Class clazz, Method testMethod, String factoryMethodName) { + Predicate isCandidate = candidate -> factoryMethodName.equals(candidate.getName()) + && !testMethod.equals(candidate); + List candidates = ReflectionUtils.findMethods(clazz, isCandidate); + + List factoryMethods = candidates.stream().filter(isFactoryMethod).collect(toList()); + + Preconditions.condition(factoryMethods.size() > 0, () -> { + // If we didn't find the factory method using the isFactoryMethod Predicate, perhaps + // the specified factory method has an invalid return type or is a test method. + // In that case, we report the invalid candidates that were found. + if (candidates.size() > 0) { + return format( + "Could not find valid factory method [%s] in class [%s] but found the following invalid candidates: %s", + factoryMethodName, clazz.getName(), candidates); + } + // Otherwise, report that we didn't find anything. + return format("Could not find factory method [%s] in class [%s]", factoryMethodName, clazz.getName()); + }); + Preconditions.condition(factoryMethods.size() == 1, + () -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(), + factoryMethodName, clazz.getName(), factoryMethods)); + return factoryMethods.get(0); + } + + private static boolean isTestMethod(Method candidate) { + return isAnnotated(candidate, Test.class) || isAnnotated(candidate, TestTemplate.class) + || isAnnotated(candidate, TestFactory.class); + } + + private static Class loadRequiredClass(String className, ClassLoader classLoader) { + return ReflectionUtils.tryToLoadClass(className, classLoader).getOrThrow( + cause -> new JUnitException(format("Could not load class [%s]", className), cause)); + } + + private static Method validateFactoryMethod(Method factoryMethod, Object testInstance) { + Preconditions.condition( + factoryMethod.getDeclaringClass().isInstance(testInstance) || ReflectionUtils.isStatic(factoryMethod), + () -> format("Method '%s' must be static: local factory methods must be static " + + "unless the PER_CLASS @TestInstance lifecycle mode is used; " + + "external factory methods must always be static.", + factoryMethod.toGenericString())); + return factoryMethod; + } +} diff --git a/src/test/java/io/papermc/paper/util/MethodParameterSource.java b/src/test/java/io/papermc/paper/util/MethodParameterSource.java new file mode 100644 index 0000000000000000000000000000000000000000..6cbf11c898439834cffb99ef84e5df1494356809 --- /dev/null +++ b/src/test/java/io/papermc/paper/util/MethodParameterSource.java @@ -0,0 +1,14 @@ +package io.papermc.paper.util; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.junitpioneer.jupiter.cartesian.CartesianArgumentsSource; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.PARAMETER, ElementType.ANNOTATION_TYPE}) +@CartesianArgumentsSource(MethodParameterProvider.class) +public @interface MethodParameterSource { + String[] value() default {}; +} diff --git a/src/test/java/org/bukkit/registry/RegistryConstantsTest.java b/src/test/java/org/bukkit/registry/RegistryConstantsTest.java index ebcb65cb74acdb9d1bcf2b4b3551a2dc6d809bc9..7d9dbed7281099b78d7f898885b37cdcfe8b099f 100644 --- a/src/test/java/org/bukkit/registry/RegistryConstantsTest.java +++ b/src/test/java/org/bukkit/registry/RegistryConstantsTest.java @@ -24,7 +24,7 @@ public class RegistryConstantsTest extends AbstractTestingBase { @Test public void testDamageType() { this.testExcessConstants(DamageType.class, Registry.DAMAGE_TYPE); - // this.testMissingConstants(DamageType.class, Registries.DAMAGE_TYPE); // WIND_CHARGE not registered + this.testMissingConstants(DamageType.class, Registries.DAMAGE_TYPE); // Paper - re-enable this one } @Test diff --git a/src/test/java/org/bukkit/support/AbstractTestingBase.java b/src/test/java/org/bukkit/support/AbstractTestingBase.java index 544307fe34cbcfa286a7d7b30900ebea127d189e..5bda0bc976920f224586f2c0e083b771c676e307 100644 --- a/src/test/java/org/bukkit/support/AbstractTestingBase.java +++ b/src/test/java/org/bukkit/support/AbstractTestingBase.java @@ -53,6 +53,11 @@ public abstract class AbstractTestingBase { MultiPackResourceManager resourceManager = new MultiPackResourceManager(PackType.SERVER_DATA, resourceRepository.getAvailablePacks().stream().map(Pack::open).toList()); // add tags and loot tables for unit tests LayeredRegistryAccess layers = RegistryLayer.createRegistryAccess(); + // Paper start - load registry here to ensure bukkit object registry are correctly delayed if needed + try { + Class.forName("org.bukkit.Registry"); + } catch (ClassNotFoundException ignored) {} + // Paper end - load registry here to ensure bukkit object registry are correctly delayed if needed layers = WorldLoader.loadAndReplaceLayer(resourceManager, layers, RegistryLayer.WORLDGEN, RegistryDataLoader.WORLDGEN_REGISTRIES); REGISTRY_CUSTOM = layers.compositeAccess().freeze(); // Register vanilla pack diff --git a/src/test/java/org/bukkit/support/DummyServer.java b/src/test/java/org/bukkit/support/DummyServer.java index 1acdf5bc439c073c1777c2c4f5743ae082f4a621..183d30e3d3d413b05c762f374a964498d6ffdec4 100644 --- a/src/test/java/org/bukkit/support/DummyServer.java +++ b/src/test/java/org/bukkit/support/DummyServer.java @@ -62,7 +62,7 @@ public final class DummyServer { when(instance.getTag(any(), any(), any())).then(mock -> { String registry = mock.getArgument(0); Class clazz = mock.getArgument(2); - MinecraftKey key = CraftNamespacedKey.toMinecraft(mock.getArgument(1)); + net.minecraft.resources.ResourceLocation key = CraftNamespacedKey.toMinecraft(mock.getArgument(1)); // Paper - address remapping issues switch (registry) { case org.bukkit.Tag.REGISTRY_BLOCKS -> { @@ -81,24 +81,32 @@ public final class DummyServer { } case org.bukkit.Tag.REGISTRY_FLUIDS -> { Preconditions.checkArgument(clazz == org.bukkit.Fluid.class, "Fluid namespace must have fluid type"); - TagKey fluidTagKey = TagKey.create(Registries.FLUID, key); + TagKey fluidTagKey = TagKey.create(Registries.FLUID, key); // Paper - address remapping issues if (BuiltInRegistries.FLUID.getTag(fluidTagKey).isPresent()) { return new CraftFluidTag(BuiltInRegistries.FLUID, fluidTagKey); } } case org.bukkit.Tag.REGISTRY_ENTITY_TYPES -> { Preconditions.checkArgument(clazz == org.bukkit.entity.EntityType.class, "Entity type namespace must have entity type"); - TagKey> entityTagKey = TagKey.create(Registries.ENTITY_TYPE, key); + TagKey> entityTagKey = TagKey.create(Registries.ENTITY_TYPE, key); // Paper - address remapping issues if (BuiltInRegistries.ENTITY_TYPE.getTag(entityTagKey).isPresent()) { return new CraftEntityTag(BuiltInRegistries.ENTITY_TYPE, entityTagKey); } } - default -> throw new IllegalArgumentException(); + default -> new io.papermc.paper.util.EmptyTag(); // Paper - testing additions } return null; }); + // Paper start - testing additions + final Thread currentThread = Thread.currentThread(); + when(instance.isPrimaryThread()).thenAnswer(ignored -> Thread.currentThread().equals(currentThread)); + + final org.bukkit.plugin.PluginManager pluginManager = new org.bukkit.plugin.SimplePluginManager(instance, new org.bukkit.command.SimpleCommandMap(instance)); + when(instance.getPluginManager()).thenReturn(pluginManager); + // paper end - testing additions + Bukkit.setServer(instance); } catch (Throwable t) { throw new Error(t);