Add parser for Clover coverage #104

pyieh · 2024-05-07T00:28:30Z

Adding a parser for Python coverage files.

Testing done

Yesting was done in both with unit tests and a test coverage file (based on real coverage files). As well as on a local Jenkins, where a local version of the Jenkins coverage-plugin was installed to use this coverage model. Ran a simple Python pipeline and validated the coverage output was available in the UI.

Submitter checklist

Give feedback

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue
Options

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

src/main/java/edu/hm/hafner/coverage/Metric.java

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

src/test/java/edu/hm/hafner/coverage/parser/CloverParserTest.java

uhafner · 2024-05-13T20:06:55Z

src/test/resources/edu/hm/hafner/coverage/parser/clover/clover.xml

+        <package name="components">
+            <metrics statements="46" coveredstatements="35" conditionals="12" coveredconditionals="8" methods="17" coveredmethods="7"/>
+            <file name="File3.jsx" path="/home/jenkins/agent/workspace/dir/src/js/components/File3.jsx">
+                <metrics statements="17" coveredstatements="11" conditionals="2" coveredconditionals="2" methods="8" coveredmethods="3"/>


Why is there conditionals="2" coveredconditionals="2" but in line 61 we have no coverage of the false branch:

truecount="2" falsecount="0"

Branches are currently not used so far (only the branch coverage). That means in the file rendering you see only green and red lines. If you want to support a visualization of the branches then you need to evaluate these values as well. Then it makes sense to see what truecount="2" falsecount="0" actually means.

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

src/test/java/edu/hm/hafner/coverage/parser/CloverParserTest.java

src/main/java/edu/hm/hafner/coverage/parser/JacocoParser.java

uhafner · 2024-05-14T19:45:31Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+    @CanIgnoreReturnValue
+    private FileNode readFile(final XMLEventReader reader, final PackageNode packageNode, final StartElement fileElement) throws XMLStreamException {
+        String fileName = getValueOf(fileElement, NAME);
+        String className = fileName.substring(0, fileName.lastIndexOf("."));


Try to implement that in a safe way as the index might be -1.

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

uhafner · 2024-05-14T19:50:19Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+                    classNode.addValue(createValue("CONDITIONAL", condCovered, condTotal - condCovered));
+                    classNode.addValue(createValue("INSTRUCTION", stmntsCovered, stmntsTotal - stmntsCovered));
+
+                } else if (LINE.equals(e.getName())) {


Did you omit the branch coverage on purpose?

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

uhafner

We are almost there! Please check the remaining comments and either mark them as resolved or update the sources.

It seems that you do not use my style guide settings so I auto-formatted the parser in IntelliJ and pushed the commit...

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

uhafner · 2024-05-15T19:29:45Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+            return new ModuleNode("empty");
+        }
+        catch (XMLStreamException exception) {
+            throw new SecureXmlParserFactory.ParsingException(exception);


Suggested change

throw new SecureXmlParserFactory.ParsingException(exception);

throw new ParsingException(exception);

Then I can simpler move the exception in a followup PR.

uhafner · 2024-05-15T19:31:53Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+                    }
+                }
+                else {
+                    new ParsingException(


Suggested change

new ParsingException(

throw new ParsingException(

uhafner · 2024-05-15T19:32:24Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+        }
+    }
+
+    private void addBranchCoverage(final Node node, final StartElement e) {// final int covered, final int total) {


Suggested change

private void addBranchCoverage(final Node node, final StartElement e) {// final int covered, final int total) {

private void addBranchCoverage(final Node node, final StartElement e) {

uhafner · 2024-05-15T19:33:41Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+        addCoverage(node, "BRANCH", condCovered, condTotal);
+    }
+
+    private void addInstructionCoverage(final Node node, final StartElement e) { //final int covered, final int total) {


Suggested change

private void addInstructionCoverage(final Node node, final StartElement e) { //final int covered, final int total) {

private void addInstructionCoverage(final Node node, final StartElement e) {

uhafner · 2024-05-15T19:34:40Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+    private void addInstructionCoverage(final Node node, final StartElement e) { //final int covered, final int total) {
+        int stmntsTotal = getIntegerValueOf(e, STATEMENTS);
+        int stmntsCovered = getIntegerValueOf(e, COVERED_STATEMENTS);
+        addCoverage(node, "INSTRUCTION", stmntsCovered, stmntsTotal);


Use the enum

uhafner · 2024-05-15T19:35:32Z

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

+    }
+
+    private void addInstructionCoverage(final Node node, final StartElement e) { //final int covered, final int total) {
+        int stmntsTotal = getIntegerValueOf(e, STATEMENTS);


In Java we typically do not use shortened variable names, use statementsTotal and so on.

uhafner · 2024-05-15T19:36:01Z

src/main/java/edu/hm/hafner/coverage/registry/ParserRegistry.java

+            case CLOVER:
+                return new CloverParser(processingMode);


Can you add Clover to the README as well?

uhafner · 2024-05-15T19:39:06Z

src/test/resources/edu/hm/hafner/coverage/parser/clover/clover.xml

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<coverage generated="1695944532781" clover="3.2.0">
+    <project timestamp="1695944532781" name="All files">
+        <metrics statements="3057" coveredstatements="2578" conditionals="1444" coveredconditionals="1068" methods="1003" coveredmethods="736" elements="5504" coveredelements="4382" complexity="0" loc="3057" ncloc="3057" packages="68" files="170" classes="170"/>


If you want to see the correct LOC you can set the LOC value on the project level (otherwise it is computed from the line values).

uhafner · 2024-05-15T19:41:23Z

src/test/resources/edu/hm/hafner/coverage/parser/clover/clover.xml

+        <package name="components">
+            <metrics statements="46" coveredstatements="35" conditionals="12" coveredconditionals="8" methods="17" coveredmethods="7"/>
+            <file name="File3.jsx" path="/home/jenkins/agent/workspace/dir/src/js/components/File3.jsx">
+                <metrics statements="17" coveredstatements="11" conditionals="2" coveredconditionals="2" methods="8" coveredmethods="3"/>


Branches are currently not used so far (only the branch coverage). That means in the file rendering you see only green and red lines. If you want to support a visualization of the branches then you need to evaluate these values as well. Then it makes sense to see what truecount="2" falsecount="0" actually means.

Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>

uhafner · 2024-06-04T09:17:03Z

Do you need some help with this parser?

pyieh · 2024-06-13T20:18:31Z

@uhafner Sorry, been pulled into other priorities, but will get to this soon!

Add Clover parser

50fc5d0

github-actions bot assigned uhafner May 7, 2024

github-advanced-security bot found potential problems May 7, 2024

View reviewed changes

uhafner added the feature New features label May 8, 2024

uhafner requested changes May 13, 2024

View reviewed changes

uhafner marked this pull request as draft May 13, 2024 20:07

Pierson Yieh added 4 commits May 13, 2024 15:08

Update CONDITIONAL to reuse BRANCH Metric

0a334dc

Update CloverParser constructor to align with other Parser constructors

e359591

Refactor test for coding style

ef73946

Change Integer to primitive int

18fc533

github-advanced-security bot found potential problems May 13, 2024

View reviewed changes

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java Fixed Show fixed Hide fixed

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java Fixed Show fixed Hide fixed

Pierson Yieh and others added 2 commits May 13, 2024 15:58

Update empty results handling to not need use for null

fde1769

Update src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

7ffa0ac

Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>

github-advanced-security bot found potential problems May 13, 2024

View reviewed changes

src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java Fixed Show fixed Hide fixed

uhafner requested changes May 14, 2024

View reviewed changes

Pierson Yieh and others added 5 commits May 14, 2024 17:06

Update with File BRANCH & INSTRUCTION values

a60b0fe

Removed need for CloverParser to override CoverageParser#createValue()

1431401

Use existing getIntegerValue() method to parse ints

4fe7004

Add top level Metrics values to their respective Nodes

0b68793

Use IntelliJ autoformatter and cleanup.

5efdaad

uhafner requested changes May 15, 2024

View reviewed changes

Update src/main/java/edu/hm/hafner/coverage/parser/CloverParser.java

0fba895

Co-authored-by: Ullrich Hafner <ullrich.hafner@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parser for Clover coverage #104

Add parser for Clover coverage #104

pyieh commented May 7, 2024

Submitter checklist

uhafner May 13, 2024

uhafner May 15, 2024

uhafner May 14, 2024

uhafner May 14, 2024

uhafner left a comment

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner May 15, 2024

uhafner commented Jun 4, 2024

pyieh commented Jun 13, 2024

	throw new SecureXmlParserFactory.ParsingException(exception);
	throw new ParsingException(exception);

	private void addBranchCoverage(final Node node, final StartElement e) {// final int covered, final int total) {
	private void addBranchCoverage(final Node node, final StartElement e) {

	private void addInstructionCoverage(final Node node, final StartElement e) { //final int covered, final int total) {
	private void addInstructionCoverage(final Node node, final StartElement e) {

Add parser for Clover coverage #104

Are you sure you want to change the base?

Add parser for Clover coverage #104

Conversation

pyieh commented May 7, 2024

Testing done

Submitter checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhafner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uhafner commented Jun 4, 2024

pyieh commented Jun 13, 2024