Skip to content

Commit

Permalink
Revert quarkusio#40601 and disable tests enabled by quarkusio#40749
Browse files Browse the repository at this point in the history
  • Loading branch information
holly-cummins committed May 30, 2024
1 parent bc50001 commit fc3988b
Show file tree
Hide file tree
Showing 13 changed files with 391 additions and 126 deletions.
5 changes: 0 additions & 5 deletions bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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() {
Expand Down
6 changes: 4 additions & 2 deletions test-framework/junit5/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package io.quarkus.test.junit.internal;
package io.quarkus.test.junit;

import java.lang.reflect.Method;
import java.util.Optional;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<>();
}
}
Original file line number Diff line number Diff line change
@@ -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<>();
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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<>();
}
}
Loading

0 comments on commit fc3988b

Please sign in to comment.