Automatic parser detection #148

pyieh · 2024-05-13T21:40:59Z

Change to the plugin behavior when no parser is provided in the tool field. Previous behavior was to error out. Now, the following scenarios can occur with their corresponding behavior:

Note Simplified error message is in the format: Attempted parser <PARSER> with pattern <PATTERN>, but was unsuccessful.

No tool is defined (i.e. no parser or pattern) & at least 1 parser is successful:
- ex call)recordCoverage(tools: [])
- Try each possible parser with its corresponding default pattern
- Print the successful parsers' logs and a simplified error message for each parser that failed.
  Snippet of example output [Successful Go coverage, rest fail]:
No tool is defined (i.e. no parser or pattern) & all parsers fail
- ex call) recordCoverage(tools: [])
- Try each possible parser with its corresponding default pattern
- Print the logs of each parser tried.
  Snippet of example output [All fail]:
At least 1 pattern is defined, but none of them have a corresponding parser:
- ex call) recordCoverage(tools: [[pattern: '*coverage.out'], [pattern: '*coverage.xml']])
- Try each input pattern with each possible parser.
- Follows same ruling as above:
  - 1+ successful -> print verbose logs of successful combination, simplified error message of other combinations
  - All fail -> print verbose logs of all
    Snippet of example output [2 patterns for Go & Python coverage successfully get auto detected]:

Note Scenario 3 can occur even if another tool with a Parser is defined. Treats the expansion of that tool as 1 set.
ex call) recordCoverage(tools: [[parser: 'JACOCO'], [pattern: '*coverage.xml']])

Testing done

Tested on a local instance of Jenkins running the plugin version. Ran the aforementioned scenarios via pipeline jobs and recorded their builds' console output.
Also created unit tests that test the same thing.

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

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorder.java

+                        throw exception;
+                    }
+                }
+                if (localLog != null) {


uhafner · 2024-05-14T20:05:06Z

Can you please elaborate why this is useful? Is specifying the parser too complicated in your setup?

pyieh · 2024-05-14T22:19:05Z

Can you please elaborate why this is useful? Is specifying the parser too complicated in your setup?

Yeah, our usecase is that we have a number of pipelines (1000+) migrating over that currently just upload their coverage files to a server/service that determines the type of coverage file it is for them, so users aren't specifying anything about the coverage file type. We want to make the migration to using the recordCoverage() call in their pipelines as hands-off and minimal steps as possible, which involved moving the "What type of coverage is this?" determination functionality into the coverage-plugin.

uhafner · 2024-05-22T10:57:59Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageToolExpander.java

+ * Expands a list of given {@link CoverageTool}'s into the actual CoverageTool's set that {@link CoverageRecorder} uses when attempting to look for and parse coverage files.
+ * <p>
+ * The CoverageTool to its working set mapping use the following rules:
+ * - Empty or null tool set -&gt; set of all possible coverage parsers with null patterns (which will default to their default patterns)


Please check the coding style and the broken build

Suggested change

* - Empty or null tool set -> set of all possible coverage parsers with null patterns (which will default to their default patterns)

* - Empty or null tool set: set of all possible coverage parsers with null patterns (which will default to their default patterns)

uhafner · 2024-05-22T10:59:30Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageTool.java

@@ -46,7 +46,7 @@ public class CoverageTool extends AbstractDescribableImpl<CoverageTool> implemen
    private JenkinsFacade jenkins = new JenkinsFacade();

    private String pattern = StringUtils.EMPTY;
-    private Parser parser = Parser.JACOCO;
+    private Parser parser;


null values are not allowed in my code base

uhafner · 2024-05-22T11:01:01Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorder.java

@@ -471,52 +466,109 @@ private static void failStage(final ResultHandler resultHandler, final LogHandle
        logHandler.log(log);
    }

+    /**
+     * Iterates over the expansions of the {@code tools} items. For a given tool's expanded set it does the following logging behavior:
+     * - All fail -&gt; Logs all messages


Fix JavaDoc

uhafner · 2024-05-22T11:02:26Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorder.java

-                        parser.getDefaultPattern());
-            }
+        CoverageToolExpander covToolExpander = new CoverageToolExpander(this.tools, log, logHandler);
+        HashMap<CoverageTool, FilteredLog> parsersErrLogs = new HashMap<>();


Suggested change

HashMap<CoverageTool, FilteredLog> parsersErrLogs = new HashMap<>();

Map<CoverageTool, FilteredLog> parsersErrLogs = new HashMap<>();

uhafner · 2024-05-22T11:06:19Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorder.java

+                    localLog.logException(exception, "Exception while parsing with tool " + tool);
+                    parsersErrLogs.put(tool, localLog);
+                }
+                catch (InterruptedException | RuntimeException exception) {


Never catch a RuntimeException

uhafner · 2024-05-22T11:09:36Z

plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoveragePluginITest.java

-        project.getPublishersList().add(new CoverageRecorder());
-
-        verifyNoParserError(project);
-    }


Did you check Freestyle jobs and the UI configuration if the jobs?

You might also need to adapt the help files

uhafner · 2024-05-22T11:10:23Z

plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorderITest.java

+        WorkflowJob job = createPipeline();
+        job.setDefinition(new CpsFlowDefinition(
+                "node {\n"
+                        + "    recordCoverage tools: []\n"


Does it work with recordCoverage without tools as well? Then please add a test.

uhafner · 2024-05-22T11:53:51Z

plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorderITest.java

+
+        assertThat(getConsoleLog(run))
+                .contains("No tool defined, trying all possible parsers.")
+                .containsPattern("Successfully parsed file .*/jacoco.xml")


Does it say something about JaCoCo? Then please verify that text as well.

uhafner · 2024-05-22T11:56:28Z

plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageRecorderITest.java

+        copyFilesToWorkspace(job, "cobertura-higher-coverage.xml");
+        job.setDefinition(new CpsFlowDefinition(
+                "node {\n"
+                        + "    recordCoverage(tools: [[pattern: '*jacoco.xml'], [pattern: '*cobertura*.xml']])\n"


I'm not sure if it is really worth to the skip the parser names here.

While this approach works in general, I wonder what does the output look like when you merge the results of different parsers?

uhafner · 2024-05-22T11:57:47Z

plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageToolExpander.java

+        {
+            this.isExpanded = true;
+            for (Parser parser: Parser.values()) {
+                if (!(parser == Parser.JUNIT || parser == Parser.NUNIT || parser == Parser.XUNIT)) {


If you want to skip non-coverage files then use ParserType. And why do you want to skip these? Because your teams do not use them? This somehow is unexpected to users.

Pierson Yieh added 2 commits May 13, 2024 14:32

Updated tests for default parser feature

75616bd

Update behavior when no provided parser to try all default parsers

a528e31

github-actions bot assigned uhafner May 13, 2024

pyieh mentioned this pull request May 13, 2024

Add parser for Python coverage jenkinsci/coverage-model#103

Closed

Pierson Yieh added 2 commits May 13, 2024 16:17

Remove LOGGER call

c7266f2

Fix compilation errors

613cb0f

pyieh marked this pull request as draft May 13, 2024 23:28

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

View reviewed changes

uhafner changed the title ~~Default Parser~~ Automatic parser detection May 16, 2024

uhafner added the enhancement Enhancement of existing functionality label May 16, 2024

uhafner requested changes May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic parser detection #148

Automatic parser detection #148

pyieh commented May 13, 2024

Submitter checklist

uhafner commented May 14, 2024

pyieh commented May 14, 2024 •

edited

Loading

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

uhafner May 22, 2024

	* - Empty or null tool set -> set of all possible coverage parsers with null patterns (which will default to their default patterns)
	* - Empty or null tool set: set of all possible coverage parsers with null patterns (which will default to their default patterns)

	HashMap<CoverageTool, FilteredLog> parsersErrLogs = new HashMap<>();
	Map<CoverageTool, FilteredLog> parsersErrLogs = new HashMap<>();

Automatic parser detection #148

Are you sure you want to change the base?

Automatic parser detection #148

Conversation

pyieh commented May 13, 2024

Testing done

Submitter checklist

uhafner commented May 14, 2024

pyieh commented May 14, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyieh commented May 14, 2024 •

edited

Loading