Skip to content

Commit 5e1d718

Browse files
uhafneruhafner
authored
Merge pull request #47 from jenkinsci/duplicate-classes
Ignore duplicate classes in Cobertura reports
2 parents 276491c + 5ccb040 commit 5e1d718

File tree

5 files changed

+197
-34
lines changed

5 files changed

+197
-34
lines changed

‎src/main/java/edu/hm/hafner/coverage/Percentage.java

+9
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,15 @@ public double toDouble() {
117117
return (double) items * 100.0 / total;
118118
}
119119

120+
/**
121+
* Returns this percentage as an int value in the interval [0, 100].
122+
*
123+
* @return the coverage percentage
124+
*/
125+
public int toInt() {
126+
return Math.round(items * 100.0f / total);
127+
}
128+
120129
/**
121130
* Formats a percentage to plain text and rounds the value to two decimals.
122131
*

‎src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

+29-22
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313

1414
import org.apache.commons.lang3.StringUtils;
1515

16+
import edu.hm.hafner.coverage.ClassNode;
1617
import edu.hm.hafner.coverage.Coverage;
1718
import edu.hm.hafner.coverage.Coverage.CoverageBuilder;
1819
import edu.hm.hafner.coverage.CoverageParser;
1920
import edu.hm.hafner.coverage.CyclomaticComplexity;
2021
import edu.hm.hafner.coverage.FileNode;
21-
import edu.hm.hafner.coverage.MethodNode;
2222
import edu.hm.hafner.coverage.Metric;
2323
import edu.hm.hafner.coverage.ModuleNode;
2424
import edu.hm.hafner.coverage.Node;
@@ -127,7 +127,7 @@ private void readPackage(final XMLEventReader reader, final ModuleNode root,
127127
var relativePath = PATH_UTIL.getRelativePath(fileName);
128128
var fileNode = packageNode.findOrCreateFileNode(getFileName(fileName),
129129
getTreeStringBuilder().intern(relativePath));
130-
readClassOrMethod(reader, fileNode, nextElement, log);
130+
readClassOrMethod(reader, fileNode, fileNode, nextElement, log);
131131
}
132132
}
133133
else if (event.isEndElement()) {
@@ -145,13 +145,14 @@ private String getFileName(final String relativePath) {
145145
}
146146

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

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

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

178-
if (CLASS.equals(parentElement.getName())) { // Counters are stored at file level
179+
if (CLASS.equals(element.getName())) { // Counters are stored at file level
179180
int lineNumber = getIntegerValueOf(nextElement, NUMBER);
180181
fileNode.addCounters(lineNumber, coverage.getCovered(), coverage.getMissed());
181182
}
182183
}
183184
else if (METHOD.equals(nextElement.getName())) {
184-
Node methodNode = readClassOrMethod(reader, fileNode, nextElement, log);
185-
if (node.hasChild(methodNode.getName()) && ignoreErrors()) {
186-
log.logError("Skipping duplicate method '%s' for class '%s'", node.getName(), methodNode.getName());
187-
}
188-
else {
189-
node.addChild(methodNode);
190-
}
185+
readClassOrMethod(reader, fileNode, node, nextElement, log); // recursive call
191186
}
192187
}
193188
else if (event.isEndElement()) {
@@ -197,7 +192,7 @@ else if (event.isEndElement()) {
197192
if (branchCoverage.isSet()) {
198193
node.addValue(branchCoverage);
199194
}
200-
return node;
195+
return;
201196
}
202197
}
203198
}
@@ -208,17 +203,29 @@ private Coverage computeLineCoverage(final int coverage) {
208203
return coverage > 0 ? LINE_COVERED : LINE_MISSED;
209204
}
210205

211-
private Node createNode(final FileNode file, final StartElement parentElement) {
212-
var name = getValueOf(parentElement, NAME);
206+
private Node createNode(final Node parentNode, final StartElement element, final FilteredLog log) {
207+
var name = getValueOf(element, NAME);
213208
if (StringUtils.isBlank(name)) { // each node must have a unique name
214-
name = UUID.randomUUID().toString();
209+
name = createId();
215210
}
216-
if (CLASS.equals(parentElement.getName())) {
217-
return file.createClassNode(name); // connect the class with the file
211+
if (CLASS.equals(element.getName())) {
212+
if (parentNode.hasChild(name) && ignoreErrors()) {
213+
log.logError("Found a duplicate class '%s' in '%s'", name, parentNode.getName());
214+
name = name + "-" + createId();
215+
}
216+
return ((FileNode)parentNode).createClassNode(name);
218217
}
219-
else {
220-
return new MethodNode(name, getValueOf(parentElement, SIGNATURE));
218+
var signature = getValueOf(element, SIGNATURE);
219+
var classNode = (ClassNode) parentNode;
220+
if (classNode.findMethod(name, signature).isPresent() && ignoreErrors()) {
221+
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'", name, signature, parentNode.getName());
222+
name = name + "-" + createId();
221223
}
224+
return classNode.createMethodNode(name, signature);
225+
}
226+
227+
private String createId() {
228+
return UUID.randomUUID().toString();
222229
}
223230

224231
private int readComplexity(final String c) {

‎src/test/java/edu/hm/hafner/coverage/PercentageTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ void shouldHandleOverflow() {
2020
Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1);
2121
Percentage percentage = Percentage.valueOf(fraction);
2222
assertThat(percentage.toDouble()).isEqualTo(100);
23+
assertThat(percentage.toInt()).isEqualTo(100);
2324
}
2425

2526
@Test

‎src/test/java/edu/hm/hafner/coverage/parser/CoberturaParserTest.java

+37-12
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,18 @@ CoberturaParser createParser() {
3838

3939
@Test
4040
void shouldIgnoreMissingConditionAttribute() {
41-
Node duplicateMethods = readReport("cobertura-missing-condition-coverage.xml");
41+
Node missingCondition = readReport("cobertura-missing-condition-coverage.xml");
42+
43+
assertThat(missingCondition.getAll(FILE)).extracting(Node::getName)
44+
.containsExactly("DataSourceProvider.cs");
45+
assertThat(missingCondition.getAll(CLASS)).extracting(Node::getName)
46+
.containsExactly("VisualOn.Data.DataSourceProvider");
47+
assertThat(missingCondition.getAll(METHOD)).extracting(Node::getName)
48+
.containsExactly("Enumerate()");
4249

43-
verifySmallTree(duplicateMethods);
4450
assertThat(getLog().hasErrors()).isFalse();
4551

46-
verifyBranchCoverageOfLine61(duplicateMethods);
52+
verifyBranchCoverageOfLine61(missingCondition);
4753
}
4854

4955
private void verifyBranchCoverageOfLine61(final Node duplicateMethods) {
@@ -53,28 +59,47 @@ private void verifyBranchCoverageOfLine61(final Node duplicateMethods) {
5359
}
5460

5561
@Test
56-
void shouldIgnoreDuplicateMethods() {
57-
Node duplicateMethods = readReport("cobertura-duplicate-methods.xml",
62+
void shouldIgnoreDuplicateClasses() {
63+
Node duplicateClasses = readReport("cobertura-duplicate-classes.xml",
5864
new CoberturaParser(ProcessingMode.IGNORE_ERRORS));
5965

60-
verifySmallTree(duplicateMethods);
66+
assertThat(duplicateClasses.getAll(FILE)).extracting(Node::getName)
67+
.containsExactly("DataSourceProvider.cs");
68+
assertThat(duplicateClasses.getAll(CLASS)).extracting(Node::getName).hasSize(2)
69+
.contains("VisualOn.Data.DataSourceProvider")
70+
.element(1).asString().startsWith("VisualOn.Data.DataSourceProvider-");
71+
6172
assertThat(getLog().hasErrors()).isTrue();
6273
assertThat(getLog().getErrorMessages())
63-
.contains("Skipping duplicate method 'VisualOn.Data.DataSourceProvider' for class 'Enumerate()'");
74+
.contains("Found a duplicate class 'VisualOn.Data.DataSourceProvider' in 'DataSourceProvider.cs'");
6475

65-
verifyBranchCoverageOfLine61(duplicateMethods);
76+
verifyBranchCoverageOfLine61(duplicateClasses);
6677

6778
assertThatIllegalArgumentException().isThrownBy(
68-
() -> readReport("cobertura-duplicate-methods.xml", new CoberturaParser()));
79+
() -> readReport("cobertura-duplicate-classes.xml", new CoberturaParser()));
6980
}
7081

71-
private void verifySmallTree(final Node duplicateMethods) {
82+
@Test
83+
void shouldIgnoreDuplicateMethods() {
84+
Node duplicateMethods = readReport("cobertura-duplicate-methods.xml",
85+
new CoberturaParser(ProcessingMode.IGNORE_ERRORS));
86+
7287
assertThat(duplicateMethods.getAll(FILE)).extracting(Node::getName)
7388
.containsExactly("DataSourceProvider.cs");
7489
assertThat(duplicateMethods.getAll(CLASS)).extracting(Node::getName)
7590
.containsExactly("VisualOn.Data.DataSourceProvider");
76-
assertThat(duplicateMethods.getAll(METHOD)).extracting(Node::getName)
77-
.containsExactly("Enumerate()");
91+
assertThat(duplicateMethods.getAll(METHOD)).extracting(Node::getName).hasSize(2)
92+
.contains("Enumerate()")
93+
.element(1).asString().startsWith("Enumerate-");
94+
95+
assertThat(getLog().hasErrors()).isTrue();
96+
assertThat(getLog().getErrorMessages())
97+
.contains("Found a duplicate method 'Enumerate' with signature '()' in 'VisualOn.Data.DataSourceProvider'");
98+
99+
verifyBranchCoverageOfLine61(duplicateMethods);
100+
101+
assertThatIllegalArgumentException().isThrownBy(
102+
() -> readReport("cobertura-duplicate-methods.xml", new CoberturaParser()));
78103
}
79104

80105
@Test @Issue("jenkinsci/code-coverage-api-plugin#729")

‎src/test/resources/edu/hm/hafner/coverage/parser/cobertura-duplicate-classes.xml

+121
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<coverage line-rate="0.8301000000000001" branch-rate="0.75" version="1.9" timestamp="1663310122" lines-covered="44"
3+
lines-valid="53" branches-covered="3" branches-valid="4">
4+
<sources>
5+
<source>/CoverageTest.Service/</source>
6+
</sources>
7+
<packages>
8+
<package name="CoverageTest.Service" line-rate="0.8301000000000001" branch-rate="0.75" complexity="8">
9+
<classes>
10+
<class name="VisualOn.Data.DataSourceProvider" filename="src/VisualOn.Core/Data/DataSourceProvider.cs"
11+
line-rate="0.925" branch-rate="0.7692307692307693" complexity="18">
12+
<methods>
13+
<method name="Enumerate" signature="()" line-rate="1" branch-rate="1" complexity="2">
14+
<lines>
15+
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
16+
<line number="63" hits="2" branch="false"/>
17+
<line number="65" hits="1" branch="false"/>
18+
</lines>
19+
</method>
20+
</methods>
21+
<lines>
22+
<line number="13" hits="2" branch="false"/>
23+
<line number="15" hits="2" branch="false"/>
24+
<line number="16" hits="2" branch="false"/>
25+
<line number="17" hits="2" branch="false"/>
26+
<line number="20" hits="2" branch="true" condition-coverage="50% (1/2)"/>
27+
<line number="24" hits="2" branch="false"/>
28+
<line number="25" hits="2" branch="true" condition-coverage="100% (4/4)"/>
29+
<line number="26" hits="2" branch="false"/>
30+
<line number="28" hits="2" branch="false"/>
31+
<line number="30" hits="2" branch="true" condition-coverage="50% (1/2)"/>
32+
<line number="33" hits="2" branch="false"/>
33+
<line number="37" hits="2" branch="true" condition-coverage="50% (1/2)"/>
34+
<line number="38" hits="1" branch="false"/>
35+
<line number="40" hits="2" branch="false"/>
36+
<line number="41" hits="2" branch="false"/>
37+
<line number="46" hits="2" branch="true" condition-coverage="50% (1/2)"/>
38+
<line number="47" hits="0" branch="false"/>
39+
<line number="48" hits="2" branch="false"/>
40+
<line number="53" hits="2" branch="true" condition-coverage="50% (1/2)"/>
41+
<line number="54" hits="0" branch="false"/>
42+
<line number="56" hits="2" branch="false"/>
43+
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
44+
<line number="63" hits="2" branch="false"/>
45+
<line number="65" hits="1" branch="false"/>
46+
<line number="69" hits="1" branch="false"/>
47+
<line number="71" hits="1" branch="true" condition-coverage="100% (2/2)"/>
48+
<line number="73" hits="1" branch="false"/>
49+
<line number="75" hits="1" branch="false"/>
50+
<line number="77" hits="1" branch="false"/>
51+
<line number="81" hits="2" branch="true" condition-coverage="50% (1/2)"/>
52+
<line number="82" hits="0" branch="false"/>
53+
<line number="84" hits="2" branch="false"/>
54+
<line number="85" hits="2" branch="false"/>
55+
<line number="87" hits="2" branch="true" condition-coverage="100% (2/2)"/>
56+
<line number="88" hits="2" branch="false"/>
57+
<line number="90" hits="2" branch="true" condition-coverage="100% (2/2)"/>
58+
<line number="91" hits="2" branch="false"/>
59+
<line number="93" hits="2" branch="true" condition-coverage="100% (2/2)"/>
60+
<line number="94" hits="2" branch="false"/>
61+
<line number="96" hits="2" branch="false"/>
62+
</lines>
63+
</class>
64+
<class name="VisualOn.Data.DataSourceProvider" filename="src/VisualOn.Core/Data/DataSourceProvider.cs"
65+
line-rate="0.925" branch-rate="0.7692307692307693" complexity="18">
66+
<methods>
67+
<method name="Enumerate" signature="()" line-rate="1" branch-rate="1" complexity="2">
68+
<lines>
69+
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
70+
<line number="63" hits="2" branch="false"/>
71+
<line number="65" hits="1" branch="false"/>
72+
</lines>
73+
</method>
74+
</methods>
75+
<lines>
76+
<line number="13" hits="2" branch="false"/>
77+
<line number="15" hits="2" branch="false"/>
78+
<line number="16" hits="2" branch="false"/>
79+
<line number="17" hits="2" branch="false"/>
80+
<line number="20" hits="2" branch="true" condition-coverage="50% (1/2)"/>
81+
<line number="24" hits="2" branch="false"/>
82+
<line number="25" hits="2" branch="true" condition-coverage="100% (4/4)"/>
83+
<line number="26" hits="2" branch="false"/>
84+
<line number="28" hits="2" branch="false"/>
85+
<line number="30" hits="2" branch="true" condition-coverage="50% (1/2)"/>
86+
<line number="33" hits="2" branch="false"/>
87+
<line number="37" hits="2" branch="true" condition-coverage="50% (1/2)"/>
88+
<line number="38" hits="1" branch="false"/>
89+
<line number="40" hits="2" branch="false"/>
90+
<line number="41" hits="2" branch="false"/>
91+
<line number="46" hits="2" branch="true" condition-coverage="50% (1/2)"/>
92+
<line number="47" hits="0" branch="false"/>
93+
<line number="48" hits="2" branch="false"/>
94+
<line number="53" hits="2" branch="true" condition-coverage="50% (1/2)"/>
95+
<line number="54" hits="0" branch="false"/>
96+
<line number="56" hits="2" branch="false"/>
97+
<line number="61" hits="2" branch="true" condition-coverage="100% (2/2)"/>
98+
<line number="63" hits="2" branch="false"/>
99+
<line number="65" hits="1" branch="false"/>
100+
<line number="69" hits="1" branch="false"/>
101+
<line number="71" hits="1" branch="true" condition-coverage="100% (2/2)"/>
102+
<line number="73" hits="1" branch="false"/>
103+
<line number="75" hits="1" branch="false"/>
104+
<line number="77" hits="1" branch="false"/>
105+
<line number="81" hits="2" branch="true" condition-coverage="50% (1/2)"/>
106+
<line number="82" hits="0" branch="false"/>
107+
<line number="84" hits="2" branch="false"/>
108+
<line number="85" hits="2" branch="false"/>
109+
<line number="87" hits="2" branch="true" condition-coverage="100% (2/2)"/>
110+
<line number="88" hits="2" branch="false"/>
111+
<line number="90" hits="2" branch="true" condition-coverage="100% (2/2)"/>
112+
<line number="91" hits="2" branch="false"/>
113+
<line number="93" hits="2" branch="true" condition-coverage="100% (2/2)"/>
114+
<line number="94" hits="2" branch="false"/>
115+
<line number="96" hits="2" branch="false"/>
116+
</lines>
117+
</class>
118+
</classes>
119+
</package>
120+
</packages>
121+
</coverage>

0 commit comments

Comments
 (0)