Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run round-trip adventure codec tests with JSON, NBT, and Java ops. Use JavaOps for conversions. #10031

Merged
merged 7 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
244 changes: 242 additions & 2 deletions patches/server/0004-Test-changes.patch
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@ Subject: [PATCH] Test changes


diff --git a/build.gradle.kts b/build.gradle.kts
index e7fa464573909d4c3d649ebb5f40ef54055e09a8..2df1cae62cff433a7f3f55f561f70719bb6a745b 100644
index e7fa464573909d4c3d649ebb5f40ef54055e09a8..8d2aa99b4bd0d1c46c66274907a1f11d605a75da 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -57,6 +57,12 @@ tasks.compileJava {
@@ -22,6 +22,7 @@ dependencies {
testImplementation("org.junit.jupiter:junit-jupiter:5.10.0")
testImplementation("org.hamcrest:hamcrest:2.2")
testImplementation("org.mockito:mockito-core:5.5.0")
+ testImplementation("org.junit-pioneer:junit-pioneer:2.2.0") // Paper - CartesianTest
}

val craftbukkitPackageVersion = "1_20_R3" // Paper
@@ -57,6 +58,12 @@ tasks.compileJava {
options.setIncremental(false)
}

Expand Down Expand Up @@ -97,6 +105,238 @@ index 0000000000000000000000000000000000000000..6eb95a5e2534974c0e52e2b78b04e7c2
+ 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<Object>, AnnotationConsumer<MethodParameterSource> {
+ private MethodParameterSource source;
+
+ MethodParameterProvider() {
+ }
+
+ @Override
+ public void accept(final MethodParameterSource source) {
+ this.source = source;
+ }
+
+ @Override
+ public Stream<Object> provideArguments(ExtensionContext context, Parameter parameter) {
+ return this.provideArguments(context, this.source);
+ }
+
+ // Below is mostly copied from MethodArgumentsProvider
+
+ private static final Predicate<Method> isFactoryMethod = //
+ method -> isConvertibleToStream(method.getReturnType()) && !isTestMethod(method);
+
+ protected Stream<Object> 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<Method> isCandidate = candidate -> factoryMethodName.equals(candidate.getName())
+ && !testMethod.equals(candidate);
+ List<Method> candidates = ReflectionUtils.findMethods(clazz, isCandidate);
+
+ List<Method> 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/support/DummyServer.java b/src/test/java/org/bukkit/support/DummyServer.java
index b19d4f7d1fcb604b448a5084f6bfe56d47ab12b3..f3017525b0c2397fdc7ce0778add2e7b38e9e2ba 100644
--- a/src/test/java/org/bukkit/support/DummyServer.java
Expand Down
Loading
Loading