Revert test serializer changes to avoid regressing existing use cases

bom/application/pom.xml

@@ -4817,11 +4817,6 @@
                 <type>pom</type>
             </dependency>
 
-            <dependency>
-                <groupId>org.jboss.marshalling</groupId>
-                <artifactId>jboss-marshalling</artifactId>
-                <version>${jboss-marshalling.version}</version>
-            </dependency>
             <dependency>
                 <groupId>org.jboss.threads</groupId>
                 <artifactId>jboss-threads</artifactId>

integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/it/TestParameterDevModeIT.java

@@ -7,6 +7,7 @@
 
 import org.apache.maven.shared.invoker.MavenInvocationException;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
 
@@ -21,6 +22,7 @@
  * mvn install -Dit.test=DevMojoIT#methodName
  */
 @DisabledIfSystemProperty(named = "quarkus.test.native", matches = "true")
+@Disabled("Needs /~https://github.com/junit-team/junit5/pull/3820 and #40601")
 public class TestParameterDevModeIT extends RunAndCheckMojoTestBase {
 
     protected int getPort() {

integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/it/TestTemplateDevModeIT.java

@@ -7,7 +7,6 @@
 
 import org.apache.maven.shared.invoker.MavenInvocationException;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
 
@@ -21,7 +20,6 @@
  * <p>
  * mvn install -Dit.test=TestTemplateDevModeIT#methodName
  */
-@Disabled("NPE in JUnit stack; See discussion in /~https://github.com/quarkiverse/quarkiverse/issues/94, should be re-enabled when /~https://github.com/quarkusio/quarkus/pull/40751 is merged")
 @DisabledIfSystemProperty(named = "quarkus.test.native", matches = "true")
 public class TestTemplateDevModeIT extends RunAndCheckMojoTestBase {
 
@@ -50,8 +48,8 @@ protected void runAndCheck(boolean performCompile, String... options)
     @Test
     public void testThatTheTestsPassed() throws MavenInvocationException, IOException {
         //we also check continuous testing
-        String executionDir = "projects/project-using-test-template-from-extension-with-bytecode-changes-processed";
-        testDir = initProject("projects/project-using-test-template-from-extension-with-bytecode-changes", executionDir);
+        String executionDir = "projects/project-using-test-template-from-extension-processed";
+        testDir = initProject("projects/project-using-test-template-from-extension", executionDir);
         runAndCheck();
 
         ContinuousTestingMavenTestUtils testingTestUtils = new ContinuousTestingMavenTestUtils(getPort());

integration-tests/test-extension/tests/src/test/resources-filtered/projects/project-using-test-template-from-extension/src/main/resources/application.properties

@@ -1 +1,3 @@
-quarkus.test.continuous-testing=enabled
+quarkus.test.continuous-testing=enabled
+# this should not be needed, but something in the tests is setting this to 1234 and confusing the test framework, so set it here to match
+quarkus.http.non-application-root-path=1234

test-framework/junit5/pom.xml

@@ -49,8 +49,10 @@
             <artifactId>quarkus-core</artifactId>
         </dependency>
         <dependency>
-            <groupId>org.jboss.marshalling</groupId>
-            <artifactId>jboss-marshalling</artifactId>
+            <groupId>com.thoughtworks.xstream</groupId>
+            <artifactId>xstream</artifactId>
+            <!-- Avoid adding this to the BOM -->
+            <version>1.4.20</version>
         </dependency>
 
         <dependency>

test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java

@@ -40,6 +40,7 @@
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
@@ -51,6 +52,7 @@
 import org.jboss.jandex.Type;
 import org.jboss.logging.Logger;
 import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.AfterTestExecutionCallback;
@@ -104,7 +106,7 @@
 import io.quarkus.test.junit.callback.QuarkusTestContext;
 import io.quarkus.test.junit.callback.QuarkusTestMethodContext;
 import io.quarkus.test.junit.internal.DeepClone;
-import io.quarkus.test.junit.internal.NewSerializingDeepClone;
+import io.quarkus.test.junit.internal.SerializationWithXStreamFallbackDeepClone;
 
 public class QuarkusTestExtension extends AbstractJvmQuarkusTestExtension
         implements BeforeEachCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, AfterEachCallback,
@@ -353,7 +355,7 @@ private void shutdownHangDetection() {
     }
 
     private void populateDeepCloneField(StartupAction startupAction) {
-        deepClone = new NewSerializingDeepClone(originalCl, startupAction.getClassLoader());
+        deepClone = new SerializationWithXStreamFallbackDeepClone(startupAction.getClassLoader());
     }
 
     private void populateTestMethodInvokers(ClassLoader quarkusClassLoader) {
@@ -960,13 +962,49 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
             Parameter[] parameters = invocationContext.getExecutable().getParameters();
             for (int i = 0; i < originalArguments.size(); i++) {
                 Object arg = originalArguments.get(i);
+                boolean cloneRequired = false;
+                Object replacement = null;
                 Class<?> argClass = parameters[i].getType();
+                if (arg != null) {
+                    Class<?> theclass = argClass;
+                    while (theclass.isArray()) {
+                        theclass = theclass.getComponentType();
+                    }
+                    if (theclass.isPrimitive()) {
+                        cloneRequired = false;
+                    } else if (TestInfo.class.isAssignableFrom(theclass)) {
+                        TestInfo info = (TestInfo) arg;
+                        Method newTestMethod = info.getTestMethod().isPresent()
+                                ? determineTCCLExtensionMethod(info.getTestMethod().get(), testClassFromTCCL)
+                                : null;
+                        replacement = new TestInfoImpl(info.getDisplayName(), info.getTags(),
+                                Optional.of(testClassFromTCCL),
+                                Optional.ofNullable(newTestMethod));
+                    } else if (clonePattern.matcher(theclass.getName()).matches()) {
+                        cloneRequired = true;
+                    } else {
+                        try {
+                            cloneRequired = runningQuarkusApplication.getClassLoader()
+                                    .loadClass(theclass.getName()) != theclass;
+                        } catch (ClassNotFoundException e) {
+                            if (arg instanceof Supplier) {
+                                cloneRequired = true;
+                            } else {
+                                throw e;
+                            }
+                        }
+                    }
+                }
 
-                if (testMethodInvokerToUse != null) {
+                if (replacement != null) {
+                    argumentsFromTccl.add(replacement);
+                } else if (cloneRequired) {
+                    argumentsFromTccl.add(deepClone.clone(arg));
+                } else if (testMethodInvokerToUse != null) {
                     argumentsFromTccl.add(testMethodInvokerToUse.getClass().getMethod("methodParamInstance", String.class)
                             .invoke(testMethodInvokerToUse, argClass.getName()));
                 } else {
-                    argumentsFromTccl.add(deepClone.clone(arg));
+                    argumentsFromTccl.add(arg);
                 }
             }
 
@@ -976,7 +1014,7 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
                         .invoke(testMethodInvokerToUse, effectiveTestInstance, newMethod, argumentsFromTccl,
                                 extensionContext.getRequiredTestClass().getName());
             } else {
-                return newMethod.invoke(effectiveTestInstance, argumentsFromTccl.toArray(Object[]::new));
+                return newMethod.invoke(effectiveTestInstance, argumentsFromTccl.toArray(new Object[0]));
             }
 
         } catch (InvocationTargetException e) {

test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/TestInfoImpl.java → test-framework/junit5/src/main/java/io/quarkus/test/junit/TestInfoImpl.java

@@ -1,4 +1,4 @@
-package io.quarkus.test.junit.internal;
+package io.quarkus.test.junit;
 
 import java.lang.reflect.Method;
 import java.util.Optional;

test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java

@@ -0,0 +1,63 @@
+package io.quarkus.test.junit.internal;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+
+import com.thoughtworks.xstream.converters.collections.CollectionConverter;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+/**
+ * A custom List converter that always uses ArrayList for unmarshalling.
+ * This is probably not semantically correct 100% of the time, but it's likely fine
+ * for all the cases where we are using marshalling / unmarshalling.
+ *
+ * The reason for doing this is to avoid XStream causing illegal access issues
+ * for internal JDK lists
+ */
+public class CustomListConverter extends CollectionConverter {
+
+    // if we wanted to be 100% sure, we'd list all the List.of methods, but I think it's pretty safe to say
+    // that the JDK won't add custom implementations for the other classes
+
+    private final Predicate<String> supported = new Predicate<String>() {
+
+        private final Set<String> JDK_LIST_CLASS_NAMES = Set.of(
+                List.of().getClass().getName(),
+                List.of(Integer.MAX_VALUE).getClass().getName(),
+                Arrays.asList(Integer.MAX_VALUE).getClass().getName(),
+                Collections.unmodifiableList(List.of()).getClass().getName(),
+                Collections.emptyList().getClass().getName(),
+                List.of(Integer.MIN_VALUE, Integer.MAX_VALUE).subList(0, 1).getClass().getName());
+
+        @Override
+        public boolean test(String className) {
+            return JDK_LIST_CLASS_NAMES.contains(className);
+        }
+    }.or(new Predicate<>() {
+
+        private static final String GUAVA_LISTS_PACKAGE = "com.google.common.collect.Lists";
+
+        @Override
+        public boolean test(String className) {
+            return className.startsWith(GUAVA_LISTS_PACKAGE);
+        }
+    });
+
+    public CustomListConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(Class type) {
+        return (type != null) && supported.test(type.getName());
+    }
+
+    @Override
+    protected Object createCollection(Class type) {
+        return new ArrayList<>();
+    }
+}

test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapConverter.java

@@ -0,0 +1,41 @@
+package io.quarkus.test.junit.internal;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import com.thoughtworks.xstream.converters.collections.MapConverter;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+/**
+ * A custom Map converter that always uses HashMap for unmarshalling.
+ * This is probably not semantically correct 100% of the time, but it's likely fine
+ * for all the cases where we are using marshalling / unmarshalling.
+ *
+ * The reason for doing this is to avoid XStream causing illegal access issues
+ * for internal JDK maps
+ */
+public class CustomMapConverter extends MapConverter {
+
+    // if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
+    // that the JDK won't add custom implementations for the other classes
+    private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
+            Map.of().getClass().getName(),
+            Map.of(Integer.MAX_VALUE, Integer.MAX_VALUE).getClass().getName(),
+            Collections.emptyMap().getClass().getName());
+
+    public CustomMapConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(Class type) {
+        return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
+    }
+
+    @Override
+    protected Object createCollection(Class type) {
+        return new HashMap<>();
+    }
+}

test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapEntryConverter.java

@@ -0,0 +1,55 @@
+package io.quarkus.test.junit.internal;
+
+import java.util.AbstractMap;
+import java.util.Map;
+import java.util.Set;
+
+import com.thoughtworks.xstream.converters.MarshallingContext;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.converters.collections.MapConverter;
+import com.thoughtworks.xstream.io.HierarchicalStreamReader;
+import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+/**
+ * A custom Map.Entry converter that always uses AbstractMap.SimpleEntry for unmarshalling.
+ * This is probably not semantically correct 100% of the time, but it's likely fine
+ * for all the cases where we are using marshalling / unmarshalling.
+ *
+ * The reason for doing this is to avoid XStream causing illegal access issues
+ * for internal JDK types
+ */
+@SuppressWarnings({ "rawtypes", "unchecked" })
+public class CustomMapEntryConverter extends MapConverter {
+
+    private final Set<String> SUPPORTED_CLASS_NAMES = Set
+            .of(Map.entry(Integer.MAX_VALUE, Integer.MAX_VALUE).getClass().getName());
+
+    public CustomMapEntryConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(Class type) {
+        return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
+    }
+
+    @Override
+    public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
+        var entryName = mapper().serializedClass(Map.Entry.class);
+        var entry = (Map.Entry) source;
+        writer.startNode(entryName);
+        writeCompleteItem(entry.getKey(), context, writer);
+        writeCompleteItem(entry.getValue(), context, writer);
+        writer.endNode();
+    }
+
+    @Override
+    public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
+        reader.moveDown();
+        var key = readCompleteItem(reader, context, null);
+        var value = readCompleteItem(reader, context, null);
+        reader.moveUp();
+        return new AbstractMap.SimpleEntry(key, value);
+    }
+}

test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java

@@ -0,0 +1,40 @@
+package io.quarkus.test.junit.internal;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import com.thoughtworks.xstream.converters.collections.CollectionConverter;
+import com.thoughtworks.xstream.mapper.Mapper;
+
+/**
+ * A custom Set converter that always uses HashSet for unmarshalling.
+ * This is probably not semantically correct 100% of the time, but it's likely fine
+ * for all the cases where we are using marshalling / unmarshalling.
+ *
+ * The reason for doing this is to avoid XStream causing illegal access issues
+ * for internal JDK sets
+ */
+public class CustomSetConverter extends CollectionConverter {
+
+    // if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
+    // that the JDK won't add custom implementations for the other classes
+    private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
+            Set.of().getClass().getName(),
+            Set.of(Integer.MAX_VALUE).getClass().getName(),
+            Collections.emptySet().getClass().getName());
+
+    public CustomSetConverter(Mapper mapper) {
+        super(mapper);
+    }
+
+    @Override
+    public boolean canConvert(Class type) {
+        return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
+    }
+
+    @Override
+    protected Object createCollection(Class type) {
+        return new HashSet<>();
+    }
+}