Skip to content

Commit

Permalink
Ignore duplicate classes in Cobertura reports.
Browse files Browse the repository at this point in the history 
Duplicate classes and methods now get a new
name to make them unique.
  • Loading branch information
@uhafner
uhafner committed Nov 2, 2023
1 parent 276491c commit c212854
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 34 deletions.
15 changes: 15 additions & 0 deletions src/main/java/edu/hm/hafner/coverage/ClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,19 @@ public MethodNode createMethodNode(final String methodName, final String signatu
addChild(fileNode);
return fileNode;
}

/**
* Returns whether this node contains a method with the specified name and signature.
*
* @param methodName
* the name of the method
* @param signature
* the signature of the method
*
* @return {@code true} if this node has a child with the specified name and signature, {@code false} otherwise
*/
public boolean hasMethod(final String methodName, final String signature) {
return getAllMethodNodes().stream()
.anyMatch(m -> m.getMethodName().equals(methodName) && m.getSignature().equals(signature));

Check warning on line 52 in src/main/java/edu/hm/hafner/coverage/ClassNode.java

View check run for this annotation

ci.jenkins.io / Mutation Coverage

Mutation survived 

One mutation survived in line 52 (BooleanTrueReturnValsMutator)
Raw output 
Survived mutations:
- replaced boolean return with true for edu/hm/hafner/coverage/ClassNode::lambda$hasMethod$0 (org.pitest.mutationtest.engine.gregor.mutators.returns.BooleanTrueReturnValsMutator)
}
}
9 changes: 9 additions & 0 deletions src/main/java/edu/hm/hafner/coverage/Percentage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@ public double toDouble() {
return (double) items * 100.0 / total;
}

/**
* Returns this percentage as an int value in the interval [0, 100].
*
* @return the coverage percentage
*/
public int toInt() {
return Math.round(items * 100.0f / total);
}

/**
* Formats a percentage to plain text and rounds the value to two decimals.
*
Expand Down
51 changes: 29 additions & 22 deletions src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@

import org.apache.commons.lang3.StringUtils;

import edu.hm.hafner.coverage.ClassNode;
import edu.hm.hafner.coverage.Coverage;
import edu.hm.hafner.coverage.Coverage.CoverageBuilder;
import edu.hm.hafner.coverage.CoverageParser;
import edu.hm.hafner.coverage.CyclomaticComplexity;
import edu.hm.hafner.coverage.FileNode;
import edu.hm.hafner.coverage.MethodNode;
import edu.hm.hafner.coverage.Metric;
import edu.hm.hafner.coverage.ModuleNode;
import edu.hm.hafner.coverage.Node;
Expand Down Expand Up @@ -127,7 +127,7 @@ private void readPackage(final XMLEventReader reader, final ModuleNode root,
var relativePath = PATH_UTIL.getRelativePath(fileName);
var fileNode = packageNode.findOrCreateFileNode(getFileName(fileName),
getTreeStringBuilder().intern(relativePath));
readClassOrMethod(reader, fileNode, nextElement, log);
readClassOrMethod(reader, fileNode, fileNode, nextElement, log);
}
}
else if (event.isEndElement()) {
Expand All @@ -145,13 +145,14 @@ private String getFileName(final String relativePath) {
}

@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.CognitiveComplexity"})
private Node readClassOrMethod(final XMLEventReader reader, final FileNode fileNode,
final StartElement parentElement, final FilteredLog log) throws XMLStreamException {
private void readClassOrMethod(final XMLEventReader reader,
final FileNode fileNode, final Node parentNode,
final StartElement element, final FilteredLog log) throws XMLStreamException {
var lineCoverage = Coverage.nullObject(Metric.LINE);
var branchCoverage = Coverage.nullObject(Metric.BRANCH);

Node node = createNode(fileNode, parentElement);
getOptionalValueOf(parentElement, COMPLEXITY)
Node node = createNode(parentNode, element, log);
getOptionalValueOf(element, COMPLEXITY)
.ifPresent(c -> node.addValue(new CyclomaticComplexity(readComplexity(c))));

while (reader.hasNext()) {
Expand All @@ -175,19 +176,13 @@ private Node readClassOrMethod(final XMLEventReader reader, final FileNode fileN
}
lineCoverage = lineCoverage.add(currentLineCoverage);

if (CLASS.equals(parentElement.getName())) { // Counters are stored at file level
if (CLASS.equals(element.getName())) { // Counters are stored at file level
int lineNumber = getIntegerValueOf(nextElement, NUMBER);
fileNode.addCounters(lineNumber, coverage.getCovered(), coverage.getMissed());
}
}
else if (METHOD.equals(nextElement.getName())) {
Node methodNode = readClassOrMethod(reader, fileNode, nextElement, log);
if (node.hasChild(methodNode.getName()) && ignoreErrors()) {
log.logError("Skipping duplicate method '%s' for class '%s'", node.getName(), methodNode.getName());
}
else {
node.addChild(methodNode);
}
readClassOrMethod(reader, fileNode, node, nextElement, log); // recursive call
}
}
else if (event.isEndElement()) {
Expand All @@ -197,7 +192,7 @@ else if (event.isEndElement()) {
if (branchCoverage.isSet()) {
node.addValue(branchCoverage);
}
return node;
return;
}
}
}
Expand All @@ -208,17 +203,29 @@ private Coverage computeLineCoverage(final int coverage) {
return coverage > 0 ? LINE_COVERED : LINE_MISSED;
}

private Node createNode(final FileNode file, final StartElement parentElement) {
var name = getValueOf(parentElement, NAME);
private Node createNode(final Node parentNode, final StartElement element, final FilteredLog log) {
var name = getValueOf(element, NAME);
if (StringUtils.isBlank(name)) { // each node must have a unique name
name = UUID.randomUUID().toString();
name = createId();
}
if (CLASS.equals(parentElement.getName())) {
return file.createClassNode(name); // connect the class with the file
if (CLASS.equals(element.getName())) {
if (parentNode.hasChild(name) && ignoreErrors()) {
log.logError("Found a duplicate class '%s' in '%s'", name, parentNode.getName());
name = name + "-" + createId();
}
return ((FileNode)parentNode).createClassNode(name);
}
else {
return new MethodNode(name, getValueOf(parentElement, SIGNATURE));
var signature = getValueOf(element, SIGNATURE);
var classNode = (ClassNode) parentNode;
if (classNode.hasMethod(name, signature) && ignoreErrors()) {
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'", name, signature, parentNode.getName());
name = name + "-" + createId();
}
return classNode.createMethodNode(name, signature);
}

private String createId() {
return UUID.randomUUID().toString();
}

private int readComplexity(final String c) {
Expand Down
1 change: 1 addition & 0 deletions src/test/java/edu/hm/hafner/coverage/PercentageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void shouldHandleOverflow() {
Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1);
Percentage percentage = Percentage.valueOf(fraction);
assertThat(percentage.toDouble()).isEqualTo(100);
assertThat(percentage.toInt()).isEqualTo(100);
}

@Test
Expand Down
49 changes: 37 additions & 12 deletions src/test/java/edu/hm/hafner/coverage/parser/CoberturaParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,18 @@ CoberturaParser createParser() {

@Test
void shouldIgnoreMissingConditionAttribute() {
Node duplicateMethods = readReport("cobertura-missing-condition-coverage.xml");
Node missingCondition = readReport("cobertura-missing-condition-coverage.xml");

assertThat(missingCondition.getAll(FILE)).extracting(Node::getName)
.containsExactly("DataSourceProvider.cs");
assertThat(missingCondition.getAll(CLASS)).extracting(Node::getName)
.containsExactly("VisualOn.Data.DataSourceProvider");
assertThat(missingCondition.getAll(METHOD)).extracting(Node::getName)
.containsExactly("Enumerate()");

verifySmallTree(duplicateMethods);
assertThat(getLog().hasErrors()).isFalse();

verifyBranchCoverageOfLine61(duplicateMethods);
verifyBranchCoverageOfLine61(missingCondition);
}

private void verifyBranchCoverageOfLine61(final Node duplicateMethods) {
Expand All @@ -53,28 +59,47 @@ private void verifyBranchCoverageOfLine61(final Node duplicateMethods) {
}

@Test
void shouldIgnoreDuplicateMethods() {
Node duplicateMethods = readReport("cobertura-duplicate-methods.xml",
void shouldIgnoreDuplicateClasses() {
Node duplicateClasses = readReport("cobertura-duplicate-classes.xml",
new CoberturaParser(ProcessingMode.IGNORE_ERRORS));

verifySmallTree(duplicateMethods);
assertThat(duplicateClasses.getAll(FILE)).extracting(Node::getName)
.containsExactly("DataSourceProvider.cs");
assertThat(duplicateClasses.getAll(CLASS)).extracting(Node::getName).hasSize(2)
.contains("VisualOn.Data.DataSourceProvider")
.element(1).asString().startsWith("VisualOn.Data.DataSourceProvider-");

assertThat(getLog().hasErrors()).isTrue();
assertThat(getLog().getErrorMessages())
.contains("Skipping duplicate method 'VisualOn.Data.DataSourceProvider' for class 'Enumerate()'");
.contains("Found a duplicate class 'VisualOn.Data.DataSourceProvider' in 'DataSourceProvider.cs'");

verifyBranchCoverageOfLine61(duplicateMethods);
verifyBranchCoverageOfLine61(duplicateClasses);

assertThatIllegalArgumentException().isThrownBy(
() -> readReport("cobertura-duplicate-methods.xml", new CoberturaParser()));
() -> readReport("cobertura-duplicate-classes.xml", new CoberturaParser()));
}

private void verifySmallTree(final Node duplicateMethods) {
@Test
void shouldIgnoreDuplicateMethods() {
Node duplicateMethods = readReport("cobertura-duplicate-methods.xml",
new CoberturaParser(ProcessingMode.IGNORE_ERRORS));

assertThat(duplicateMethods.getAll(FILE)).extracting(Node::getName)
.containsExactly("DataSourceProvider.cs");
assertThat(duplicateMethods.getAll(CLASS)).extracting(Node::getName)
.containsExactly("VisualOn.Data.DataSourceProvider");
assertThat(duplicateMethods.getAll(METHOD)).extracting(Node::getName)
.containsExactly("Enumerate()");
assertThat(duplicateMethods.getAll(METHOD)).extracting(Node::getName).hasSize(2)
.contains("Enumerate()")
.element(1).asString().startsWith("Enumerate-");

assertThat(getLog().hasErrors()).isTrue();
assertThat(getLog().getErrorMessages())
.contains("Found a duplicate method 'Enumerate' with signature '()' in 'VisualOn.Data.DataSourceProvider'");

verifyBranchCoverageOfLine61(duplicateMethods);

assertThatIllegalArgumentException().isThrownBy(
() -> readReport("cobertura-duplicate-methods.xml", new CoberturaParser()));
}

@Test @Issue("jenkinsci/code-coverage-api-plugin#729")
Expand Down
121 changes: 121 additions & 0 deletions src/test/resources/edu/hm/hafner/coverage/parser/cobertura-duplicate-classes.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?xml version="1.0" encoding="utf-8"?>
<coverage line-rate="0.8301000000000001" branch-rate="0.75" version="1.9" timestamp="1663310122" lines-covered="44"
lines-valid="53" branches-covered="3" branches-valid="4">
<sources>
<source>/CoverageTest.Service/</source>
</sources>
<packages>
<package name="CoverageTest.Service" line-rate="0.8301000000000001" branch-rate="0.75" complexity="8">
<classes>
<class name="VisualOn.Data.DataSourceProvider" filename="src/VisualOn.Core/Data/DataSourceProvider.cs"
line-rate="0.925" branch-rate="0.7692307692307693" complexity="18">
<methods>
<method name="Enumerate" signature="()" line-rate="1" branch-rate="1" complexity="2">
<lines>
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="63" hits="2" branch="false"/>
<line number="65" hits="1" branch="false"/>
</lines>
</method>
</methods>
<lines>
<line number="13" hits="2" branch="false"/>
<line number="15" hits="2" branch="false"/>
<line number="16" hits="2" branch="false"/>
<line number="17" hits="2" branch="false"/>
<line number="20" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="24" hits="2" branch="false"/>
<line number="25" hits="2" branch="true" condition-coverage="100% (4/4)"/>
<line number="26" hits="2" branch="false"/>
<line number="28" hits="2" branch="false"/>
<line number="30" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="33" hits="2" branch="false"/>
<line number="37" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="38" hits="1" branch="false"/>
<line number="40" hits="2" branch="false"/>
<line number="41" hits="2" branch="false"/>
<line number="46" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="47" hits="0" branch="false"/>
<line number="48" hits="2" branch="false"/>
<line number="53" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="54" hits="0" branch="false"/>
<line number="56" hits="2" branch="false"/>
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="63" hits="2" branch="false"/>
<line number="65" hits="1" branch="false"/>
<line number="69" hits="1" branch="false"/>
<line number="71" hits="1" branch="true" condition-coverage="100% (2/2)"/>
<line number="73" hits="1" branch="false"/>
<line number="75" hits="1" branch="false"/>
<line number="77" hits="1" branch="false"/>
<line number="81" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="82" hits="0" branch="false"/>
<line number="84" hits="2" branch="false"/>
<line number="85" hits="2" branch="false"/>
<line number="87" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="88" hits="2" branch="false"/>
<line number="90" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="91" hits="2" branch="false"/>
<line number="93" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="94" hits="2" branch="false"/>
<line number="96" hits="2" branch="false"/>
</lines>
</class>
<class name="VisualOn.Data.DataSourceProvider" filename="src/VisualOn.Core/Data/DataSourceProvider.cs"
line-rate="0.925" branch-rate="0.7692307692307693" complexity="18">
<methods>
<method name="Enumerate" signature="()" line-rate="1" branch-rate="1" complexity="2">
<lines>
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="63" hits="2" branch="false"/>
<line number="65" hits="1" branch="false"/>
</lines>
</method>
</methods>
<lines>
<line number="13" hits="2" branch="false"/>
<line number="15" hits="2" branch="false"/>
<line number="16" hits="2" branch="false"/>
<line number="17" hits="2" branch="false"/>
<line number="20" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="24" hits="2" branch="false"/>
<line number="25" hits="2" branch="true" condition-coverage="100% (4/4)"/>
<line number="26" hits="2" branch="false"/>
<line number="28" hits="2" branch="false"/>
<line number="30" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="33" hits="2" branch="false"/>
<line number="37" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="38" hits="1" branch="false"/>
<line number="40" hits="2" branch="false"/>
<line number="41" hits="2" branch="false"/>
<line number="46" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="47" hits="0" branch="false"/>
<line number="48" hits="2" branch="false"/>
<line number="53" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="54" hits="0" branch="false"/>
<line number="56" hits="2" branch="false"/>
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="63" hits="2" branch="false"/>
<line number="65" hits="1" branch="false"/>
<line number="69" hits="1" branch="false"/>
<line number="71" hits="1" branch="true" condition-coverage="100% (2/2)"/>
<line number="73" hits="1" branch="false"/>
<line number="75" hits="1" branch="false"/>
<line number="77" hits="1" branch="false"/>
<line number="81" hits="2" branch="true" condition-coverage="50% (1/2)"/>
<line number="82" hits="0" branch="false"/>
<line number="84" hits="2" branch="false"/>
<line number="85" hits="2" branch="false"/>
<line number="87" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="88" hits="2" branch="false"/>
<line number="90" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="91" hits="2" branch="false"/>
<line number="93" hits="2" branch="true" condition-coverage="100% (2/2)"/>
<line number="94" hits="2" branch="false"/>
<line number="96" hits="2" branch="false"/>
</lines>
</class>
</classes>
</package>
</packages>
</coverage>

0 comments on commit c212854

Please sign in to comment.