diff --git a/pom.xml b/pom.xml index 6a9876f..35534aa 100755 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ 1.9.2 5.9.2 2.7.6 - 1.9.0 + 1.13.2 5.0.0 2.3-groovy-4.0 4.0.11 diff --git a/src/main/java/org/pitest/junit5/JUnit5TestPluginFactory.java b/src/main/java/org/pitest/junit5/JUnit5TestPluginFactory.java index 2cb3a19..fa8dc6c 100755 --- a/src/main/java/org/pitest/junit5/JUnit5TestPluginFactory.java +++ b/src/main/java/org/pitest/junit5/JUnit5TestPluginFactory.java @@ -27,11 +27,10 @@ public class JUnit5TestPluginFactory implements TestPluginFactory { @Override - public Configuration createTestFrameworkConfiguration(TestGroupConfig config, - ClassByteArraySource source, + public Configuration createTestFrameworkConfiguration(TestGroupConfig config, + ClassByteArraySource source, Collection excludedRunners, Collection includedTestMethods) { - System.setProperty("junit.jupiter.execution.parallel.enabled", "false"); return new JUnit5Configuration(config, includedTestMethods); } diff --git a/src/main/java/org/pitest/junit5/JUnit5TestUnitFinder.java b/src/main/java/org/pitest/junit5/JUnit5TestUnitFinder.java index 8d9b79f..5deae87 100755 --- a/src/main/java/org/pitest/junit5/JUnit5TestUnitFinder.java +++ b/src/main/java/org/pitest/junit5/JUnit5TestUnitFinder.java @@ -16,16 +16,24 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.StringJoiner; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReentrantLock; import static java.util.Collections.emptyList; -import static java.util.Collections.synchronizedList; +import static java.util.Collections.synchronizedSet; import static java.util.Collections.unmodifiableList; import static java.util.stream.Collectors.toList; import org.junit.platform.commons.PreconditionViolationException; import org.junit.platform.engine.Filter; import org.junit.platform.engine.TestExecutionResult; +import org.junit.platform.engine.UniqueId; import org.junit.platform.engine.discovery.DiscoverySelectors; import org.junit.platform.engine.support.descriptor.MethodSource; import org.junit.platform.launcher.Launcher; @@ -35,6 +43,7 @@ import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; import org.junit.platform.launcher.core.LauncherFactory; import org.pitest.testapi.Description; +import org.pitest.testapi.NullExecutionListener; import org.pitest.testapi.TestGroupConfig; import org.pitest.testapi.TestUnit; import org.pitest.testapi.TestUnitExecutionListener; @@ -46,12 +55,27 @@ */ public class JUnit5TestUnitFinder implements TestUnitFinder { + /** + * The test group config. + */ private final TestGroupConfig testGroupConfig; + /** + * Test methods that should be included. + */ private final Collection includedTestMethods; + /** + * The JUnit platform launcher used to execute tests. + */ private final Launcher launcher; + /** + * Constructs a new JUnit 5 test unit finder. + * + * @param testGroupConfig the test group config + * @param includedTestMethods test methods that should be included + */ public JUnit5TestUnitFinder(TestGroupConfig testGroupConfig, Collection includedTestMethods) { this.testGroupConfig = testGroupConfig; this.includedTestMethods = includedTestMethods; @@ -64,7 +88,7 @@ public List findTestUnits(Class clazz, TestUnitExecutionListener ex return emptyList(); } - List filters = new ArrayList<>(2); + List> filters = new ArrayList<>(2); try { List excludedGroups = testGroupConfig.getExcludedGroups(); if(excludedGroups != null && !excludedGroups.isEmpty()) { @@ -84,7 +108,7 @@ public List findTestUnits(Class clazz, TestUnitExecutionListener ex launcher.execute(LauncherDiscoveryRequestBuilder .request() .selectors(DiscoverySelectors.selectClass(clazz)) - .filters(filters.toArray(new Filter[filters.size()])) + .filters(filters.toArray(new Filter[0])) .build(), listener); return listener.getIdentifiers() @@ -93,17 +117,92 @@ public List findTestUnits(Class clazz, TestUnitExecutionListener ex .collect(toList()); } + /** + * A test execution listener that listens for test identifiers, supporting atomic test units + * and notifying the supplied test unit execution listener so that for example coverage can + * be recorded right away during discovery phase already. + */ private class TestIdentifierListener implements TestExecutionListener { + /** + * The test class as given to the test unit finder for forwarding to the test unit execution listener. + */ private final Class testClass; - private final TestUnitExecutionListener l; - private final List identifiers = synchronizedList(new ArrayList<>()); - public TestIdentifierListener(Class testClass, TestUnitExecutionListener l) { + /** + * The test unit execution listener, that for example is used for coverage recording per test. + */ + private final TestUnitExecutionListener testUnitExecutionListener; + + /** + * The collected test identifiers. + */ + private final Set identifiers = synchronizedSet(new LinkedHashSet<>()); + + /** + * Whether to serialize test execution, because we are during coverage recording which is + * done through static fields and thus does not support parallel test execution. + */ + private final boolean serializeExecution; + + /** + * A map that holds the locks that child tests of locked parent tests should use. + * For example parallel data-driven Spock features start the feature execution which is CONTAINER_AND_TEST, + * then wait for the parallel iteration executions to be finished which are TEST, + * then finish the feature execution. + * Due to that we cannot lock the iteration executions on the same lock as the feature executions, + * as the feature execution is around all the subordinate iteration executions. + * + *

This logic will of course break if there is some test engine that does strange setups like + * having CONTAINER_AND_TEST with child CONTAINER that have child TEST and similar. + * If those engines happen to be used, tests will start to deadlock, as the grand-child test + * would not find the parent serializer and thus use the root serializer on which the grand-parent + * CONTAINER_AND_TEST already locks. + * + *

This setup would probably not make much sense, so should not be taken into account + * unless such an engine actually pops up. If it does and someone tries to use it with PIT, + * the logic should maybe be made more sophisticated like remembering the parent-child relationships + * to be able to find the grand-parent serializer which is not possible stateless, because we are + * only able to get the parent identifier directly, but not further up stateless. + */ + private final Map> parentCoverageSerializers = new ConcurrentHashMap<>(); + + /** + * A map that holds the actual lock used for a specific test to be able to easily and safely unlock + * without the need to recalculate which lock to use. + */ + private final Map coverageSerializers = new ConcurrentHashMap<>(); + + /** + * The root coverage serializer to be used for the top-most recorded tests. + */ + private final ReentrantLock rootCoverageSerializer = new ReentrantLock(); + + /** + * Constructs a new test identifier listener. + * + * @param testClass the test class as given to the test unit finder for forwarding to the result collector + * @param testUnitExecutionListener the test unit execution listener to notify during test execution + */ + public TestIdentifierListener(Class testClass, TestUnitExecutionListener testUnitExecutionListener) { this.testClass = testClass; - this.l = l; + this.testUnitExecutionListener = testUnitExecutionListener; + // PIT gives a coverage recording listener here during coverage recording + // At the later stage during minion hunting a NullExecutionListener is given + // as PIT is only interested in the resulting list of identifiers. + // Serialization of test execution is only necessary during coverage calculation + // currently. To be on the safe side serialize test execution for any listener + // type except listener types where we know tests can run in parallel safely, + // i.e. currently the NullExecutionListener which is the only other one besides + // the coverage recording listener. + serializeExecution = !(testUnitExecutionListener instanceof NullExecutionListener); } - List getIdentifiers() { + /** + * Returns the collected test identifiers. + * + * @return the collected test identifiers + */ + private List getIdentifiers() { return unmodifiableList(new ArrayList<>(identifiers)); } @@ -117,25 +216,59 @@ public void executionStarted(TestIdentifier testIdentifier) { && !includedTestMethods.contains(((MethodSource)testIdentifier.getSource().get()).getMethodName())) { return; } - l.executionStarted(new Description(testIdentifier.getUniqueId(), testClass)); + + if (serializeExecution) { + coverageSerializers.compute(testIdentifier.getUniqueIdObject(), (uniqueId, lock) -> { + if (lock != null) { + throw new AssertionError("No lock should be present"); + } + + // find the serializer to lock the test on + // if there is a parent test locked, use the lock for its children if not, + // use the root serializer + return testIdentifier + .getParentIdObject() + .map(parentCoverageSerializers::get) + .map(lockRef -> lockRef.updateAndGet(parentLock -> + parentLock == null ? new ReentrantLock() : parentLock)) + .orElse(rootCoverageSerializer); + }).lock(); + // record a potential serializer for child tests to lock on + parentCoverageSerializers.put(testIdentifier.getUniqueIdObject(), new AtomicReference<>()); + } + + testUnitExecutionListener.executionStarted(new Description(testIdentifier.getUniqueId(), testClass), true); identifiers.add(testIdentifier); } } - @Override public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) { // Classes with failing BeforeAlls never start execution and identify as 'containers' not 'tests' if (testExecutionResult.getStatus() == TestExecutionResult.Status.FAILED) { - if (!identifiers.contains(testIdentifier)) { - identifiers.add(testIdentifier); - } - l.executionFinished(new Description(testIdentifier.getUniqueId(), testClass), false); + identifiers.add(testIdentifier); + testUnitExecutionListener.executionFinished(new Description(testIdentifier.getUniqueId(), testClass), false); } else if (testIdentifier.isTest()) { - l.executionFinished(new Description(testIdentifier.getUniqueId(), testClass), true); + testUnitExecutionListener.executionFinished(new Description(testIdentifier.getUniqueId(), testClass), true); } - } + if (serializeExecution) { + // forget the potential serializer for child tests + parentCoverageSerializers.remove(testIdentifier.getUniqueIdObject()); + // unlock the serializer for the finished tests to let the next test continue + ReentrantLock lock = coverageSerializers.remove(testIdentifier.getUniqueIdObject()); + if (lock != null) { + lock.unlock(); + } + } + } } + @Override + public String toString() { + return new StringJoiner(", ", JUnit5TestUnitFinder.class.getSimpleName() + "[", "]") + .add("testGroupConfig=" + testGroupConfig) + .add("includedTestMethods=" + includedTestMethods) + .toString(); + } }