SONARPY-985 Show UI warning when errors occur in coverage report parsing (#1103)

Author: andrea-guarino-sonarsource

sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonReportSensor.java

@@ -58,15 +58,15 @@ public void execute(SensorContext context) {
     String reportPathPropertyKey = reportPathKey();
     String reportPath = conf.get(reportPathPropertyKey).orElse(defaultReportPath());
     try {
-      List<File> reports = getReports(conf, context.fileSystem().baseDir().getPath(), reportPathPropertyKey, reportPath);
+      List<File> reports = getReports(conf, context.fileSystem().baseDir().getPath(), reportPathPropertyKey, reportPath, analysisWarnings);
       processReports(context, reports);
     } catch (Exception e) {
       LOG.warn("Cannot read report '{}', the following exception occurred: {}", reportPath, e.getMessage());
       analysisWarnings.addUnique(String.format("An error occurred while trying to import %s report(s): '%s'", reportType, reportPath));
     }
   }
 
-  public static List<File> getReports(Configuration conf, String baseDirPath, String reportPathPropertyKey, String reportPath) {
+  public static List<File> getReports(Configuration conf, String baseDirPath, String reportPathPropertyKey, String reportPath, AnalysisWarningsWrapper analysisWarnings) {
     LOG.debug("Using pattern '{}' to find reports", reportPath);
 
     FileProvider provider = new FileProvider(new File(baseDirPath), reportPath);
@@ -77,7 +77,9 @@ public static List<File> getReports(Configuration conf, String baseDirPath, Stri
         // try absolute path
         File file = new File(reportPath);
         if (!file.exists()) {
-          LOG.warn("No report was found for {} using pattern {}", reportPathPropertyKey, reportPath);
+          String formattedMessage = String.format("No report was found for %s using pattern %s", reportPathPropertyKey, reportPath);
+          LOG.warn(formattedMessage);
+          analysisWarnings.addUnique(formattedMessage);
         } else {
           matchingFiles.add(file);
         }

sonar-python-plugin/src/main/java/org/sonar/plugins/python/coverage/CoberturaParser.java

@@ -22,8 +22,10 @@
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import javax.xml.stream.XMLStreamException;
@@ -42,6 +44,7 @@ public class CoberturaParser {
 
   private static final Logger LOG = Loggers.get(CoberturaParser.class);
 
+  private final Set<String> errors = new HashSet<>();
   private int unresolvedFilenameCount;
 
   public void parseReport(File xmlFile, SensorContext context, final Map<InputFile, NewCoverage> coverageData) throws XMLStreamException {
@@ -68,11 +71,13 @@ public void parseReport(File xmlFile, SensorContext context, final Map<InputFile
     });
     parser.parse(xmlFile);
     if (unresolvedFilenameCount > 1) {
-      LOG.error("Cannot resolve {} file paths, ignoring coverage measures for those files", unresolvedFilenameCount);
+      String message = String.format("Cannot resolve %d file paths, ignoring coverage measures for those files", unresolvedFilenameCount);
+      LOG.error(message);
+      errors.add(message);
     }
   }
 
-  private static List<File> extractBaseDirectories(SMInputCursor sources, File defaultBaseDirectory) throws XMLStreamException {
+  private List<File> extractBaseDirectories(SMInputCursor sources, File defaultBaseDirectory) throws XMLStreamException {
     List<File> baseDirectories = new ArrayList<>();
     SMInputCursor source = sources.childElementCursor("source");
     while (source.getNext() != null) {
@@ -82,7 +87,9 @@ private static List<File> extractBaseDirectories(SMInputCursor sources, File def
         if (baseDirectory.isDirectory()) {
           baseDirectories.add(baseDirectory);
         } else {
-          LOG.warn("Invalid directory path in 'source' element: {}", path);
+          String formattedMessage = String.format("Invalid directory path in 'source' element: %s", path);
+          LOG.warn(formattedMessage);
+          errors.add(formattedMessage);
         }
       }
     }
@@ -112,7 +119,7 @@ private InputFile resolve(SensorContext context, List<File> baseDirectories, Str
     File file = new File(filename);
     if (file.isAbsolute()) {
       if (!file.exists()) {
-        logUnresolvedFile("Cannot resolve the file path '{}' of the coverage report, the file does not exist in all <source>.", filename);
+        logUnresolvedFile("Cannot resolve the file path '%s' of the coverage report, the file does not exist in all 'source'.", filename);
       }
       absolutePath = file.getAbsolutePath();
     } else {
@@ -121,11 +128,11 @@ private InputFile resolve(SensorContext context, List<File> baseDirectories, Str
         .filter(File::exists)
         .collect(Collectors.toList());
       if (fileList.isEmpty()) {
-        logUnresolvedFile("Cannot resolve the file path '{}' of the coverage report, the file does not exist in all <source>.", filename);
+        logUnresolvedFile("Cannot resolve the file path '%s' of the coverage report, the file does not exist in all 'source'.", filename);
         return null;
       }
       if (fileList.size() > 1) {
-        logUnresolvedFile("Cannot resolve the file path '{}' of the coverage report, ambiguity, the file exists in several <source>.", filename);
+        logUnresolvedFile("Cannot resolve the file path '%s' of the coverage report, ambiguity, the file exists in several 'source'.", filename);
         return null;
       }
       absolutePath = fileList.get(0).getAbsolutePath();
@@ -136,7 +143,9 @@ private InputFile resolve(SensorContext context, List<File> baseDirectories, Str
   private void logUnresolvedFile(String message, String filename) {
     unresolvedFilenameCount++;
     if (unresolvedFilenameCount == 1) {
-      LOG.error(message, filename);
+      String formattedMessage = String.format(message, filename);
+      LOG.error(formattedMessage);
+      errors.add(formattedMessage);
     }
   }
 
@@ -154,4 +163,8 @@ private static void collectFileData(SMInputCursor classCursor, NewCoverage cover
       }
     }
   }
+
+  public Set<String> errors() {
+    return errors;
+  }
 }

sonar-python-plugin/src/main/java/org/sonar/plugins/python/coverage/PythonCoverageSensor.java

@@ -84,13 +84,13 @@ public void execute(SensorContext context) {
     }
   }
 
-  private static List<File> getCoverageReports(String baseDir, Configuration config) {
+  private List<File> getCoverageReports(String baseDir, Configuration config) {
     if (!config.hasKey(REPORT_PATHS_KEY)) {
-      return getReports(config, baseDir, REPORT_PATHS_KEY, DEFAULT_REPORT_PATH);
+      return getReports(config, baseDir, REPORT_PATHS_KEY, DEFAULT_REPORT_PATH, analysisWarnings);
     }
 
     return Arrays.stream(config.getStringArray(REPORT_PATHS_KEY))
-      .flatMap(path -> getReports(config, baseDir, REPORT_PATHS_KEY, path).stream())
+      .flatMap(path -> getReports(config, baseDir, REPORT_PATHS_KEY, path, analysisWarnings).stream())
       .collect(Collectors.toList());
   }
 
@@ -108,12 +108,17 @@ private static Set<File> uniqueAbsolutePaths(List<File> reports) {
       .collect(Collectors.toCollection(LinkedHashSet::new));
   }
 
-  private static Map<InputFile, NewCoverage> parseReport(File report, SensorContext context) {
+  private Map<InputFile, NewCoverage> parseReport(File report, SensorContext context) {
     Map<InputFile, NewCoverage> coverageMeasures = new HashMap<>();
     try {
       CoberturaParser parser = new CoberturaParser();
       parser.parseReport(report, context, coverageMeasures);
+      if (!parser.errors().isEmpty()) {
+        String parseErrors = String.join("%n", parser.errors());
+        analysisWarnings.addUnique(String.format("The following error(s) occurred while trying to import coverage report:%n%s", parseErrors));
+      }
     } catch (EmptyReportException e) {
+      analysisWarnings.addUnique(String.format("The coverage report '%s' has been ignored because it seems to be empty.", report));
       LOG.warn("The report '{}' seems to be empty, ignoring. '{}'", report, e);
     } catch (XMLStreamException e) {
       throw new IllegalStateException("Error parsing the report '" + report + "'", e);

sonar-python-plugin/src/test/java/org/sonar/plugins/python/coverage/PythonCoverageSensorTest.java

@@ -237,7 +237,7 @@ public void test_unresolved_path() {
     coverageSensor.execute(context);
 
     String expectedLogMessage = String.format(
-      "Cannot resolve the file path '%sabsolute%ssources%snot_exist.py' of the coverage report, the file does not exist in all <source>.",
+      "Cannot resolve the file path '%sabsolute%ssources%snot_exist.py' of the coverage report, the file does not exist in all 'source'.",
       currentFileSeparator,
       currentFileSeparator,
       currentFileSeparator);
@@ -300,6 +300,16 @@ public void should_do_nothing_on_empty_report() {
     assertThat(context.lineHits(FILE1_KEY, 1)).isNull();
   }
 
+  @Test
+  public void should_warn_if_source_is_not_directory() {
+    settings.setProperty(PythonCoverageSensor.REPORT_PATHS_KEY, "coverage_source_invalid_directory.xml");
+    coverageSensor.execute(context);
+    File file = new File("src/test/resources/org/sonar/plugins/python/coverage-reports/sources/file1.py");
+    String message = "Invalid directory path in 'source' element: " + file.getPath();
+    assertThat(logTester.logs(LoggerLevel.WARN)).contains(message);
+    verify(analysisWarnings, times(1)).addUnique("The following error(s) occurred while trying to import coverage report:" + System.lineSeparator() + message);
+  }
+
   @Test
   public void no_default_report_log() {
     settings.clear();

sonar-python-plugin/src/test/resources/org/sonar/plugins/python/coverage-reports/coverage_source_invalid_directory.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" ?>
+<coverage branch-rate="0.5" branches-covered="2" branches-valid="4" complexity="0" line-rate="0.75" lines-covered="6" lines-valid="8" timestamp="1515594208483" version="4.4.2">
+	<!-- Generated by coverage.py: https://coverage.readthedocs.io -->
+	<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
+	<sources>
+		<source>src/test/resources/org/sonar/plugins/python/coverage-reports/sources/file1.py</source>
+	</sources>
+	<packages></packages>
+</coverage>