Merge pull request #22 from Pablissimo/issue21

Fix issue #21 - Longer TsLint timeout and now configurable via sonar.ts.tslinttimeout project property

pom.xml

@@ -5,7 +5,7 @@
   <groupId>com.pablissimo.sonar</groupId>
   <artifactId>sonar-typescript-plugin</artifactId>
   <packaging>sonar-plugin</packaging>
-  <version>0.2-SNAPSHOT</version>
+  <version>0.3-SNAPSHOT</version>
 
   <name>TypeScript</name>
   <description>Analyse TypeScript projects</description>
@@ -14,7 +14,7 @@
     <connection>scm:git:git@github.com:Pablissimo/SonarTsPlugin.git</connection>
     <developerConnection>scm:git:git@github.com:Pablissimo/SonarTsPlugin.git</developerConnection>
     <url>https://github.com/Pablissimo/SonarTsPlugin</url>
-    <tag>0.2</tag>
+    <tag>0.3</tag>
   </scm>
 
   <properties>

src/main/java/com/pablissimo/sonar/TsLintExecutor.java

@@ -1,5 +1,5 @@
 package com.pablissimo.sonar;
 
 public interface TsLintExecutor {
-    String execute(String pathToTsLint, String configFile, String file);
+    String execute(String pathToTsLint, String configFile, String file, Integer timeoutMs);
 }

src/main/java/com/pablissimo/sonar/TsLintExecutorImpl.java

@@ -12,7 +12,7 @@ public class TsLintExecutorImpl implements TsLintExecutor {
     private StringBuilder stdOut;
     private StringBuilder stdErr;
 
-    public String execute(String pathToTsLint, String configFile, String file) {
+    public String execute(String pathToTsLint, String configFile, String file, Integer timeoutMs) {
         LOG.info("TsLint executing for " + file);
         Command command = Command.create("node");
         command.addArgument(pathToTsLint);
@@ -40,7 +40,7 @@ public void consumeLine(String line) {
             }
         };
 
-        this.createExecutor().execute(command, stdOutConsumer, stdErrConsumer, 5000);
+        this.createExecutor().execute(command, stdOutConsumer, stdErrConsumer, timeoutMs);
 
         return stdOut.toString();
     }

src/main/java/com/pablissimo/sonar/TsLintSensor.java

@@ -66,6 +66,7 @@ private boolean hasFilesToAnalyze() {
     public void analyse(Project project, SensorContext context) {
         String pathToTsLint = settings.getString(TypeScriptPlugin.SETTING_TS_LINT_PATH);
         String pathToTsLintConfig = settings.getString(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH);
+        Integer tsLintTimeoutMs = Math.max(5000, settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT));
 
         if (pathToTsLint == null) {
             LOG.warn("Path to tslint (" + TypeScriptPlugin.SETTING_TS_LINT_PATH + ") is not defined. Skipping tslint analysis.");
@@ -96,7 +97,7 @@ else if (pathToTsLintConfig == null) {
             Resource resource = this.getFileFromIOFile(file, project);
             Issuable issuable = perspectives.as(Issuable.class, resource);
 
-            String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, file.getAbsolutePath());
+            String jsonResult = executor.execute(pathToTsLint, pathToTsLintConfig, file.getAbsolutePath(), tsLintTimeoutMs);
 
             TsLintIssue[] issues = parser.parse(jsonResult);
 

src/main/java/com/pablissimo/sonar/TypeScriptPlugin.java

@@ -51,13 +51,23 @@
         description = "Path to the file that configures the rules to be included in analysis",
         project = true,
         global = false
+    ),
+    @Property(
+        key = TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT,
+        defaultValue = "60000",
+        type = PropertyType.INTEGER,
+        name = "Max TsLint wait time (milliseconds)",
+        description = "Maximum time to wait for TsLint execution to finish before aborting (in milliseconds)",
+        project = true,
+        global = false
     )
 })
 public class TypeScriptPlugin extends SonarPlugin {
     public static final String SETTING_EXCLUDE_TYPE_DEFINITION_FILES = "sonar.ts.excludetypedefinitionfiles";
     public static final String SETTING_FORCE_ZERO_COVERAGE = "sonar.ts.forceZeroCoverage";
     public static final String SETTING_TS_LINT_PATH = "sonar.ts.tslintpath";
     public static final String SETTING_TS_LINT_CONFIG_PATH = "sonar.ts.tslintconfigpath";
+    public static final String SETTING_TS_LINT_TIMEOUT = "sonar.ts.tslinttimeout";
     public static final String SETTING_LCOV_REPORT_PATH = "sonar.ts.lcov.reportpath";
 
     public List getExtensions() {

src/test/java/com/pablissimo/sonar/TsLintExecutorImplTest.java

@@ -26,22 +26,26 @@ public void setUp() throws Exception {
     @Test
     public void executesCommandWithCorrectArguments() {
         final ArrayList<Command> capturedCommands = new ArrayList<Command>();
+        final ArrayList<Long> capturedTimeouts = new ArrayList<Long>();
 
         Answer<Integer> captureCommand = new Answer<Integer>() {
             @Override
             public Integer answer(InvocationOnMock invocation) throws Throwable {
                 capturedCommands.add((Command) invocation.getArguments()[0]);
+                capturedTimeouts.add((long) invocation.getArguments()[3]);
                 return 0;
             }
         };
 
         when(this.commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), any(long.class))).then(captureCommand);
-        this.executorImpl.execute("path/to/tslint", "path/to/config", "path/to/file");
+        this.executorImpl.execute("path/to/tslint", "path/to/config", "path/to/file", 40000);
 
         assertEquals(1, capturedCommands.size());
 
         Command theCommand = capturedCommands.get(0);
+        long theTimeout = capturedTimeouts.get(0);
 
         assertEquals("node path/to/tslint --format json --config path/to/config path/to/file", theCommand.toCommandLine());
+        assertEquals(40000, theTimeout);        
     }
 }

src/test/java/com/pablissimo/sonar/TsLintSensorTest.java

@@ -51,6 +51,7 @@ public void setUp() throws Exception {
         this.settings = mock(Settings.class);
         when(this.settings.getString(TypeScriptPlugin.SETTING_TS_LINT_PATH)).thenReturn("/path/to/tslint");
         when(this.settings.getString(TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH)).thenReturn("/path/to/tslint.json");
+        when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(45000);
 
         this.fileSystem = mock(FileSystem.class);
         this.perspectives = mock(ResourcePerspectives.class);
@@ -149,4 +150,20 @@ public void analyse_doesNothingWhenNoConfigPathset() throws IOException {
 
         verify(this.issuable, never()).addIssue(any(Issue.class));
     }
+
+    @Test
+    public void analyse_callsExecutorWithSuppliedTimeout() throws IOException {
+        this.sensor.analyse(mock(Project.class), mock(SensorContext.class));
+
+        verify(this.executor, times(1)).execute(any(String.class), any(String.class), any(String.class), eq(45000));
+    }
+
+    @Test
+    public void analyze_callsExecutorWithAtLeast5000msTimeout() throws IOException {
+        when(this.settings.getInt(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT)).thenReturn(-500);
+
+        this.sensor.analyse(mock(Project.class), mock(SensorContext.class));
+
+        verify(this.executor, times(1)).execute(any(String.class), any(String.class), any(String.class), eq(5000));
+    }
 }

src/test/java/com/pablissimo/sonar/TypeScriptPluginTest.java

@@ -53,7 +53,7 @@ public void definesExpectedProperties() {
         Annotation annotation = plugin.getClass().getAnnotations()[0];
         Properties propertiesAnnotation = (Properties) annotation;
 
-        assertEquals(5, propertiesAnnotation.value().length);
+        assertEquals(6, propertiesAnnotation.value().length);
 
         Property[] properties = propertiesAnnotation.value();
         assertNotNull(findPropertyByName(properties,
@@ -66,6 +66,8 @@ public void definesExpectedProperties() {
                 TypeScriptPlugin.SETTING_TS_LINT_PATH));
         assertNotNull(findPropertyByName(properties,
                 TypeScriptPlugin.SETTING_TS_LINT_CONFIG_PATH));
+        assertNotNull(findPropertyByName(properties,
+                TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT));
     }
 
     @Test
@@ -108,6 +110,16 @@ public void forceZeroCoverageSetting_definedAppropriately() {
         assertEquals(false, property.global());
     }
 
+    @Test
+    public void tsLintTimeoutSettings_definedAppropriately() {
+        Property property = findPropertyByName(TypeScriptPlugin.SETTING_TS_LINT_TIMEOUT);
+
+        assertEquals(PropertyType.INTEGER, property.type());
+        assertEquals("60000", property.defaultValue());
+        assertEquals(true, property.project());
+        assertEquals(false, property.global());
+    }
+
     private Property findPropertyByName(String property) {
         return findPropertyByName(((Properties) plugin.getClass()
                 .getAnnotations()[0]).value(), property);