Skip to content

Commit

Permalink
Revert "Revert #40601 and disable tests enabled by #40749"
Browse files Browse the repository at this point in the history
This reverts commit fc3988b.

It has some additional changes which re-revert part of #40601, to reintroduce removed guards to avoid cloning things like Quarkus runtime classes. Otherwise we get test failures.
  • Loading branch information
holly-cummins committed Jul 1, 2024
1 parent 88db83a commit 6b63320
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 352 deletions.
5 changes: 5 additions & 0 deletions bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4848,6 +4848,11 @@
<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,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;

Expand All @@ -22,7 +21,6 @@
* 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: 2 additions & 4 deletions test-framework/junit5/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@
<artifactId>quarkus-core</artifactId>
</dependency>
<dependency>
<groupId>com.thoughtworks.xstream</groupId>
<artifactId>xstream</artifactId>
<!-- Avoid adding this to the BOM -->
<version>1.4.20</version>
<groupId>org.jboss.marshalling</groupId>
<artifactId>jboss-marshalling</artifactId>
</dependency>

<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@
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.SerializationWithXStreamFallbackDeepClone;
import io.quarkus.test.junit.internal.NewSerializingDeepClone;
import io.quarkus.test.junit.internal.TestInfoImpl;

public class QuarkusTestExtension extends AbstractJvmQuarkusTestExtension
implements BeforeEachCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, AfterEachCallback,
Expand Down Expand Up @@ -351,7 +352,7 @@ private void shutdownHangDetection() {
}

private void populateDeepCloneField(StartupAction startupAction) {
deepClone = new SerializationWithXStreamFallbackDeepClone(startupAction.getClassLoader());
deepClone = new NewSerializingDeepClone(originalCl, startupAction.getClassLoader());
}

private void populateTestMethodInvokers(ClassLoader quarkusClassLoader) {
Expand Down Expand Up @@ -979,6 +980,7 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
} else if (clonePattern.matcher(theclass.getName()).matches()) {
cloneRequired = true;
} else {
// Don't clone things which are already loaded by the quarkus application's classloader side of the tree
try {
cloneRequired = runningQuarkusApplication.getClassLoader()
.loadClass(theclass.getName()) != theclass;
Expand All @@ -1002,6 +1004,7 @@ private Object runExtensionMethod(ReflectiveInvocationContext<Method> invocation
} else {
argumentsFromTccl.add(arg);
}

}

if (testMethodInvokerToUse != null) {
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package io.quarkus.test.junit.internal;

import java.io.IOException;
import java.io.Serializable;
import java.io.UncheckedIOException;
import java.lang.reflect.Method;
import java.util.Set;
import java.util.function.Supplier;

import org.jboss.marshalling.cloner.ClassCloner;
import org.jboss.marshalling.cloner.ClonerConfiguration;
import org.jboss.marshalling.cloner.ObjectCloner;
import org.jboss.marshalling.cloner.ObjectCloners;
import org.junit.jupiter.api.TestInfo;

/**
* A deep-clone implementation using JBoss Marshalling's fast object cloner.
*/
public final class NewSerializingDeepClone implements DeepClone {
private final ObjectCloner cloner;

public NewSerializingDeepClone(final ClassLoader sourceLoader, final ClassLoader targetLoader) {
ClonerConfiguration cc = new ClonerConfiguration();
cc.setSerializabilityChecker(clazz -> clazz != Object.class);
cc.setClassCloner(new ClassCloner() {
public Class<?> clone(final Class<?> original) {
if (isUncloneable(original)) {
return original;
}
try {
return targetLoader.loadClass(original.getName());
} catch (ClassNotFoundException ignored) {
return original;
}
}

public Class<?> cloneProxy(final Class<?> proxyClass) {
// not really supported
return proxyClass;
}
});
cc.setCloneTable(
(original, objectCloner, classCloner) -> {
if (EXTRA_IDENTITY_CLASSES.contains(original.getClass())) {
// avoid copying things that do not need to be copied
return original;
} else if (isUncloneable(original.getClass())) {
if (original instanceof Supplier<?> s) {
// sneaky
return (Supplier<?>) () -> clone(s.get());
} else {
return original;
}
} else if (original instanceof TestInfo info) {
// copy the test info correctly
return new TestInfoImpl(info.getDisplayName(), info.getTags(),
info.getTestClass().map(this::cloneClass),
info.getTestMethod().map(this::cloneMethod));
} else if (original == sourceLoader) {
return targetLoader;
}
// let the default cloner handle it
return null;
});
cloner = ObjectCloners.getSerializingObjectClonerFactory().createCloner(cc);
}

private static boolean isUncloneable(Class<?> clazz) {
return clazz.isHidden() && !Serializable.class.isAssignableFrom(clazz);
}

private Class<?> cloneClass(Class<?> clazz) {
try {
return (Class<?>) cloner.clone(clazz);
} catch (IOException | ClassNotFoundException e) {
return null;
}
}

private Method cloneMethod(Method method) {
try {
Class<?> declaring = (Class<?>) cloner.clone(method.getDeclaringClass());
Class<?>[] argTypes = (Class<?>[]) cloner.clone(method.getParameterTypes());
return declaring.getDeclaredMethod(method.getName(), argTypes);
} catch (Exception e) {
return null;
}
}

public Object clone(final Object objectToClone) {
try {
return cloner.clone(objectToClone);
} catch (IOException e) {
throw new UncheckedIOException(e);
} catch (ClassNotFoundException e) {
throw new IllegalStateException(e);
}
}

/**
* Classes which do not need to be cloned.
*/
private static final Set<Class<?>> EXTRA_IDENTITY_CLASSES = Set.of(
Object.class,
byte[].class,
short[].class,
int[].class,
long[].class,
char[].class,
boolean[].class,
float[].class,
double[].class);
}
Loading

0 comments on commit 6b63320

Please sign in to comment.