Skip to content

Commit

Permalink
Serialize tests during coverage calculation
Browse files Browse the repository at this point in the history
This generically fixes hcoles/pitest#760 and pitest#73 for all platform engines,
removing the Jupiter specific work-around from pitest#74 and serializing test execution
during coverage calculation using locks. This way the tests can also properly run
in parallel later on during mutant hunting.
  • Loading branch information
Vampire committed May 15, 2023
1 parent 0f30f27 commit 325f0c5
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 4 deletions.
5 changes: 2 additions & 3 deletions src/main/java/org/pitest/junit5/JUnit5TestPluginFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> excludedRunners,
Collection<String> includedTestMethods) {
System.setProperty("junit.jupiter.execution.parallel.enabled", "false");
return new JUnit5Configuration(config, includedTestMethods);
}

Expand Down
71 changes: 70 additions & 1 deletion src/main/java/org/pitest/junit5/JUnit5TestUnitFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
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;
Expand All @@ -26,6 +30,7 @@
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;
Expand All @@ -35,6 +40,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;
Expand Down Expand Up @@ -97,10 +103,43 @@ private class TestIdentifierListener implements TestExecutionListener {
private final Class<?> testClass;
private final TestUnitExecutionListener l;
private final List<TestIdentifier> identifiers = synchronizedList(new ArrayList<>());
private final boolean serializeExecution;
// This map 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<UniqueId, AtomicReference<ReentrantLock>> parentCoverageSerializers = new ConcurrentHashMap<>();
// This map 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<UniqueId, ReentrantLock> coverageSerializers = new ConcurrentHashMap<>();
private final ReentrantLock rootCoverageSerializer = new ReentrantLock();

public TestIdentifierListener(Class<?> testClass, TestUnitExecutionListener l) {
this.testClass = testClass;
this.l = l;
// 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 = !(l instanceof NullExecutionListener);
}

List<TestIdentifier> getIdentifiers() {
Expand All @@ -117,12 +156,32 @@ public void executionStarted(TestIdentifier testIdentifier) {
&& !includedTestMethods.contains(((MethodSource)testIdentifier.getSource().get()).getMethodName())) {
return;
}

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<>());
}

l.executionStarted(new Description(testIdentifier.getUniqueId(), testClass));
identifiers.add(testIdentifier);
}
}


@Override
public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) {
// Classes with failing BeforeAlls never start execution and identify as 'containers' not 'tests'
Expand All @@ -134,6 +193,16 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult
} else if (testIdentifier.isTest()) {
l.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();
}
}
}

}
Expand Down

0 comments on commit 325f0c5

Please sign in to comment.