Skip to content

Commit

Permalink
Allowed more tests to be considered killing a mutant when using histo…
Browse files Browse the repository at this point in the history
…ric data (fixes #763) (#765)

* made incremental analysis more tolerant of coverage changes

fixes #763

Incremental analysis honors all killing tests now instead of limiting
the scope of analysis to the first killing test.
This is often identical if there is no matrix mutation in place but
TestNG plugin calculates test units in terms of test classes thus
executes all test methods of a test class and records historic data
with method level granularity and class level granularity. We need
to take all killing tests into account to match history data and
executable test units in the analyzer. This will also help if
different test classes cover a certain production class and one of
those test classes is deleted or renamed.

Improved logging to help spotting analyzer misbehaviour.

* remove log catcher after tests

Changing the configuration of the Logger is a global setting per JVM.
When running tests with surefire the JVM will be reused among tests.
Permanently changing the logging configuration might interfere with
other tests.
  • Loading branch information
StefanPenndorf authored May 10, 2020
1 parent 0b8b582 commit 503e939
Show file tree
Hide file tree
Showing 4 changed files with 314 additions and 84 deletions.
13 changes: 13 additions & 0 deletions pitest-entry/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,19 @@
<version>${testng.version}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<version>${hamcrest.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<version>${hamcrest.version}</version>
<scope>test</scope>
</dependency>

</dependencies>

Expand Down
18 changes: 8 additions & 10 deletions pitest-entry/src/main/java/org/pitest/coverage/CoverageData.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package org.pitest.coverage;

import static java.util.stream.Collectors.toCollection;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -31,7 +33,6 @@
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.pitest.classinfo.ClassInfo;
Expand Down Expand Up @@ -61,7 +62,7 @@ public class CoverageData implements CoverageDatabase {
private final List<Description> failingTestDescriptions = new ArrayList<>();

public CoverageData(final CodeSource code, final LineMap lm) {
this(code, lm, new LinkedHashMap<InstructionLocation, Set<TestInfo>>());
this(code, lm, new LinkedHashMap<>());
}


Expand Down Expand Up @@ -116,13 +117,10 @@ public int getNumberOfCoveredLines(final Collection<ClassName> mutatedClass) {

@Override
public Collection<TestInfo> getTestsForClass(final ClassName clazz) {
final Set<TestInfo> tis = new TreeSet<>(
new TestInfoNameComparator());
tis.addAll(this.instructionCoverage.entrySet().stream().filter(isFor(clazz))
.flatMap(toTests())
.collect(Collectors.toList())
);
return tis;
return this.instructionCoverage.entrySet().stream()
.filter(isFor(clazz))
.flatMap(toTests())
.collect(toCollection(() -> new TreeSet<>(new TestInfoNameComparator())));
}

public void calculateClassCoverage(final CoverageResult cr) {
Expand Down Expand Up @@ -172,7 +170,7 @@ public Collection<ClassInfo> getClassesForFile(final String sourceFile,
final Collection<ClassInfo> value = this.getClassesForFileCache().get(
keyFromSourceAndPackage(sourceFile, packageName));
if (value == null) {
return Collections.<ClassInfo> emptyList();
return Collections.emptyList();
} else {
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,14 @@ public class IncrementalAnalyser implements MutationAnalyser {

private final CodeHistory history;
private final CoverageDatabase coverage;
private final Map<DetectionStatus, Long> preAnalysed = createStatusMap();
private final Map<DetectionStatus, Long> preAnalysed = new EnumMap<>(DetectionStatus.class);

public IncrementalAnalyser(final CodeHistory history,
final CoverageDatabase coverage) {
this.history = history;
this.coverage = coverage;
}

private static Map<DetectionStatus, Long> createStatusMap() {
final EnumMap<DetectionStatus, Long> map = new EnumMap<>(DetectionStatus.class);
for (final DetectionStatus each : DetectionStatus.values()) {
map.put(each, 0L);
}
return map;
}

@Override
public Collection<MutationResult> analyse(
final Collection<MutationDetails> mutation) {
Expand All @@ -67,13 +59,18 @@ public Collection<MutationResult> analyse(
}

private void logTotals() {
int numberOfReducedMutations = 0;
for (final Entry<DetectionStatus, Long> each : this.preAnalysed.entrySet()) {
if (each.getValue() != 0) {
LOG.fine("Incremental analysis set " + each.getValue()
+ " mutations to a status of " + each.getKey());
final Long numberOfMutationsInStatus = each.getValue();
final DetectionStatus mutationStatus = each.getKey();
LOG.fine("Incremental analysis set " + numberOfMutationsInStatus
+ " mutations to a status of " + mutationStatus);
if (mutationStatus != DetectionStatus.NOT_STARTED) {
numberOfReducedMutations += numberOfMutationsInStatus;
}
}

LOG.info("Incremental analysis reduced number of mutations by " + numberOfReducedMutations );
}

private MutationResult analyseFromHistory(final MutationDetails each,
Expand All @@ -89,10 +86,16 @@ private MutationResult analyseFromHistory(final MutationDetails each,
return makeResult(each, DetectionStatus.TIMED_OUT);
}

if ((mutationStatusTestPair.getStatus() == DetectionStatus.KILLED)
&& killingTestHasNotChanged(each, mutationStatusTestPair)) {
return makeResult(each, DetectionStatus.KILLED, mutationStatusTestPair
.getKillingTests(), mutationStatusTestPair.getSucceedingTests());
if ((mutationStatusTestPair.getStatus() == DetectionStatus.KILLED)) {
final List<String> killingTestNames = filterUnchangedKillingTests(each, mutationStatusTestPair);

if (!killingTestNames.isEmpty()) {
return makeResult(
each,
DetectionStatus.KILLED,
killingTestNames,
mutationStatusTestPair.getSucceedingTests());
}
}

if ((mutationStatusTestPair.getStatus() == DetectionStatus.SURVIVED)
Expand All @@ -104,26 +107,23 @@ && killingTestHasNotChanged(each, mutationStatusTestPair)) {
return analyseFromScratch(each);
}

private boolean killingTestHasNotChanged(final MutationDetails each,
final MutationStatusTestPair mutationStatusTestPair) {
final Collection<TestInfo> allTests = this.coverage.getTestsForClass(each
.getClassName());
private List<String> filterUnchangedKillingTests(final MutationDetails each,
final MutationStatusTestPair mutationStatusTestPair) {

final List<ClassName> testClasses = allTests.stream()
.filter(testIsCalled(mutationStatusTestPair.getKillingTest().get()))
.map(TestInfo.toDefiningClassName())
return this.coverage.getTestsForClass(each.getClassName()).stream()
.filter(isAKillingTestFor(mutationStatusTestPair))
.filter(testClassDidNotChange())
.map(TestInfo::getName)
.collect(Collectors.toList());
}

if (testClasses.isEmpty()) {
return false;
}

return !this.history.hasClassChanged(testClasses.get(0));

private Predicate<TestInfo> testClassDidNotChange() {
return a -> !this.history.hasClassChanged(TestInfo.toDefiningClassName().apply(a));
}

private static Predicate<TestInfo> testIsCalled(final String testName) {
return a -> a.getName().equals(testName);
private static Predicate<TestInfo> isAKillingTestFor(final MutationStatusTestPair mutation) {
final List<String> killingTestNames = mutation.getKillingTests();
return a -> killingTestNames.contains(a.getName());
}

private MutationResult analyseFromScratch(final MutationDetails mutation) {
Expand All @@ -144,10 +144,7 @@ private MutationResult makeResult(final MutationDetails each,
}

private void updatePreanalysedTotal(final DetectionStatus status) {
if (status != DetectionStatus.NOT_STARTED) {
final long count = this.preAnalysed.get(status);
this.preAnalysed.put(status, count + 1);
}
this.preAnalysed.merge(status, 1L, Long::sum);
}

}
Loading

0 comments on commit 503e939

Please sign in to comment.