Add parser for GoCov #99

pyieh · 2024-04-23T00:49:25Z

Adding a parser for GoCov coverage files.

Testing done

Testing 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 Go pipeline and validated the coverage output was available in the UI.

Once approved and merged, the corresponding coverage-plugin change can be found here: N/A

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

…geParser.parseReport() method declaration

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

+                log.logError(USAGE_ERROR_MSG);
+                throw new InvalidGoCoverageFormatException("Expected the coverage mode specification, but got: [" + line + "]");
+            }
+            switch(line) {


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

+                String domain = split[0];
+                String org = split[1];
+                String repo = split[2];
+                String module = String.join("/", domain, org, repo);


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

+
+                String[] start = split2[1].split("\\.");
+                String startLine = start[0];
+                String startCol = start[1];


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

+                String startCol = start[1];
+                String[] end = split2[2].split("\\.");
+                String endLine = end[0];
+                String endCol = end[1];


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

+                String[] end = split2[2].split("\\.");
+                String endLine = end[0];
+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);


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

+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);
+                Integer executionCount = Integer.parseInt(split[split.length-1]);
+                for (Integer i = Integer.parseInt(startLine), j = Integer.parseInt(endLine); i <= j; i++) {


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

+                String[] end = split2[2].split("\\.");
+                String endLine = end[0];
+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);


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

+                String endLine = end[0];
+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);
+                Integer executionCount = Integer.parseInt(split[split.length-1]);


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

+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);
+                Integer executionCount = Integer.parseInt(split[split.length-1]);
+                for (Integer i = Integer.parseInt(startLine), j = Integer.parseInt(endLine); i <= j; i++) {


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

+                String endCol = end[1];
+                Integer statementCount = Integer.parseInt(split[split.length-2]);
+                Integer executionCount = Integer.parseInt(split[split.length-1]);
+                for (Integer i = Integer.parseInt(startLine), j = Integer.parseInt(endLine); i <= j; i++) {


uhafner

Thanks for your pull request for this new format!

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

src/test/java/edu/hm/hafner/coverage/AbstractNodeTest.java

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

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

uhafner · 2024-04-23T18:54:04Z

src/test/resources/edu/hm/hafner/coverage/parser/go/go-coverage-atomic.out

@@ -0,0 +1,12 @@
+mode: atomic
+github.com/example/project/pkg/utils/file1.go:5.12,8.22 3 1


Can you explain the counters? Is there no function information?

The format is:
<FILE>:<start row>.<start column>,<end row>.<end column> <# of statements> <# of statements covered>

Ok, then this format actually provides only line and instruction (or in your case statement) coverage. That means the counters for the line and branch coverage are either 1/1 or 0/1 (covered/total), see next comment.

The exact numbers of statements can be used to set the instruction coverage. Since we have only files as elements this coverage needs to be put per file.

My mistake, the format is actually:
<FILE>:<start row>.<start column>,<end row>.<end column> <# of statements covered> <# of statements missed>
And so that's we have a running tallies of the number of covered & missed statements, then resolve everything at the end.

This does not make sense when I look at the test case file? All of your lines actually show a coverage > 0. Or am I missing something?

mode: atomic github.com/example/project/pkg/utils/file1.go:5.12,8.22 3 1 github.com/example/project/pkg/utils/file1.go:10.9,15.23 5 3 github.com/example/project/pkg/utils/file1.go:19.16,20.23 2 0 github.com/example/project/pkg/utils/file1.go:24.3,25.25 1 1 github.com/example/project/pkg/utils/file1.go:30.16,32.25 1 1 github.com/example/project/pkg/db/file2.go:12.7,15.7 1 0 github.com/example/project/pkg/db/file2.go:17.7,22.14 3 2 github.com/example/project/cmd/file3.go:10.12,10.21 1 0 github.com/example/project/cmd/file3.go:15.17,17.27 3 0 github.com/example/project/pkg/test/file4.go:10.12,11.21 1 0 github.com/example/project/pkg/test/file4.go:10.12,11.21 1 1

I rewrote the test so it matches my coding style:
3ae1498

I will try to refactor the parser as well. What is important and missing: your parser produces INSTRUCTION coverage. Which implies line coverage.

Let's take the example line:

github.com/example/project/pkg/utils/file1.go:5.12,8.22 3 1

It'd be read as:
For file1.go , lines 5-8, there were 3 statements that were covered and 1 statement that was missed.

But then I do not understand the actual test case:

Report:

github.com/example/project/pkg/db/file2.go:12.7,15.7 1 0 github.com/example/project/pkg/db/file2.go:17.7,22.14 3 2

Actual test:

two -> assertThat(two).hasName("file2.go") .hasMissedLines(12, 13, 14, 15) .hasCoveredLines(17, 18, 19, 20, 21, 22)

From the counters I would expect: Lines 12-15 have one statement covered and 0 missed. So we should have:

two -> assertThat(two).hasName("file2.go") .hasCoveredLines(12, 13, 14, 15, 17, 18, 19, 20, 21, 22)

Do you need some help to get this parser finished? I think we are currently struggling at the missed/covered computation, see comment above.

uhafner · 2024-04-23T18:58:30Z

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

@@ -521,6 +521,17 @@ public FileNode addCounters(final int lineNumber, final int covered, final int m
        return this;
    }

+    @CanIgnoreReturnValue
+    public FileNode addCounterIncremental(final int lineNumber, final int covered, final int missed) {


I don't think that an extra method is required. From the format of your test case it looks like you do not use branch coverage at all. So the counter should be c=1 m0 or c=0 m1. If c +n > 1 then the UI assumes that you are using branch coverage.

As addressed in previous statement, my mistake on the previous incorrect information. But because each line read in the coverage file gives N number of covered statements, and M number of missed statements, we keep running tallies of each one and then resolve everything at the end.

uhafner · 2024-04-23T18:59:11Z

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

+import static edu.hm.hafner.coverage.parser.JacocoParser.createValue;
+
+public class GoParser extends CoverageParser {
+    private static final java.util.logging.Logger LOGGER = java.util.logging.Logger.getLogger(GoParser.class.getName());


Please use the provided logger that will report the information in the UI.

I'm already are doing so (example), I was using these to also log the information to Jenkins logs, but they can be removed to better align with the rest of the module, if you want.

uhafner · 2024-04-23T19:07:53Z

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

This parser seems to be way to complex for the simple format of the test case? Are you planning to parser different reports? So I am skipping the review on this class right now. It would also make sense to keep the static analysis happy before starting a review...

pyieh · 2024-05-09T22:43:18Z

@uhafner Addressed your comments and corrected an incorrect statement I made before. Hopefully the correct explanation of the Go coverage files justifies our methodology for counting coverage.

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

+
+    @Test
+    void testAtomic() {
+        var instructionBuilder = new Coverage.CoverageBuilder().withMetric(INSTRUCTION);


uhafner · 2024-05-13T10:40:21Z

Wouldn't it make sense to use https://github.com/AlekSi/gocov-xml? Then we simply can reuse the Cobertura parser? Wouldn't there be also method names when used in that way?

pyieh · 2024-05-13T21:51:17Z

Wouldn't it make sense to use https://github.com/AlekSi/gocov-xml? Then we simply can reuse the Cobertura parser? Wouldn't there be also method names when used in that way?

@uhafner we tried avoiding using any non-organization type of libraries due to possible lack of future support reasons, but this author seems to be highly involved in the Go community and if you're okay with us using this library, I can re-implement it to do so.

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

Wouldn't it make sense to use https://github.com/AlekSi/gocov-xml? Then we simply can reuse the Cobertura parser? Wouldn't there be also method names when used in that way?

@uhafner we tried avoiding using any non-organization type of libraries due to possible lack of future support reasons, but this author seems to be highly involved in the Go community and if you're okay with us using this library, I can re-implement it to do so.

I'm not sure if I can follow. What I am suggesting is: rather than creating the file go-coverage-atomic.out in your build and parsing this file in Jenkins, it looks much easier to call https://github.com/AlekSi/gocov-xml in your build pipeline. Then a cobertura.xml file will be created that can be read into Jenkins without any modification in the coverage-model.

At least it would make sense to use https://github.com/AlekSi/gocov-xml in a protoype to see if the created report contains more information than the plain go-coverage-atomic.out file?

So I am not against your parser in this PR but if it is not required because a better tool already exists it would make sense to follow the other approach...

And if this does not work we need to improve your parser so that the created model is correct.

pyieh · 2024-05-14T22:06:32Z

Wouldn't it make sense to use https://github.com/AlekSi/gocov-xml? Then we simply can reuse the Cobertura parser? Wouldn't there be also method names when used in that way?

@uhafner we tried avoiding using any non-organization type of libraries due to possible lack of future support reasons, but this author seems to be highly involved in the Go community and if you're okay with us using this library, I can re-implement it to do so.

I'm not sure if I can follow. What I am suggesting is: rather than creating the file go-coverage-atomic.out in your build and parsing this file in Jenkins, it looks much easier to call https://github.com/AlekSi/gocov-xml in your build pipeline. Then a cobertura.xml file will be created that can be read into Jenkins without any modification in the coverage-model.

At least it would make sense to use https://github.com/AlekSi/gocov-xml in a protoype to see if the created report contains more information than the plain go-coverage-atomic.out file?

So I am not against your parser in this PR but if it is not required because a better tool already exists it would make sense to follow the other approach...

And if this does not work we need to improve your parser so that the created model is correct.

Unfortunately, we're trying to automate and minimize the number of steps needed for our users to migrate over to this new plugin. And users are currently using that .out coverage format.
Can I ask you to clarify what's currently incorrect about my model? I thought with the updated/correct explanation of each line, my model shows the correct coverage data.

uhafner · 2024-05-15T06:34:59Z

Wouldn't it make sense to use https://github.com/AlekSi/gocov-xml? Then we simply can reuse the Cobertura parser? Wouldn't there be also method names when used in that way?

@uhafner we tried avoiding using any non-organization type of libraries due to possible lack of future support reasons, but this author seems to be highly involved in the Go community and if you're okay with us using this library, I can re-implement it to do so.

I'm not sure if I can follow. What I am suggesting is: rather than creating the file go-coverage-atomic.out in your build and parsing this file in Jenkins, it looks much easier to call https://github.com/AlekSi/gocov-xml in your build pipeline. Then a cobertura.xml file will be created that can be read into Jenkins without any modification in the coverage-model.
At least it would make sense to use https://github.com/AlekSi/gocov-xml in a protoype to see if the created report contains more information than the plain go-coverage-atomic.out file?
So I am not against your parser in this PR but if it is not required because a better tool already exists it would make sense to follow the other approach...
And if this does not work we need to improve your parser so that the created model is correct.

Unfortunately, we're trying to automate and minimize the number of steps needed for our users to migrate over to this new plugin. And users are currently using that .out coverage format. Can I ask you to clarify what's currently incorrect about my model? I thought with the updated/correct explanation of each line, my model shows the correct coverage data.

See #99 (comment)

Add parser for GoCov

b48b068

github-actions bot assigned uhafner Apr 23, 2024

Pierson Yieh added 3 commits April 22, 2024 18:00

Fix protected static JacocoParser.createValue(), update to new Covera…

9c22dc5

…geParser.parseReport() method declaration

Fix duplicate filename var

238b308

fix empty msg

783f455

github-advanced-security bot found potential problems Apr 23, 2024

View reviewed changes

Pierson Yieh added 3 commits April 22, 2024 18:15

add empty go cov file

4b3671b

Fix empty file test

969fbee

Fix empty file test

72fd13b

uhafner added the feature New features label Apr 23, 2024

uhafner requested changes Apr 23, 2024

View reviewed changes

uhafner marked this pull request as draft April 23, 2024 19:09

Pierson Yieh added 5 commits April 25, 2024 17:58

Move JacocoParser#createValue() to utility class ParserUtil

1c81239

Move createValue() out of util class to base CoverageParser class

87b89e1

Update coding style to match module

af370be

Update GoParserTest to reuse more of AbstractParserTest

b8ca86c

Revert unnecessary call

29da3be

pyieh requested a review from uhafner May 9, 2024 22:42

Simplify test case.

3ae1498

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

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parser for GoCov #99

Add parser for GoCov #99

pyieh commented Apr 23, 2024

Submitter checklist

uhafner left a comment

uhafner Apr 23, 2024

pyieh Apr 26, 2024 •

edited

Loading

uhafner Apr 26, 2024

pyieh May 9, 2024

uhafner May 13, 2024

pyieh May 15, 2024

uhafner May 16, 2024 •

edited

Loading

uhafner Jun 4, 2024

uhafner Apr 23, 2024

pyieh May 9, 2024

uhafner Apr 23, 2024

pyieh May 9, 2024

uhafner Apr 23, 2024

pyieh commented May 9, 2024

uhafner commented May 13, 2024

pyieh commented May 13, 2024

uhafner commented May 14, 2024

pyieh commented May 14, 2024 •

edited

Loading

uhafner commented May 15, 2024 •

edited

Loading

		@@ -0,0 +1,12 @@
		mode: atomic
		github.com/example/project/pkg/utils/file1.go:5.12,8.22 3 1

Add parser for GoCov #99

Are you sure you want to change the base?

Add parser for GoCov #99

Conversation

pyieh commented Apr 23, 2024

Testing done

Submitter checklist

uhafner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyieh Apr 26, 2024 • edited Loading

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 May 16, 2024 • edited Loading

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

pyieh commented May 9, 2024

uhafner commented May 13, 2024

pyieh commented May 13, 2024

uhafner commented May 14, 2024

pyieh commented May 14, 2024 • edited Loading

uhafner commented May 15, 2024 • edited Loading

pyieh Apr 26, 2024 •

edited

Loading

uhafner May 16, 2024 •

edited

Loading

pyieh commented May 14, 2024 •

edited

Loading

uhafner commented May 15, 2024 •

edited

Loading