Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup cobertura parser #49

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Cleanup cobertura parser #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f78905
Make sure that a query for a parser always uses uppercase.
uhafner Nov 14, 2023
713bca8
Improve handling of duplicate classes and methods.
uhafner Nov 14, 2023
acb0120
Simplify method.
uhafner Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/dependency-graph.puml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ skinparam rectangle {
BackgroundColor<<runtime>> lightBlue
BackgroundColor<<provided>> lightGray
}
rectangle "coverage-model\n\n0.29.0" as edu_hm_hafner_coverage_model_jar
rectangle "coverage-model\n\n0.30.0-SNAPSHOT" as edu_hm_hafner_coverage_model_jar
rectangle "spotbugs-annotations\n\n4.7.3" as com_github_spotbugs_spotbugs_annotations_jar
rectangle "error_prone_annotations\n\n2.23.0" as com_google_errorprone_error_prone_annotations_jar
rectangle "streamex\n\n0.8.2" as one_util_streamex_jar
Expand Down
106 changes: 67 additions & 39 deletions src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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 @@ -85,49 +86,63 @@
try {
var eventReader = new SecureXmlParserFactory().createXmlEventReader(reader);

var root = new ModuleNode("-");
boolean isEmpty = true;
var root = new ModuleNode("-"); // Cobertura has no support for module names
handleEmptyResult(log, readModule(log, eventReader, root));
return root;
}
catch (XMLStreamException exception) {
throw new ParsingException(exception);
}
}

while (eventReader.hasNext()) {
XMLEvent event = eventReader.nextEvent();
private boolean readModule(final FilteredLog log, final XMLEventReader eventReader, final ModuleNode root)
throws XMLStreamException {
boolean isEmpty = true;

if (event.isStartElement()) {
var startElement = event.asStartElement();
var tagName = startElement.getName();
if (SOURCE.equals(tagName)) {
readSource(eventReader, root);
}
else if (PACKAGE.equals(tagName)) {
readPackage(eventReader, root, startElement, log);
isEmpty = false;
}
while (eventReader.hasNext()) {
XMLEvent event = eventReader.nextEvent();

if (event.isStartElement()) {
var startElement = event.asStartElement();
var tagName = startElement.getName();
if (SOURCE.equals(tagName)) {
readSource(eventReader, root);
}
else if (PACKAGE.equals(tagName)) {
readPackage(eventReader, root, readName(startElement), log);
isEmpty = false;
}
}
}
return isEmpty;
}

private void handleEmptyResult(final FilteredLog log, final boolean isEmpty) {
if (isEmpty) {
if (ignoreErrors()) {

Check warning on line 122 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 122 is only partially covered, one branch is missing
log.logError("No coverage information found in the specified file.");

Check warning on line 123 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 123 is not covered by tests

Check warning on line 123 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java#L123 

Added line #L123 was not covered by tests
}
if (isEmpty) {
else {
throw new NoSuchElementException("No coverage information found in the specified file.");
}
return root;
}
catch (XMLStreamException exception) {
throw new ParsingException(exception);
}
}

private void readPackage(final XMLEventReader reader, final ModuleNode root,
final StartElement currentStartElement, final FilteredLog log) throws XMLStreamException {
var packageNode = root.findOrCreatePackageNode(getValueOf(currentStartElement, NAME));
final String packageName, final FilteredLog log) throws XMLStreamException {
var packageNode = root.findOrCreatePackageNode(packageName);

while (reader.hasNext()) {
XMLEvent event = reader.nextEvent();

if (event.isStartElement()) {
var nextElement = event.asStartElement();
if (CLASS.equals(nextElement.getName())) {
var fileName = getValueOf(nextElement, FILE_NAME);
var element = event.asStartElement();
if (CLASS.equals(element.getName())) {
var fileName = getValueOf(element, FILE_NAME);
var relativePath = PATH_UTIL.getRelativePath(fileName);
var fileNode = packageNode.findOrCreateFileNode(getFileName(fileName),
getTreeStringBuilder().intern(relativePath));
readClassOrMethod(reader, fileNode, fileNode, nextElement, log);
readClassOrMethod(reader, fileNode, fileNode, element, log);
}
}
else if (event.isEndElement()) {
Expand Down Expand Up @@ -176,7 +191,7 @@
}
lineCoverage = lineCoverage.add(currentLineCoverage);

if (CLASS.equals(element.getName())) { // Counters are stored at file level
if (CLASS.equals(element.getName())) { // Counters are stored at file level only
int lineNumber = getIntegerValueOf(nextElement, NUMBER);
fileNode.addCounters(lineNumber, coverage.getCovered(), coverage.getMissed());
}
Expand Down Expand Up @@ -204,24 +219,37 @@
}

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 = createId();
}
var name = readName(element);
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);
return createClassNode(parentNode, log, name);
}
return createMethodNode(parentNode, element, log, name);
}

private MethodNode createMethodNode(final Node parentNode, final StartElement element, final FilteredLog log,
final String name) {
String className = name;
var signature = getValueOf(element, SIGNATURE);
var classNode = (ClassNode) parentNode;
if (classNode.findMethod(name, signature).isPresent() && ignoreErrors()) {
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'", name, signature, parentNode.getName());
name = name + "-" + createId();
if (classNode.findMethod(className, signature).isPresent() && ignoreErrors()) {
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'",
className, signature, parentNode.getName());
className = name + "-" + createId();
}
return classNode.createMethodNode(name, signature);
return classNode.createMethodNode(className, signature);
}

private ClassNode createClassNode(final Node parentNode, final FilteredLog log, final String name) {
String className = name;
if (parentNode.hasChild(className) && ignoreErrors()) {
log.logError("Found a duplicate class '%s' in '%s'", className, parentNode.getName());
className = name + "-" + createId();
}
return ((FileNode) parentNode).createClassNode(className);
}

private String readName(final StartElement element) {
return StringUtils.defaultIfBlank(getValueOf(element, NAME), createId());
}

private String createId() {
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/edu/hm/hafner/coverage/registry/ParserRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package edu.hm.hafner.coverage.registry;

import org.apache.commons.lang3.StringUtils;

import edu.hm.hafner.coverage.CoverageParser;
import edu.hm.hafner.coverage.CoverageParser.ProcessingMode;
import edu.hm.hafner.coverage.parser.CoberturaParser;
Expand Down Expand Up @@ -31,7 +33,7 @@ public enum CoverageParserType {
*/
public CoverageParser getParser(final String parserName, final ProcessingMode processingMode) {
try {
return getParser(CoverageParserType.valueOf(parserName), processingMode);
return getParser(CoverageParserType.valueOf(StringUtils.upperCase(parserName)), processingMode);
}
catch (IllegalArgumentException exception) {
throw new IllegalArgumentException("Unknown parser name: " + parserName, exception);
Expand Down
7 changes: 2 additions & 5 deletions src/test/java/edu/hm/hafner/coverage/parser/CoberturaParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ void shouldConvertCoberturaBigToTree() {
Node root = readExampleReport();

assertThat(root.getAll(MODULE)).hasSize(1);
assertThat(root.getAll(PACKAGE)).hasSize(1);
assertThat(root.getAll(PACKAGE)).hasSize(5);
assertThat(root.getAll(FILE)).hasSize(4);
assertThat(root.getAll(CLASS)).hasSize(5);
assertThat(root.getAll(METHOD)).hasSize(10);
Expand Down Expand Up @@ -389,7 +389,7 @@ void shouldConvertCoberturaBigToTree() {

assertThat(root.aggregateValues()).containsExactly(
builder.setMetric(MODULE).setCovered(1).setMissed(0).build(),
builder.setMetric(PACKAGE).setCovered(1).setMissed(0).build(),
builder.setMetric(PACKAGE).setCovered(4).setMissed(1).build(),
builder.setMetric(FILE).setCovered(4).setMissed(0).build(),
builder.setMetric(CLASS).setCovered(5).setMissed(0).build(),
builder.setMetric(METHOD).setCovered(7).setMissed(3).build(),
Expand All @@ -400,9 +400,6 @@ void shouldConvertCoberturaBigToTree() {
new FractionValue(COMPLEXITY_DENSITY, 22, 63 + 19),
new LinesOfCode(63 + 19));

assertThat(root.getChildren()).extracting(Node::getName)
.containsExactly("-");

verifyCoverageMetrics(root);

List<Node> nodes = root.getAll(FILE);
Expand Down
Loading