From b37f140b96aaf8d1bcb79a99606667ff3259b19f Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Thu, 19 May 2022 14:12:30 +0100 Subject: [PATCH 1/2] support quarkus test runner The quarkus test runner runs code within a non delegating classloader making mutants invisible. This change adds a transformer to catch this case. --- pitest-maven-verification/pom.xml | 8 +- .../src/test/java/org/pitest/PitMojoIT.java | 25 +++- .../src/test/resources/pit-quarkus/pom.xml | 115 +++++++++++++++++ .../example/controller/ExampleController.java | 23 ++++ .../com/example/service/ExampleService.java | 13 ++ .../src/main/resources/application.properties | 1 + .../com/example/ExampleControllerTest.java | 37 ++++++ .../java/com/example/ExampleServiceTest.java | 27 ++++ .../src/test/java/com/example/NormalTest.java | 13 ++ .../java/org/pitest/boot/HotSwapAgent.java | 1 - .../pitest/functional/prelude/Prelude.java | 11 -- .../CatchNewClassLoadersTransformer.java | 122 ++++++++++++++++++ .../pitest/mutationtest/execute/HotSwap.java | 3 + .../execute/MutationTestMinion.java | 3 + .../execute/MutationTestWorker.java | 8 +- 15 files changed, 395 insertions(+), 15 deletions(-) create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/controller/ExampleController.java create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/service/ExampleService.java create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/main/resources/application.properties create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleControllerTest.java create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleServiceTest.java create mode 100644 pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/NormalTest.java create mode 100644 pitest/src/main/java/org/pitest/mutationtest/execute/CatchNewClassLoadersTransformer.java diff --git a/pitest-maven-verification/pom.xml b/pitest-maven-verification/pom.xml index f8c81169e..654c6b30b 100644 --- a/pitest-maven-verification/pom.xml +++ b/pitest-maven-verification/pom.xml @@ -99,6 +99,12 @@ 1.7.2 test - + + org.pitest + pitest-entry + 1.0.0-SNAPSHOT + test-jar + test + diff --git a/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java b/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java index e2804d236..70f94b99e 100755 --- a/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java +++ b/pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java @@ -24,6 +24,7 @@ import org.junit.rules.TestName; import org.pitest.support.DirectoriesOnlyWalker; import org.pitest.testapi.execute.Pitest; +import org.pitest.util.CurrentRuntime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,6 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.*; import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; /** * @author Stefan Penndorf @@ -55,7 +57,7 @@ public class PitMojoIT { @Before public void beforeEachTest() { - LOGGER.info("running test '{}'", testName.getMethodName()); + LOGGER.info("running test '{}' with {} ", testName.getMethodName(), VERSION); startTime = System.currentTimeMillis(); } @@ -391,6 +393,27 @@ public void shouldWorkWithYatspec() throws Exception { assertThat(actual).doesNotContain("status='RUN_ERROR'"); } + @Test + // note this test depends on the junit5 plugin + public void shouldWorkWithQuarkus() throws Exception { + assumeTrue(CurrentRuntime.version() >= 11); + + File testDir = prepare("/pit-quarkus"); + verifier.executeGoal("test"); + verifier.executeGoal("org.pitest:pitest-maven:mutationCoverage"); + + String actual = readResults(testDir); + assertThat(actual) + .contains( + "" + + "ExampleController.java"); + + assertThat(actual) + .contains( + "ExampleController.java"); + + } + private static String readResults(File testDir) throws IOException { File mutationReport = new File(testDir.getAbsoluteFile() + File.separator diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml b/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml new file mode 100644 index 000000000..3e5acc47b --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml @@ -0,0 +1,115 @@ + + + 4.0.0 + com.example + pitest-quarlus + 1.0.0-SNAPSHOT + + 3.8.1 + false + 11 + UTF-8 + UTF-8 + quarkus-bom + io.quarkus.platform + 2.8.3.Final + 3.0.0-M5 + 0.14 + + + + + ${quarkus.platform.group-id} + ${quarkus.platform.artifact-id} + ${quarkus.platform.version} + pom + import + + + + + + io.quarkus + quarkus-resteasy-reactive + + + io.quarkus + quarkus-junit5 + test + + + io.quarkus + quarkus-junit5 + test + + + io.quarkus + quarkus-junit5-mockito + 2.8.1.Final + test + + + io.quarkus + quarkus-resteasy-reactive-jackson + + + + + + ${quarkus.platform.group-id} + quarkus-maven-plugin + ${quarkus.platform.version} + true + + + + build + generate-code + generate-code-tests + + + + + + maven-compiler-plugin + ${compiler-plugin.version} + + + -parameters + + + + + maven-surefire-plugin + ${surefire-plugin.version} + + + org.jboss.logmanager.LogManager + ${maven.home} + + + + + + org.pitest + pitest-maven + ${pit.version} + + true + XML + false + true + + + + org.pitest + pitest-junit5-plugin + ${pitest.junit5.version} + + + + + + + diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/controller/ExampleController.java b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/controller/ExampleController.java new file mode 100644 index 000000000..88552124c --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/controller/ExampleController.java @@ -0,0 +1,23 @@ +package com.example.controller; + +import com.example.service.ExampleService; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.ws.rs.*; +import javax.ws.rs.core.MediaType; + +@Path("/example") +@ApplicationScoped +public class ExampleController { + @Inject + ExampleService service; + + @POST + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + public boolean doStuff(String s) { + System.out.println("Won't die"); + return service.doStuff(s); + } +} diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/service/ExampleService.java b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/service/ExampleService.java new file mode 100644 index 000000000..7c0a6c76f --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/java/com/example/service/ExampleService.java @@ -0,0 +1,13 @@ +package com.example.service; + + +import javax.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class ExampleService { + + public boolean doStuff(String s) { + return s.equals("foo"); + } + +} diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/resources/application.properties b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/resources/application.properties new file mode 100644 index 000000000..44d34eadb --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/main/resources/application.properties @@ -0,0 +1 @@ +quarkus.rest-client."org.acme.rest.client.ExtensionsService".scope=javax.inject.ApplicationScoped # \ No newline at end of file diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleControllerTest.java b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleControllerTest.java new file mode 100644 index 000000000..61ebefe02 --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleControllerTest.java @@ -0,0 +1,37 @@ +package com.example; + +import com.example.controller.ExampleController; +import io.quarkus.test.junit.QuarkusTest; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import com.example.service.ExampleService; + +import javax.inject.Inject; +import io.quarkus.test.junit.mockito.InjectMock; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; + +@QuarkusTest +public class ExampleControllerTest { + @Inject + ExampleController controller; + + @InjectMock + ExampleService service; + + @Test + void doTrue() { + Mockito.when(service.doStuff(anyString())).thenReturn(true); + assertTrue(controller.doStuff("s")); + } + + @Test + void doFalse() { + Mockito.when(service.doStuff(anyString())).thenReturn(false); + assertFalse(controller.doStuff("s")); + } + + +} \ No newline at end of file diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleServiceTest.java b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleServiceTest.java new file mode 100644 index 000000000..2b02aade9 --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/ExampleServiceTest.java @@ -0,0 +1,27 @@ +package com.example; + +import io.quarkus.test.junit.QuarkusTest; +import com.example.service.ExampleService; +import org.junit.jupiter.api.Test; + +import javax.inject.Inject; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@QuarkusTest +public class ExampleServiceTest { + @Inject + ExampleService service; + + + @Test + void doTrue() { + assertTrue(service.doStuff("foo")); + } + + @Test + void doFalse() { + assertFalse(service.doStuff("notfoo")); + } +} diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/NormalTest.java b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/NormalTest.java new file mode 100644 index 000000000..8f242d3a5 --- /dev/null +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/src/test/java/com/example/NormalTest.java @@ -0,0 +1,13 @@ +package com.example; + +import org.junit.jupiter.api.Test; + +class NormalTest { + + @Test + void dd() { + long pid = ProcessHandle.current().pid(); + System.out.println("!!!!!!!!!!!!! " + pid); + } + +} diff --git a/pitest/src/main/java/org/pitest/boot/HotSwapAgent.java b/pitest/src/main/java/org/pitest/boot/HotSwapAgent.java index a81c3000b..4263c785d 100644 --- a/pitest/src/main/java/org/pitest/boot/HotSwapAgent.java +++ b/pitest/src/main/java/org/pitest/boot/HotSwapAgent.java @@ -44,7 +44,6 @@ public static boolean hotSwap(final Class mutateMe, final byte[] bytes) { // try { instrumentation.redefineClasses(definitions); - return true; } catch (final ClassNotFoundException | UnmodifiableClassException | VerifyError | InternalError e) { // swallow diff --git a/pitest/src/main/java/org/pitest/functional/prelude/Prelude.java b/pitest/src/main/java/org/pitest/functional/prelude/Prelude.java index 5b035bc31..7b2a3c41f 100644 --- a/pitest/src/main/java/org/pitest/functional/prelude/Prelude.java +++ b/pitest/src/main/java/org/pitest/functional/prelude/Prelude.java @@ -16,7 +16,6 @@ import java.io.PrintStream; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.function.Consumer; @@ -54,12 +53,6 @@ public static Or or(final Iterable> ps) { return new Or<>(ps); } - public static Consumer accumulateTo( - final Collection collection) { - return collection::add; - - } - public static Consumer putToMap(final Map map, final B value) { return key -> map.put(key, value); @@ -94,10 +87,6 @@ public static Consumer printlnWith(final T t) { return a -> System.out.println(t + " : " + a); } - public static Predicate isGreaterThan(final T value) { - return o -> o.longValue() > value.longValue(); - } - public static Function> asList(final Class type) { return Collections::singletonList; } diff --git a/pitest/src/main/java/org/pitest/mutationtest/execute/CatchNewClassLoadersTransformer.java b/pitest/src/main/java/org/pitest/mutationtest/execute/CatchNewClassLoadersTransformer.java new file mode 100644 index 000000000..5965a1684 --- /dev/null +++ b/pitest/src/main/java/org/pitest/mutationtest/execute/CatchNewClassLoadersTransformer.java @@ -0,0 +1,122 @@ +package org.pitest.mutationtest.execute; + +import org.pitest.boot.HotSwapAgent; +import org.pitest.classinfo.ClassName; + +import java.lang.instrument.ClassFileTransformer; +import java.security.ProtectionDomain; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Pitest mainly inserts mutants by calling Instrumentation.redefineClasses using + * the current context classloader. This works in most cases, but if a test + * runner switches the context classloader and loads classes into it, the + * mutant will not be active. + * + * The transformer acts as a net to catch these instances... but it all gets a bit messy. + * + * The classloaders created by the test may (or may not) delegate to their parent classloader, + * or may only do so partially. They may also transform classes which is problematic as we overwrite + * the byte array (modifying by rerunning the mutator might result in a different mutant for + * the same id if the input has changed). + * + * So, sometimes we want to transform the class, sometimes we don't. + * + * Not found a way to reliably work out which we need to do, so need to maintain + * a list. As mutants unexpectedly surviving are far more likely to be reported than + * ones unexpectedly killed, we list the loaders we should mutate for (currently just + * quarkus, but others likely exist). + * + */ +public class CatchNewClassLoadersTransformer implements ClassFileTransformer { + + private static byte[] currentMutant; + private static String currentClass; + + // The context classloader at the point pitest started. + // we do not want to transform classes from this as they are already handled + // by the primary mechanism + private static ClassLoader ignore; + + // Map of loaders we have transformed the current class in, so we can restore them + static final Map ORIGINAL_LOADER_CLASSES = new ConcurrentHashMap<>(); + + public static synchronized void setLoader(ClassLoader loader) { + ignore = loader; + } + + public static synchronized void setMutant(String className, byte[] mutant) { + String toRestore = currentClass; + currentMutant = mutant; + + // prevent transforming again when we restore if the same class is mutated twice + currentClass = null; + + restoreClasses(toRestore); + + currentClass = className; + } + + private static void restoreClasses(String toRestore) { + for (Map.Entry each : ORIGINAL_LOADER_CLASSES.entrySet()) { + final Class clazz = checkClassForLoader(each.getKey(), toRestore); + if (clazz != null) { + HotSwapAgent.hotSwap(clazz, each.getValue()); + } + } + ORIGINAL_LOADER_CLASSES.clear(); + } + + @Override + public byte[] transform(final ClassLoader loader, final String className, + final Class classBeingRedefined, + final ProtectionDomain protectionDomain, final byte[] classfileBuffer) { + if (loader == null || ignore == loader) { + return null; + } + + // Only loader identified so far where mutants must be inserted is Quarkus, but + // others likely exist. At least one loader (gwtmockito) results incorrectly + // killed mutants if we insert. + if (!loader.getClass().getName().startsWith("io.quarkus.bootstrap.classloading")) { + return null; + } + + if (className.equals(currentClass)) { + // skip if class already loaded + if (classBeingRedefined != null) { + return null; + } + + // avoid restoring an already mutated class + // Not clear if this situation is possible, but check left in + // out of fear. + if (!Arrays.equals(classfileBuffer, currentMutant)) { + ORIGINAL_LOADER_CLASSES.put(loader, classfileBuffer); + } + + return currentMutant; + } + return null; + } + + private static Class checkClassForLoader(ClassLoader loader, String className) { + try { + Class clazz = Class.forName(ClassName.fromString(className).asJavaName(), false, loader); + // loaded by parent + if (clazz.getClassLoader() != loader) { + return null; + } + return clazz; + } catch (ClassNotFoundException ex) { + // *think* this occurs as a result of a loader not delegating + // Does not occur when we only mutate for the Quarkus loader, but + // left in place for when/if the list expands + return null; + } + + } + +} diff --git a/pitest/src/main/java/org/pitest/mutationtest/execute/HotSwap.java b/pitest/src/main/java/org/pitest/mutationtest/execute/HotSwap.java index 09bb5fd39..85ca561d0 100644 --- a/pitest/src/main/java/org/pitest/mutationtest/execute/HotSwap.java +++ b/pitest/src/main/java/org/pitest/mutationtest/execute/HotSwap.java @@ -21,6 +21,9 @@ class HotSwap implements F3 { public Boolean apply(final ClassName clazzName, final ClassLoader loader, final byte[] b) { try { + + System.out.println("Hotswap loader " + loader); + restoreLastClass(this.byteSource, clazzName, loader); this.lastUsedLoader = loader; Class clazz = Class.forName(clazzName.asJavaName(), false, loader); diff --git a/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestMinion.java b/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestMinion.java index cab74d15f..8352f5979 100644 --- a/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestMinion.java +++ b/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestMinion.java @@ -79,6 +79,8 @@ public void run() { final ClassLoader loader = IsolationUtils.getContextClassLoader(); + CatchNewClassLoadersTransformer.setLoader(loader); + final ClassByteArraySource byteSource = new CachingByteArraySource(new ClassloaderByteArraySource( loader), CACHE_SIZE); @@ -126,6 +128,7 @@ public static void main(final String[] args) { LOG.fine(() -> "minion started"); enablePowerMockSupport(); + HotSwapAgent.addTransformer(new CatchNewClassLoadersTransformer()); final int port = Integer.parseInt(args[0]); diff --git a/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestWorker.java b/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestWorker.java index 538317ce9..71613b512 100644 --- a/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestWorker.java +++ b/pitest/src/main/java/org/pitest/mutationtest/execute/MutationTestWorker.java @@ -87,7 +87,7 @@ protected void run(final Collection range, final Reporter r, private void processMutation(final Reporter r, final TimeOutDecoratedTestSource testSource, - final MutationDetails mutationDetails) throws IOException { + final MutationDetails mutationDetails) { final MutationIdentifier mutationId = mutationDetails.getId(); final Mutant mutatedClass = this.mutater.getMutation(mutationId); @@ -141,12 +141,18 @@ private MutationStatusTestPair handleCoveredMutation( final Container c = createNewContainer(); final long t0 = System.currentTimeMillis(); + + // Some frameworks (eg quarkus) run tests in non delegating + // classloaders. Need to make sure these are transformed too + CatchNewClassLoadersTransformer.setMutant(mutatedClass.getDetails().getClassName().asInternalName(), mutatedClass.getBytes()); + if (this.hotswap.apply(mutationId.getClassName(), this.loader, mutatedClass.getBytes())) { if (DEBUG) { LOG.fine("replaced class with mutant in " + (System.currentTimeMillis() - t0) + " ms"); } + mutationDetected = doTestsDetectMutation(c, relevantTests); } else { LOG.warning("Mutation " + mutationId + " was not viable "); From 4bb31712fac49a603a4be0dcf28bd161d0c2d368 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 20 May 2022 11:01:43 +0100 Subject: [PATCH 2/2] increase timeout constant for flaky ci --- pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml b/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml index 3e5acc47b..802c92942 100644 --- a/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml +++ b/pitest-maven-verification/src/test/resources/pit-quarkus/pom.xml @@ -96,6 +96,7 @@ pitest-maven ${pit.version} + 10000 true XML false