Change lcov_merger flag parsing to allow legacy flags
- Improve lcov_merger flag parsing (use jcommander instead of custom parser)
- Allow usage of empty space as flag separator
- Allow unknown flags for for legacy lcov_merger convergence
- Allow simultaneous use of --coverage_dir value in favor of --report_file, the latter takes precedence.
PiperOrigin-RevId: 285043913
diff --git a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/BUILD b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/BUILD
index 70594bb..db22b5c 100644
--- a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/BUILD
+++ b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/BUILD
@@ -51,6 +51,7 @@
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
+ "//third_party/java/jcommander",
],
)
@@ -147,6 +148,7 @@
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
+ "//third_party/java/jcommander",
],
)
diff --git a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/LcovMergerFlags.java b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/LcovMergerFlags.java
index 798b737..9b5363c 100644
--- a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/LcovMergerFlags.java
+++ b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/LcovMergerFlags.java
@@ -14,23 +14,31 @@
package com.google.devtools.coverageoutputgenerator;
-import com.google.auto.value.AutoValue;
+import com.beust.jcommander.JCommander;
+import com.beust.jcommander.Parameter;
+import com.beust.jcommander.ParameterException;
+import com.beust.jcommander.Parameters;
import com.google.common.collect.ImmutableList;
import java.util.List;
+import java.util.logging.Logger;
import javax.annotation.Nullable;
-@AutoValue
-abstract class LcovMergerFlags {
+@Parameters(separators = "= ", optionPrefixes = "--")
+class LcovMergerFlags {
+ private static final Logger logger = Logger.getLogger(LcovMergerFlags.class.getName());
+
+ @Parameter(names = "--coverage_dir")
+ private String coverageDir;
@Nullable
- abstract String coverageDir();
+ @Parameter(names = {"--reports_file", "--lcovfile_path"})
+ private String reportsFile;
- @Nullable
- abstract String reportsFile();
+ @Parameter(names = "--output_file")
+ private String outputFile;
- abstract String outputFile();
-
- abstract List<String> filterSources();
+ @Parameter(names = "--filter_sources")
+ private List<String> filterSources;
/**
* The path to a source file manifest. This file contains multiple lines that represent file names
@@ -40,73 +48,61 @@
* @return
*/
@Nullable
- abstract String sourceFileManifest();
+ @Parameter(names = "--source_file_manifest")
+ private String sourceFileManifest;
@Nullable
- abstract String sourcesToReplaceFile();
+ @Parameter(names = "--sources_to_replace_file")
+ private String sourcesToReplaceFile;
- /** Parse flags in the form of "--coverage_dir=... -output_file=..." */
- static LcovMergerFlags parseFlags(String[] args) {
- ImmutableList.Builder<String> filterSources = new ImmutableList.Builder<>();
- String coverageDir = null;
- String reportsFile = null;
- String outputFile = null;
- String sourceFileManifest = null;
- String sourcesToReplaceFile = null;
+ public String coverageDir() {
+ return coverageDir;
+ }
- for (String arg : args) {
- if (!arg.startsWith("--")) {
- throw new IllegalArgumentException("Argument (" + arg + ") should start with --");
- }
- String[] parts = arg.substring(2).split("=", 2);
- if (parts.length != 2) {
- throw new IllegalArgumentException("There should be = in argument (" + arg + ")");
- }
- switch (parts[0]) {
- case "coverage_dir":
- coverageDir = parts[1];
- break;
- case "reports_file":
- reportsFile = parts[1];
- break;
- case "output_file":
- outputFile = parts[1];
- break;
- case "filter_sources":
- filterSources.add(parts[1]);
- break;
- case "source_file_manifest":
- sourceFileManifest = parts[1];
- break;
- case "sources_to_replace_file":
- sourcesToReplaceFile = parts[1];
- break;
- default:
- throw new IllegalArgumentException("Unknown flag " + arg);
- }
- }
+ public String outputFile() {
+ return outputFile;
+ }
- if (coverageDir == null && reportsFile == null) {
- throw new IllegalArgumentException(
- "At least one of --coverage_dir or --reports_file should be specified.");
- }
- if (coverageDir != null && reportsFile != null) {
- throw new IllegalArgumentException(
- "Only one of --coverage_dir or --reports_file must be specified.");
- }
- if (outputFile == null) {
- throw new IllegalArgumentException("--output_file was not specified.");
- }
- return new AutoValue_LcovMergerFlags(
- coverageDir,
- reportsFile,
- outputFile,
- filterSources.build(),
- sourceFileManifest,
- sourcesToReplaceFile);
+ public List<String> filterSources() {
+ return filterSources == null ? ImmutableList.of() : filterSources;
+ }
+
+ public String reportsFile() {
+ return reportsFile;
+ }
+
+ public String sourceFileManifest() {
+ return sourceFileManifest;
+ }
+
+ public String sourcesToReplaceFile() {
+ return sourcesToReplaceFile;
}
boolean hasSourceFileManifest() {
- return sourceFileManifest() != null;
+ return sourceFileManifest != null;
+ }
+
+ static LcovMergerFlags parseFlags(String[] args) {
+ LcovMergerFlags flags = new LcovMergerFlags();
+ JCommander jCommander = new JCommander(flags);
+ jCommander.setAllowParameterOverwriting(true);
+ jCommander.setAcceptUnknownOptions(true);
+ try {
+ jCommander.parse(args);
+ } catch (ParameterException e) {
+ throw new IllegalArgumentException("Error parsing args", e);
+ }
+ if (flags.coverageDir == null && flags.reportsFile == null) {
+ throw new IllegalArgumentException(
+ "At least one of coverage_dir or reports_file should be specified.");
+ }
+ if (flags.coverageDir != null && flags.reportsFile != null) {
+ logger.warning("Overriding --coverage_dir value in favor of --reports_file");
+ }
+ if (flags.outputFile == null) {
+ throw new IllegalArgumentException("output_file was not specified.");
+ }
+ return flags;
}
}
diff --git a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/Main.java b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/Main.java
index 1d215c1..0086dde 100644
--- a/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/Main.java
+++ b/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/Main.java
@@ -207,10 +207,10 @@
private static List<File> getTracefiles(LcovMergerFlags flags, List<File> filesInCoverageDir) {
List<File> lcovTracefiles = new ArrayList<>();
- if (flags.coverageDir() != null) {
- lcovTracefiles = getFilesWithExtension(filesInCoverageDir, TRACEFILE_EXTENSION);
- } else if (flags.reportsFile() != null) {
+ if (flags.reportsFile() != null) {
lcovTracefiles = getTracefilesFromFile(flags.reportsFile());
+ } else if (flags.coverageDir() != null) {
+ lcovTracefiles = getFilesWithExtension(filesInCoverageDir, TRACEFILE_EXTENSION);
}
if (lcovTracefiles.isEmpty()) {
logger.log(Level.INFO, "No lcov file found.");
diff --git a/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerFlagsTest.java b/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerFlagsTest.java
index 8ac8c9b..1599e85 100644
--- a/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerFlagsTest.java
+++ b/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerFlagsTest.java
@@ -17,7 +17,10 @@
import static com.google.common.truth.Truth.assertThat;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+@RunWith(JUnit4.class)
public class LcovMergerFlagsTest {
@Test
public void parseFlagsTestCoverageDirOutputFile() {
diff --git a/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerTestUtils.java b/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerTestUtils.java
index 9ccff5e..cfb6486 100644
--- a/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerTestUtils.java
+++ b/tools/test/CoverageOutputGenerator/javatests/com/google/devtools/coverageoutputgenerator/LcovMergerTestUtils.java
@@ -307,14 +307,14 @@
static void assertTracefile1(SourceFileCoverage sourceFile) {
Map<String, Integer> lineNumbers = sourceFile.getLineNumbers();
assertThat(lineNumbers.size()).isEqualTo(3);
- assertThat(lineNumbers.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(lineNumbers.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(lineNumbers.get(FUNC_1)).isEqualTo(FUNC_1_LINE_NR);
assertThat(lineNumbers.get(FUNC_2)).isEqualTo(FUNC_2_LINE_NR);
assertThat(lineNumbers.get(FUNC_3)).isEqualTo(FUNC_3_LINE_NR);
Map<String, Integer> functionsExecution = sourceFile.getFunctionsExecution();
assertThat(functionsExecution.size()).isEqualTo(3);
- assertThat(functionsExecution.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(functionsExecution.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(functionsExecution.get(FUNC_1)).isEqualTo(FUNC_1_NR_EXECUTED_LINES_TRACEFILE1);
assertThat(functionsExecution.get(FUNC_2)).isEqualTo(FUNC_2_NR_EXECUTED_LINES_TRACEFILE1);
assertThat(functionsExecution.get(FUNC_3)).isEqualTo(FUNC_3_NR_EXECUTED_LINES_TRACEFILE1);
@@ -328,14 +328,14 @@
static void assertTracefile2(SourceFileCoverage sourceFile) {
Map<String, Integer> lineNumbers = sourceFile.getLineNumbers();
assertThat(lineNumbers.size()).isEqualTo(3);
- assertThat(lineNumbers.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(lineNumbers.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(lineNumbers.get(FUNC_1)).isEqualTo(FUNC_1_LINE_NR);
assertThat(lineNumbers.get(FUNC_2)).isEqualTo(FUNC_2_LINE_NR);
assertThat(lineNumbers.get(FUNC_3)).isEqualTo(FUNC_3_LINE_NR);
Map<String, Integer> functionsExecution = sourceFile.getFunctionsExecution();
assertThat(functionsExecution.size()).isEqualTo(3);
- assertThat(functionsExecution.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(functionsExecution.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(functionsExecution.get(FUNC_1)).isEqualTo(FUNC_1_NR_EXECUTED_LINES_TRACEFILE2);
assertThat(functionsExecution.get(FUNC_2)).isEqualTo(FUNC_2_NR_EXECECUTED_LINES_TRACEFILE2);
assertThat(functionsExecution.get(FUNC_3)).isEqualTo(FUNC_3_NR_EXECUTED_LINES_TRACEFILE2);
@@ -348,7 +348,7 @@
static void assertMergedLineNumbers(TreeMap<String, Integer> lineNumbers) {
assertThat(lineNumbers.size()).isEqualTo(3);
- assertThat(lineNumbers.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(lineNumbers.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(lineNumbers.get(FUNC_1)).isEqualTo(FUNC_1_LINE_NR);
assertThat(lineNumbers.get(FUNC_2)).isEqualTo(FUNC_2_LINE_NR);
assertThat(lineNumbers.get(FUNC_3)).isEqualTo(FUNC_3_LINE_NR);
@@ -356,7 +356,7 @@
static void assertMergedFunctionsExecution(TreeMap<String, Integer> functionsExecution) {
assertThat(functionsExecution.size()).isEqualTo(3);
- assertThat(functionsExecution.keySet()).containsAllOf(FUNC_1, FUNC_2, FUNC_3);
+ assertThat(functionsExecution.keySet()).containsAtLeast(FUNC_1, FUNC_2, FUNC_3);
assertThat(functionsExecution.get(FUNC_1))
.isEqualTo(FUNC_1_NR_EXECUTED_LINES_TRACEFILE1 + FUNC_1_NR_EXECUTED_LINES_TRACEFILE2);
assertThat(functionsExecution.get(FUNC_2))
