Merge pull request #985 from jmecosta/bugfix/handle-invalid-issues

implement recovery for issues
SonarOpenCommunity · Nov 5, 2016 · b1e0841 · b1e0841
2 parents 985c0d7 + 7bb841e
commit b1e0841
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 37 deletions.
diff --git a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/coverage/CxxCoverageSensor.java b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/coverage/CxxCoverageSensor.java
@@ -39,6 +39,7 @@
 import org.sonar.plugins.cxx.CxxLanguage;
 import org.sonar.plugins.cxx.CxxPlugin;
 import org.sonar.plugins.cxx.utils.CxxReportSensor;
+import org.sonar.plugins.cxx.utils.CxxUtils;
 
 /**
  * {@inheritDoc}
@@ -161,10 +162,7 @@ private void saveMeasures(SensorContext context,
               newCoverage.lineHits(measure.getLine(), measure.getHits());
             } catch(Exception ex) {
               LOG.error("Cannot save Line Hits for Line '{}' '{}' : '{}', ignoring measure", filePath, measure.getLine(), ex.getMessage());
-              if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-                LOG.info("Recovery is disabled, failing analysis.");
-                throw ex;
-              }
+              CxxUtils.ValidateRecovery(ex, settings);
             }            
           }
 
@@ -174,10 +172,7 @@ private void saveMeasures(SensorContext context,
               newCoverage.conditions(measure.getLine(), measure.getConditions(), measure.getCoveredConditions());
             } catch(Exception ex) {
               LOG.error("Cannot save Conditions Hits for Line '{}' '{}' : '{}', ignoring measure", filePath, measure.getLine(), ex.getMessage());
-              if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-                LOG.info("Recovery is disabled, failing analysis.");
-                throw ex;
-              }
+              CxxUtils.ValidateRecovery(ex, settings);
             }                         
           }                             
         }
@@ -187,10 +182,7 @@ private void saveMeasures(SensorContext context,
           newCoverage.save();
         } catch(Exception ex) {
           LOG.error("Cannot save measure '{}' : '{}', ignoring measure", filePath, ex.getMessage());
-          if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-            LOG.info("Recovery is disabled, failing analysis.");
-            throw ex;
-          }
+          CxxUtils.ValidateRecovery(ex, settings);
         }        
       } else {
         LOG.debug("Cannot find the file '{}', ignoring coverage measures", filePath);

diff --git a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/tests/xunit/CxxXunitSensor.java b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/tests/xunit/CxxXunitSensor.java
@@ -51,6 +51,7 @@
 import org.sonar.plugins.cxx.CxxLanguage;
 import org.sonar.plugins.cxx.CxxPlugin;
 import static org.sonar.plugins.cxx.coverage.CxxCoverageSensor.LOG;
+import org.sonar.plugins.cxx.utils.CxxUtils;
 import org.sonar.plugins.cxx.utils.StaxParser;
 
 /**
@@ -165,10 +166,7 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
            .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure TESTS : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       }       
 
       try
@@ -180,10 +178,7 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
          .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure TEST_ERRORS : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       } 
 
       try
@@ -195,10 +190,7 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
          .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure TEST_FAILURES : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       } 
 
       try
@@ -210,10 +202,7 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
          .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure SKIPPED_TESTS : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       } 
 
       try
@@ -225,10 +214,7 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
          .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure TEST_SUCCESS_DENSITY : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       }       
 
       try
@@ -240,16 +226,15 @@ private void simpleMode(final SensorContext context, List<TestCase> testcases)
          .save();
       } catch(Exception ex) {
         LOG.error("Cannot save measure TEST_EXECUTION_TIME : '{}', ignoring measure", ex.getMessage());
-        if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
-          LOG.info("Recovery is disabled, failing analysis.");
-          throw ex;
-        }
+        CxxUtils.ValidateRecovery(ex, settings);
       }       
     } else {
       LOG.debug("The reports contain no testcases");
     }      
   }
 
+
+
   File transformReport(File report)
     throws java.io.IOException, javax.xml.transform.TransformerException {
     File transformed = report;

diff --git a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java
@@ -85,6 +85,7 @@ public void execute(SensorContext context) {
              violationsCount - prevViolationsCount);
         } catch (EmptyReportException e) {
           LOG.warn("The report '{}' seems to be empty, ignoring.", report);
+          CxxUtils.ValidateRecovery(e, settings);
         }
       }
 
@@ -220,14 +221,15 @@ private void saveViolation(SensorContext sensorContext, String ruleRepoKey, Stri
             violationsCount++;
           } catch (Exception ex) {
             LOG.error("Could not add the issue '{}', skipping issue", ex.getMessage());
+            CxxUtils.ValidateRecovery(ex, settings);
           }
         } else {
           LOG.warn("Cannot find the file '{}', skipping violations", normalPath);
           notFoundFiles.add(normalPath);
         }
       }
     } else { // project level
-
+      try {
         NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(ruleRepoKey, ruleId));
         NewIssueLocation location = newIssue.newLocation()
           .on(sensorContext.module())
@@ -236,6 +238,10 @@ private void saveViolation(SensorContext sensorContext, String ruleRepoKey, Stri
         newIssue.at(location);
         newIssue.save();
         violationsCount++;
+      } catch (Exception ex) {
+        LOG.error("Could not add the issue '{}', skipping issue", ex.getMessage());
+        CxxUtils.ValidateRecovery(ex, settings);
+      }
     }
   }
 
@@ -251,6 +257,7 @@ private int getLineAsInt(String line, int maxLine) {
         }
       } catch (java.lang.NumberFormatException nfe) {
         LOG.warn("Skipping invalid line number: {}", line);
+        CxxUtils.ValidateRecovery(nfe, settings);
         lineNr = -1;
       }
     }

diff --git a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxUtils.java b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxUtils.java
@@ -22,8 +22,10 @@
 import java.io.File;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import org.sonar.api.config.Settings;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.plugins.cxx.CxxPlugin;
 
 
 /**
@@ -82,4 +84,11 @@ public static String getStackTrace(final Throwable throwable) {
     throwable.printStackTrace(pw);
     return sw.getBuffer().toString();
   }
+
+  public static void ValidateRecovery(Exception ex, Settings settings) throws IllegalStateException {
+    if (!settings.getBoolean(CxxPlugin.ERROR_RECOVERY_KEY)) {
+      LOG.info("Recovery is disabled, failing analysis : '{}'", ex.toString());
+      throw new IllegalStateException(ex.getMessage(), ex.getCause());
+    }
+  }
 }
diff --git a/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/cppcheck/CxxCppCheckSensorTest.java b/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/cppcheck/CxxCppCheckSensorTest.java
@@ -30,6 +30,7 @@
 
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
+import org.sonar.plugins.cxx.CxxPlugin;
 
 public class CxxCppCheckSensorTest {
 
@@ -47,6 +48,7 @@ public void shouldReportCorrectViolations() {
     SensorContextTester context = SensorContextTester.create(new File("src/samples/SampleProject"));
     settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY, fs.baseDir().getAbsolutePath() + 
       "/cppcheck-reports/cppcheck-result-*.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
     context.fileSystem().add(new DefaultInputFile("myProjectKey", "sources/utils/code_chunks.cpp").setLanguage("cpp").initMetadata(new String("asd\nasdas\nasda\n")));
     context.fileSystem().add(new DefaultInputFile("myProjectKey", "sources/utils/utils.cpp").setLanguage("cpp").initMetadata(new String("asd\nasdas\nasda\n")));
@@ -59,6 +61,7 @@ public void shouldReportProjectLevelViolationsV1() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY,
       "cppcheck-reports/cppcheck-result-projectlevelviolation-V1.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
     sensor.execute(context);
     assertThat(context.allIssues()).hasSize(3);
@@ -69,6 +72,7 @@ public void shouldReportProjectLevelViolationsV2() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY,
       "cppcheck-reports/cppcheck-result-projectlevelviolation-V2.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
     sensor.execute(context);
     assertThat(context.allIssues()).hasSize(3);
@@ -79,6 +83,7 @@ public void shouldIgnoreAViolationWhenTheResourceCouldntBeFoundV1() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY,
       "cppcheck-reports/cppcheck-result-SAMPLE-V1.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
     sensor.execute(context);
     assertThat(context.allIssues()).hasSize(0);
@@ -89,8 +94,19 @@ public void shouldIgnoreAViolationWhenTheResourceCouldntBeFoundV2() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY,
       "cppcheck-reports/cppcheck-result-SAMPLE-V2.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
     sensor.execute(context);
     assertThat(context.allIssues()).hasSize(0);
   }
+
+  @Test(expected=IllegalStateException.class)
+  public void shouldThrowExceptionWhenRecoveryIsDisabled() {
+    SensorContextTester context = SensorContextTester.create(fs.baseDir());
+    settings.setProperty(CxxCppCheckSensor.REPORT_PATH_KEY,
+      "cppcheck-reports/cppcheck-result-empty.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "False");
+    CxxCppCheckSensor sensor = new CxxCppCheckSensor(settings);
+    sensor.execute(context);
+  }  
 }
diff --git a/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/pclint/CxxPCLintSensorTest.java b/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/pclint/CxxPCLintSensorTest.java
@@ -32,6 +32,7 @@
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
 import org.sonar.api.batch.sensor.issue.Issue;
+import org.sonar.plugins.cxx.CxxPlugin;
 
 public class CxxPCLintSensorTest {
   private DefaultFileSystem fs;
@@ -96,6 +97,7 @@ public void shouldNotSaveIssuesWhenMisra2004DescIsWrong() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     Settings settings = new Settings();
     settings.setProperty(CxxPCLintSensor.REPORT_PATH_KEY, "pclint-reports/incorrect-pclint-MISRA2004-desc.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     context.fileSystem().add(new DefaultInputFile("myProjectKey", "test.c").setLanguage("cpp").initMetadata(new String("asd\nasdas\nasda\n")));
     CxxPCLintSensor sensor = new CxxPCLintSensor(settings);
     sensor.execute(context);
@@ -107,6 +109,7 @@ public void shouldNotSaveAnythingWhenMisra2004RuleDoNotExist() {
     SensorContextTester context = SensorContextTester.create(fs.baseDir());
     Settings settings = new Settings();
     settings.setProperty(CxxPCLintSensor.REPORT_PATH_KEY, "pclint-reports/incorrect-pclint-MISRA2004-rule-do-not-exist.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     context.fileSystem().add(new DefaultInputFile("myProjectKey", "test.c").setLanguage("cpp").initMetadata(new String("asd\nasdas\nasda\n")));
     CxxPCLintSensor sensor = new CxxPCLintSensor(settings);
     sensor.execute(context);

diff --git a/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/veraxx/CxxVeraxxSensorTest.java b/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/veraxx/CxxVeraxxSensorTest.java
@@ -29,6 +29,7 @@
 
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
+import org.sonar.plugins.cxx.CxxPlugin;
 
 public class CxxVeraxxSensorTest {
 
@@ -40,6 +41,7 @@ public void setUp() {
     fs = TestUtils.mockFileSystem();
     Settings settings = new Settings();
     settings.setProperty(CxxVeraxxSensor.REPORT_PATH_KEY, "vera++-reports/vera++-result-*.xml");
+    settings.setProperty(CxxPlugin.ERROR_RECOVERY_KEY, "True");
     sensor = new CxxVeraxxSensor(settings);
   }