Skip to content

Commit bebe255

Browse files
holly-cumminsdanielsoro
holly-cummins
authored and
danielsoro
committed
Revert quarkusio#40601 and disable tests enabled by quarkusio#40749 
1 parent 57936a3 commit bebe255

File tree

13 files changed

+391
-126
lines changed

13 files changed

+391
-126
lines changed

‎bom/application/pom.xml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4817,11 +4817,6 @@
48174817
<type>pom</type>
48184818
</dependency>
48194819

4820-
<dependency>
4821-
<groupId>org.jboss.marshalling</groupId>
4822-
<artifactId>jboss-marshalling</artifactId>
4823-
<version>${jboss-marshalling.version}</version>
4824-
</dependency>
48254820
<dependency>
48264821
<groupId>org.jboss.threads</groupId>
48274822
<artifactId>jboss-threads</artifactId>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.apache.maven.shared.invoker.MavenInvocationException;
99
import org.junit.jupiter.api.Assertions;
10+
import org.junit.jupiter.api.Disabled;
1011
import org.junit.jupiter.api.Test;
1112
import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
1213

@@ -21,6 +22,7 @@
2122
* mvn install -Dit.test=DevMojoIT#methodName
2223
*/
2324
@DisabledIfSystemProperty(named = "quarkus.test.native", matches = "true")
25+
@Disabled("Needs https://github.com/junit-team/junit5/pull/3820 and #40601")
2426
public class TestParameterDevModeIT extends RunAndCheckMojoTestBase {
2527

2628
protected int getPort() {

‎test-framework/junit5/pom.xml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@
4949
<artifactId>quarkus-core</artifactId>
5050
</dependency>
5151
<dependency>
52-
<groupId>org.jboss.marshalling</groupId>
53-
<artifactId>jboss-marshalling</artifactId>
52+
<groupId>com.thoughtworks.xstream</groupId>
53+
<artifactId>xstream</artifactId>
54+
<!-- Avoid adding this to the BOM -->
55+
<version>1.4.20</version>
5456
</dependency>
5557

5658
<dependency>

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

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.function.Consumer;
4141
import java.util.function.Function;
4242
import java.util.function.Predicate;
43+
import java.util.function.Supplier;
4344
import java.util.regex.Pattern;
4445

4546
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
@@ -51,6 +52,7 @@
5152
import org.jboss.jandex.Type;
5253
import org.jboss.logging.Logger;
5354
import org.junit.jupiter.api.Nested;
55+
import org.junit.jupiter.api.TestInfo;
5456
import org.junit.jupiter.api.extension.AfterAllCallback;
5557
import org.junit.jupiter.api.extension.AfterEachCallback;
5658
import org.junit.jupiter.api.extension.AfterTestExecutionCallback;
@@ -104,7 +106,7 @@
104106
import io.quarkus.test.junit.callback.QuarkusTestContext;
105107
import io.quarkus.test.junit.callback.QuarkusTestMethodContext;
106108
import io.quarkus.test.junit.internal.DeepClone;
107-
import io.quarkus.test.junit.internal.NewSerializingDeepClone;
109+
import io.quarkus.test.junit.internal.SerializationWithXStreamFallbackDeepClone;
108110

109111
public class QuarkusTestExtension extends AbstractJvmQuarkusTestExtension
110112
implements BeforeEachCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, AfterEachCallback,
@@ -353,7 +355,7 @@ private void shutdownHangDetection() {
353355
}
354356

355357
private void populateDeepCloneField(StartupAction startupAction) {
356-
deepClone = new NewSerializingDeepClone(originalCl, startupAction.getClassLoader());
358+
deepClone = new SerializationWithXStreamFallbackDeepClone(startupAction.getClassLoader());
357359
}
358360

359361
private void populateTestMethodInvokers(ClassLoader quarkusClassLoader) {
@@ -960,13 +962,49 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
960962
Parameter[] parameters = invocationContext.getExecutable().getParameters();
961963
for (int i = 0; i < originalArguments.size(); i++) {
962964
Object arg = originalArguments.get(i);
965+
boolean cloneRequired = false;
966+
Object replacement = null;
963967
Class<?> argClass = parameters[i].getType();
968+
if (arg != null) {
969+
Class<?> theclass = argClass;
970+
while (theclass.isArray()) {
971+
theclass = theclass.getComponentType();
972+
}
973+
if (theclass.isPrimitive()) {
974+
cloneRequired = false;
975+
} else if (TestInfo.class.isAssignableFrom(theclass)) {
976+
TestInfo info = (TestInfo) arg;
977+
Method newTestMethod = info.getTestMethod().isPresent()
978+
? determineTCCLExtensionMethod(info.getTestMethod().get(), testClassFromTCCL)
979+
: null;
980+
replacement = new TestInfoImpl(info.getDisplayName(), info.getTags(),
981+
Optional.of(testClassFromTCCL),
982+
Optional.ofNullable(newTestMethod));
983+
} else if (clonePattern.matcher(theclass.getName()).matches()) {
984+
cloneRequired = true;
985+
} else {
986+
try {
987+
cloneRequired = runningQuarkusApplication.getClassLoader()
988+
.loadClass(theclass.getName()) != theclass;
989+
} catch (ClassNotFoundException e) {
990+
if (arg instanceof Supplier) {
991+
cloneRequired = true;
992+
} else {
993+
throw e;
994+
}
995+
}
996+
}
997+
}
964998

965-
if (testMethodInvokerToUse != null) {
999+
if (replacement != null) {
1000+
argumentsFromTccl.add(replacement);
1001+
} else if (cloneRequired) {
1002+
argumentsFromTccl.add(deepClone.clone(arg));
1003+
} else if (testMethodInvokerToUse != null) {
9661004
argumentsFromTccl.add(testMethodInvokerToUse.getClass().getMethod("methodParamInstance", String.class)
9671005
.invoke(testMethodInvokerToUse, argClass.getName()));
9681006
} else {
969-
argumentsFromTccl.add(deepClone.clone(arg));
1007+
argumentsFromTccl.add(arg);
9701008
}
9711009
}
9721010

@@ -976,7 +1014,7 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
9761014
.invoke(testMethodInvokerToUse, effectiveTestInstance, newMethod, argumentsFromTccl,
9771015
extensionContext.getRequiredTestClass().getName());
9781016
} else {
979-
return newMethod.invoke(effectiveTestInstance, argumentsFromTccl.toArray(Object[]::new));
1017+
return newMethod.invoke(effectiveTestInstance, argumentsFromTccl.toArray(new Object[0]));
9801018
}
9811019

9821020
} catch (InvocationTargetException e) {

‎test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/TestInfoImpl.java renamed to ‎test-framework/junit5/src/main/java/io/quarkus/test/junit/TestInfoImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package io.quarkus.test.junit.internal;
1+
package io.quarkus.test.junit;
22

33
import java.lang.reflect.Method;
44
import java.util.Optional;

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package io.quarkus.test.junit.internal;
2+
3+
import java.util.ArrayList;
4+
import java.util.Arrays;
5+
import java.util.Collections;
6+
import java.util.List;
7+
import java.util.Set;
8+
import java.util.function.Predicate;
9+
10+
import com.thoughtworks.xstream.converters.collections.CollectionConverter;
11+
import com.thoughtworks.xstream.mapper.Mapper;
12+
13+
/**
14+
* A custom List converter that always uses ArrayList for unmarshalling.
15+
* This is probably not semantically correct 100% of the time, but it's likely fine
16+
* for all the cases where we are using marshalling / unmarshalling.
17+
*
18+
* The reason for doing this is to avoid XStream causing illegal access issues
19+
* for internal JDK lists
20+
*/
21+
public class CustomListConverter extends CollectionConverter {
22+
23+
// if we wanted to be 100% sure, we'd list all the List.of methods, but I think it's pretty safe to say
24+
// that the JDK won't add custom implementations for the other classes
25+
26+
private final Predicate<String> supported = new Predicate<String>() {
27+
28+
private final Set<String> JDK_LIST_CLASS_NAMES = Set.of(
29+
List.of().getClass().getName(),
30+
List.of(Integer.MAX_VALUE).getClass().getName(),
31+
Arrays.asList(Integer.MAX_VALUE).getClass().getName(),
32+
Collections.unmodifiableList(List.of()).getClass().getName(),
33+
Collections.emptyList().getClass().getName(),
34+
List.of(Integer.MIN_VALUE, Integer.MAX_VALUE).subList(0, 1).getClass().getName());
35+
36+
@Override
37+
public boolean test(String className) {
38+
return JDK_LIST_CLASS_NAMES.contains(className);
39+
}
40+
}.or(new Predicate<>() {
41+
42+
private static final String GUAVA_LISTS_PACKAGE = "com.google.common.collect.Lists";
43+
44+
@Override
45+
public boolean test(String className) {
46+
return className.startsWith(GUAVA_LISTS_PACKAGE);
47+
}
48+
});
49+
50+
public CustomListConverter(Mapper mapper) {
51+
super(mapper);
52+
}
53+
54+
@Override
55+
public boolean canConvert(Class type) {
56+
return (type != null) && supported.test(type.getName());
57+
}
58+
59+
@Override
60+
protected Object createCollection(Class type) {
61+
return new ArrayList<>();
62+
}
63+
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package io.quarkus.test.junit.internal;
2+
3+
import java.util.Collections;
4+
import java.util.HashMap;
5+
import java.util.Map;
6+
import java.util.Set;
7+
8+
import com.thoughtworks.xstream.converters.collections.MapConverter;
9+
import com.thoughtworks.xstream.mapper.Mapper;
10+
11+
/**
12+
* A custom Map converter that always uses HashMap for unmarshalling.
13+
* This is probably not semantically correct 100% of the time, but it's likely fine
14+
* for all the cases where we are using marshalling / unmarshalling.
15+
*
16+
* The reason for doing this is to avoid XStream causing illegal access issues
17+
* for internal JDK maps
18+
*/
19+
public class CustomMapConverter extends MapConverter {
20+
21+
// if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
22+
// that the JDK won't add custom implementations for the other classes
23+
private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
24+
Map.of().getClass().getName(),
25+
Map.of(Integer.MAX_VALUE, Integer.MAX_VALUE).getClass().getName(),
26+
Collections.emptyMap().getClass().getName());
27+
28+
public CustomMapConverter(Mapper mapper) {
29+
super(mapper);
30+
}
31+
32+
@Override
33+
public boolean canConvert(Class type) {
34+
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
35+
}
36+
37+
@Override
38+
protected Object createCollection(Class type) {
39+
return new HashMap<>();
40+
}
41+
}

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package io.quarkus.test.junit.internal;
2+
3+
import java.util.AbstractMap;
4+
import java.util.Map;
5+
import java.util.Set;
6+
7+
import com.thoughtworks.xstream.converters.MarshallingContext;
8+
import com.thoughtworks.xstream.converters.UnmarshallingContext;
9+
import com.thoughtworks.xstream.converters.collections.MapConverter;
10+
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
11+
import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
12+
import com.thoughtworks.xstream.mapper.Mapper;
13+
14+
/**
15+
* A custom Map.Entry converter that always uses AbstractMap.SimpleEntry for unmarshalling.
16+
* This is probably not semantically correct 100% of the time, but it's likely fine
17+
* for all the cases where we are using marshalling / unmarshalling.
18+
*
19+
* The reason for doing this is to avoid XStream causing illegal access issues
20+
* for internal JDK types
21+
*/
22+
@SuppressWarnings({ "rawtypes", "unchecked" })
23+
public class CustomMapEntryConverter extends MapConverter {
24+
25+
private final Set<String> SUPPORTED_CLASS_NAMES = Set
26+
.of(Map.entry(Integer.MAX_VALUE, Integer.MAX_VALUE).getClass().getName());
27+
28+
public CustomMapEntryConverter(Mapper mapper) {
29+
super(mapper);
30+
}
31+
32+
@Override
33+
public boolean canConvert(Class type) {
34+
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
35+
}
36+
37+
@Override
38+
public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
39+
var entryName = mapper().serializedClass(Map.Entry.class);
40+
var entry = (Map.Entry) source;
41+
writer.startNode(entryName);
42+
writeCompleteItem(entry.getKey(), context, writer);
43+
writeCompleteItem(entry.getValue(), context, writer);
44+
writer.endNode();
45+
}
46+
47+
@Override
48+
public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
49+
reader.moveDown();
50+
var key = readCompleteItem(reader, context, null);
51+
var value = readCompleteItem(reader, context, null);
52+
reader.moveUp();
53+
return new AbstractMap.SimpleEntry(key, value);
54+
}
55+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package io.quarkus.test.junit.internal;
2+
3+
import java.util.Collections;
4+
import java.util.HashSet;
5+
import java.util.Set;
6+
7+
import com.thoughtworks.xstream.converters.collections.CollectionConverter;
8+
import com.thoughtworks.xstream.mapper.Mapper;
9+
10+
/**
11+
* A custom Set converter that always uses HashSet for unmarshalling.
12+
* This is probably not semantically correct 100% of the time, but it's likely fine
13+
* for all the cases where we are using marshalling / unmarshalling.
14+
*
15+
* The reason for doing this is to avoid XStream causing illegal access issues
16+
* for internal JDK sets
17+
*/
18+
public class CustomSetConverter extends CollectionConverter {
19+
20+
// if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
21+
// that the JDK won't add custom implementations for the other classes
22+
private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
23+
Set.of().getClass().getName(),
24+
Set.of(Integer.MAX_VALUE).getClass().getName(),
25+
Collections.emptySet().getClass().getName());
26+
27+
public CustomSetConverter(Mapper mapper) {
28+
super(mapper);
29+
}
30+
31+
@Override
32+
public boolean canConvert(Class type) {
33+
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
34+
}
35+
36+
@Override
37+
protected Object createCollection(Class type) {
38+
return new HashSet<>();
39+
}
40+
}

0 commit comments

Comments
 (0)