Add Clover parser #159
Add Clover parser #159
Conversation
| @@ -315,16 +309,4 @@ private void readValueCounter(final Node node, final StartElement startElement) | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| private Value createValue(final String currentType, final int covered, final int missed) { | |||
There was a problem hiding this comment.
Moved this method to parent class, so that multiple parsers can use it.
|
@uhafner Can you please review this clover parser when you get a chance? |
There was a problem hiding this comment.
Thanks for updating this PR! Clover is a long missing format. The PR already looks very well! I wonder if you can make it more robust with respect to https://github.com/jenkinsci/clover-plugin/blob/master/src/test/resources/hudson/plugins/clover so that other teams that use it for other languages will get all available information....
| ModuleNode root = new ModuleNode(""); | ||
| if (readCoverage(fileName, eventReader, root).hasChildren()) { |
There was a problem hiding this comment.
Would it be possible to read the project name property and create a ModuleNode with that name in readCoverage?
| if (event.isStartElement()) { | ||
| var startElement = event.asStartElement(); | ||
| if (PROJECT.equals(startElement.getName())) { | ||
| readProject(fileName, reader, root); |
There was a problem hiding this comment.
Here the name is available, maybe we can return a new node from here?
| String className = fileName.substring(0, fileName.lastIndexOf('.')); | ||
| String filePath = getValueOf(fileElement, PATH); |
There was a problem hiding this comment.
Do you need synthetic classes in your JS reports?
Official clover format supports classes: https://github.com/jenkinsci/clover-plugin/tree/master/src/test/resources/hudson/plugins/clover
Can you make sure that these files are also parsed correctly (with the classes that are part of the file)?
| } | ||
|
|
||
| private void resolveLines(final FileNode fileNode) { | ||
| var val = createValue("LINE", fileNode.getCoveredLines().size(), fileNode.getMissedLines().size()); |
There was a problem hiding this comment.
Can you work with Metric instances? There is no need to use Strings.
| addCoverage(node, "INSTRUCTION", stmntsCovered, stmntsTotal); | ||
| } | ||
|
|
||
| private void addCoverage(final Node node, final String metricName, final int covered, final int total) { |
| int line = getIntegerValueOf(e, NUM); | ||
| int count = getIntegerValueOf(e, COUNT); | ||
| if (count > 0) { | ||
| fileNode.addCounters(line, 1, 0); | ||
| } | ||
| else { | ||
| fileNode.addCounters(line, 0, 1); | ||
| } |
There was a problem hiding this comment.
Here it makes sense to actually use the branch coverage information as well, so that it is correctly shown in the source code view:
<line num="24" count="2" type="cond" truecount="3" falsecount="1"/>
src/test/java/edu/hm/hafner/coverage/parser/CloverParserTest.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| @SuppressWarnings({"PMD.OptimizableToArrayCall", "PMD.AvoidThrowingRawExceptionTypes"}) | ||
| void testBasic() { | ||
| var root = readReport("clover.xml"); |
There was a problem hiding this comment.
Can you check for the module and the package, that the correct metrics (LINE, BRANCH) are computed?
| addRange(covered, 24, 43); | ||
| addRange(covered, 45, 77); | ||
| addRange(covered, 79, 77); | ||
| Assertions.assertThat(f).hasMissedLines().hasCoveredLines(covered.toArray(new Integer[covered.size()])); |
There was a problem hiding this comment.
Can you check here that the path is correct and that the metrics (LINE and BRANCH) are correctly set for each file node?
| <project timestamp="1715805755763" name="All files"> | ||
| <metrics statements="69" coveredstatements="69" conditionals="14" coveredconditionals="14" methods="16" coveredmethods="16" elements="99" coveredelements="99" complexity="0" loc="69" ncloc="69" packages="1" files="3" classes="3"/> | ||
| <file name="HelloWidget.jsx" path="/home/jenkins/agent/workspace/_1_deleteme-test-coverage-1_PR-1/src/js/widgets/hello/HelloWidget.jsx"> | ||
| <metrics statements="12" coveredstatements="12" conditionals="0" coveredconditionals="0" methods="5" coveredmethods="5"/> |
There was a problem hiding this comment.
Even though you do not have methods in the model it would help users if you can add the correct METHOD metric from methods="5" coveredmethods="5".
Co-authored-by: Ullrich Hafner <[email protected]>
This cover parser has been implemented in order to support coverage files generated by nodejs applications by using clover generated coverage reports.
Please note that this PR is a replacement for #104 as Pierson has left Intuit.
Testing done
Unit tests have been added for clover parser
It has been used by 1000+ project in Intuit and reporting coverage successfully.
Submitter checklist