Reuse logic from SonarJS to discover node executable (#173)

* Reuse logic from SonarJS to discover node executable

* change min size

* Fix lookup of node from maven plugin on Win

4 files changed, 43 insertions, 117 deletions

diff --git a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssPluginTest.java b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssPluginTest.java
index b8c5cd4..792f065 100644
--- a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssPluginTest.java
+++ b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssPluginTest.java
@@ -36,7 +36,7 @@ public class CssPluginTest {
     Plugin.Context context = new Plugin.Context(runtime);
     Plugin underTest = new CssPlugin();
     underTest.define(context);
-    assertThat(context.getExtensions()).hasSize(11);
+    assertThat(context.getExtensions()).hasSize(10);
   }
 
   @Test
@@ -45,7 +45,7 @@ public class CssPluginTest {
     Plugin.Context context = new Plugin.Context(runtime);
     Plugin underTest = new CssPlugin();
     underTest.define(context);
-    assertThat(context.getExtensions()).hasSize(12);
+    assertThat(context.getExtensions()).hasSize(11);
   }
 
   @Test
@@ -54,7 +54,7 @@ public class CssPluginTest {
     Plugin.Context context = new Plugin.Context(runtime);
     Plugin underTest = new CssPlugin();
     underTest.define(context);
-    assertThat(context.getExtensions()).hasSize(13);
+    assertThat(context.getExtensions()).hasSize(12);
   }
 
   @Test
@@ -63,6 +63,6 @@ public class CssPluginTest {
     Plugin.Context context = new Plugin.Context(runtime);
     Plugin underTest = new CssPlugin();
     underTest.define(context);
-    assertThat(context.getExtensions()).hasSize(12);
+    assertThat(context.getExtensions()).hasSize(11);
   }
 }
diff --git a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java
index df3d034..f7f0d31 100644
--- a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java
+++ b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/CssRuleSensorTest.java
@@ -27,6 +27,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.awaitility.Awaitility;
 import org.junit.Before;
@@ -42,16 +43,16 @@ import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
 import org.sonar.api.batch.sensor.issue.Issue;
-import org.sonar.api.config.Configuration;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.css.plugin.bundle.BundleHandler;
 import org.sonar.css.plugin.bundle.CssBundleHandler;
+import org.sonarsource.nodejs.NodeCommand;
+import org.sonarsource.nodejs.NodeCommandException;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.ArgumentMatchers.matches;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -107,37 +108,12 @@ public class CssRuleSensorTest {
 
   @Test
   public void test_invalid_node() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable += " " + TestLinterCommandProvider.resourceScript("/executables/invalidNodeVersion.js");
+    InvalidCommandProvider commandProvider = new InvalidCommandProvider();
     CssRuleSensor sensor = createCssRuleSensor(commandProvider);
     sensor.execute(context);
 
     assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Failed to parse Node.js version, got 'Invalid version'. No CSS files will be analyzed.");
-    verifyZeroInteractions(analysisWarnings);
-  }
-
-  @Test
-  public void test_no_node() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable = TestLinterCommandProvider.resourceScript("/executables/invalidNodeVersion.js");
-    CssRuleSensor sensor = createCssRuleSensor(commandProvider);
-    sensor.execute(context);
-
-    assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Failed to get Node.js version. No CSS files will be analyzed.");
-    verifyZeroInteractions(analysisWarnings);
-  }
-
-  @Test
-  public void test_old_node() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable += " " + TestLinterCommandProvider.resourceScript("/executables/oldNodeVersion.js");
-    CssRuleSensor sensor = createCssRuleSensor(commandProvider);
-    sensor.execute(context);
-
-    assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Only Node.js v6 or later is supported, got 3.2.1. No CSS files will be analyzed.");
+    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Some problem happened. No CSS files will be analyzed.");
     verifyZeroInteractions(analysisWarnings);
   }
 
@@ -157,39 +133,14 @@ public class CssRuleSensorTest {
   }
 
   @Test
-  public void test_invalid_node_with_analysisWarnings() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable += " " + TestLinterCommandProvider.resourceScript("/executables/invalidNodeVersion.js");
-    CssRuleSensor sensor = createCssRuleSensor(commandProvider, analysisWarnings);
-    sensor.execute(context);
-
-    assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Failed to parse Node.js version, got 'Invalid version'. No CSS files will be analyzed.");
-    verify(analysisWarnings).addUnique(eq("CSS files were not analyzed. Failed to parse Node.js version, got 'Invalid version'."));
-  }
-
-  @Test
-  public void test_no_node_with_analysisWarnings() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable = TestLinterCommandProvider.resourceScript("/executables/invalidNodeVersion.js");
+  public void test_invalid_node_command_with_analysisWarnings() {
+    InvalidCommandProvider commandProvider = new InvalidCommandProvider();
     CssRuleSensor sensor = createCssRuleSensor(commandProvider, analysisWarnings);
     sensor.execute(context);
 
     assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Failed to get Node.js version. No CSS files will be analyzed.");
-    verify(analysisWarnings).addUnique(matches("CSS files were not analyzed. Node.js version could not be detected using command: .* -v"));
-  }
-
-  @Test
-  public void test_old_node_with_analysisWarnings() {
-    TestLinterCommandProvider commandProvider = getCommandProvider();
-    commandProvider.nodeExecutable += " " + TestLinterCommandProvider.resourceScript("/executables/oldNodeVersion.js");
-    CssRuleSensor sensor = createCssRuleSensor(commandProvider, analysisWarnings);
-    sensor.execute(context);
-
-    assertThat(context.allIssues()).hasSize(0);
-    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Only Node.js v6 or later is supported, got 3.2.1. No CSS files will be analyzed.");
-    verify(analysisWarnings).addUnique(eq("CSS files were not analyzed. Only Node.js v6 or later is supported, got 3.2.1."));
+    assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Some problem happened. No CSS files will be analyzed.");
+    verify(analysisWarnings).addUnique(eq("CSS files were not analyzed. Some problem happened."));
   }
 
   @Test
@@ -267,11 +218,11 @@ public class CssRuleSensorTest {
     return inputFile;
   }
 
-  private CssRuleSensor createCssRuleSensor(TestLinterCommandProvider commandProvider) {
+  private CssRuleSensor createCssRuleSensor(LinterCommandProvider commandProvider) {
     return new CssRuleSensor(new TestBundleHandler(), checkFactory, commandProvider);
   }
 
-  private CssRuleSensor createCssRuleSensor(TestLinterCommandProvider commandProvider, @Nullable AnalysisWarningsWrapper analysisWarnings) {
+  private CssRuleSensor createCssRuleSensor(LinterCommandProvider commandProvider, @Nullable AnalysisWarningsWrapper analysisWarnings) {
     return new CssRuleSensor(new TestBundleHandler(), checkFactory, commandProvider, analysisWarnings);
   }
 
@@ -281,20 +232,8 @@ public class CssRuleSensorTest {
 
   private static class TestLinterCommandProvider implements LinterCommandProvider {
 
-    String nodeExecutable = findNodeExecutable();
-
     private String[] elements;
 
-    private static String findNodeExecutable() {
-      try {
-        String nodeFromMavenPlugin = "target/node/node";
-        Runtime.getRuntime().exec(nodeFromMavenPlugin);
-        return nodeFromMavenPlugin;
-      } catch (IOException e) {
-        return "node";
-      }
-    }
-
     private static String resourceScript(String script) {
       try {
         return new File(TestLinterCommandProvider.class.getResource(script).toURI()).getAbsolutePath();
@@ -304,31 +243,45 @@ public class CssRuleSensorTest {
     }
 
     TestLinterCommandProvider nodeScript(String script, String args) {
-      this.elements = new String[]{ nodeExecutable, resourceScript(script), args};
+      this.elements = new String[]{ resourceScript(script), args};
       return this;
     }
 
     @Override
-    public String[] commandParts(File deployDestination, SensorContext context) {
-      return elements;
+    public NodeCommand nodeCommand(File deployDestination, SensorContext context, Consumer<String> output, Consumer<String> error) {
+      return NodeCommand.builder()
+        .outputConsumer(output)
+        .errorConsumer(error)
+        .minNodeVersion(6)
+        .configuration(context.config())
+        .nodeJsArgs(elements)
+        .build();
     }
 
     @Override
     public String configPath(File deployDestination) {
       return new File(deployDestination, "testconfig.json").getAbsolutePath();
     }
+  }
+
+  private static class InvalidCommandProvider implements LinterCommandProvider {
+
+    @Override
+    public NodeCommand nodeCommand(File deployDestination, SensorContext context, Consumer<String> output, Consumer<String> error) {
+      throw new NodeCommandException("Some problem happened.");
+    }
 
     @Override
-    public String nodeExecutable(Configuration configuration) {
-      return nodeExecutable;
+    public String configPath(File deployDestination) {
+      return new File(deployDestination, "testconfig.json").getAbsolutePath();
     }
   }
 
   private static class TestBundleHandler implements BundleHandler {
-
     @Override
     public void deployBundle(File deployDestination) {
       // do nothing
     }
   }
+
 }
diff --git a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/StylelintCommandProviderTest.java b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/StylelintCommandProviderTest.java
index 1ae9a1d..9251fe0 100644
--- a/sonar-css-plugin/src/test/java/org/sonar/css/plugin/StylelintCommandProviderTest.java
+++ b/sonar-css-plugin/src/test/java/org/sonar/css/plugin/StylelintCommandProviderTest.java
@@ -20,13 +20,13 @@
 package org.sonar.css.plugin;
 
 import java.io.File;
+import java.util.function.Consumer;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.api.batch.sensor.internal.SensorContextTester;
-import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.utils.log.LogTester;
-import org.sonar.api.utils.log.LoggerLevel;
+import org.sonarsource.nodejs.NodeCommand;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -39,48 +39,22 @@ public class StylelintCommandProviderTest {
   public final LogTester logTester = new LogTester();
 
   @Test
-  public void test() throws Exception {
+  public void test() {
     StylelintCommandProvider stylelintCommandProvider = new StylelintCommandProvider();
     File deployDestination = new File("deploy_destination");
     File baseDir = new File("src/test/resources").getAbsoluteFile();
     SensorContextTester context = SensorContextTester.create(baseDir);
     context.settings().setProperty(CssPlugin.FILE_SUFFIXES_KEY, ".foo,.bar");
-    assertThat(stylelintCommandProvider.commandParts(deployDestination, context)).containsExactly(
-      "node",
+    Consumer<String> noop = a -> {};
+    NodeCommand nodeCommand = stylelintCommandProvider.nodeCommand(deployDestination, context, noop, noop);
+    assertThat(nodeCommand.toString()).endsWith(
+      String.join(" ",
       new File(deployDestination, "css-bundle/node_modules/stylelint/bin/stylelint").getAbsolutePath(),
       baseDir.getAbsolutePath() + File.separator + "**" + File.separator + "*{.foo,.bar}",
       "--config",
       new File(deployDestination, "css-bundle/stylelintconfig.json").getAbsolutePath(),
       "-f",
-      "json"
+      "json")
     );
   }
-
-  @Test
-  public void test_node_executable_wo_settings() throws Exception {
-    StylelintCommandProvider stylelintCommandProvider = new StylelintCommandProvider();
-    MapSettings settings = new MapSettings();
-    assertThat(stylelintCommandProvider.nodeExecutable(settings.asConfig())).isEqualTo("node");
-    assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
-  }
-
-  @Test
-  public void test_node_executable_custom() throws Exception {
-    StylelintCommandProvider stylelintCommandProvider = new StylelintCommandProvider();
-    MapSettings settings = new MapSettings();
-    File customNode = temporaryFolder.newFile("custom-node.exe");
-    settings.setProperty(CssPlugin.NODE_EXECUTABLE, customNode.getAbsolutePath());
-    assertThat(stylelintCommandProvider.nodeExecutable(settings.asConfig())).isEqualTo(customNode.getAbsolutePath());
-    assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
-  }
-
-  @Test
-  public void test_node_executable_custom_invalid() throws Exception {
-    StylelintCommandProvider stylelintCommandProvider = new StylelintCommandProvider();
-
-    MapSettings settings = new MapSettings();
-    settings.setProperty(CssPlugin.NODE_EXECUTABLE, "mynode");
-    assertThat(stylelintCommandProvider.nodeExecutable(settings.asConfig())).isEqualTo("node");
-    assertThat(logTester.logs(LoggerLevel.WARN)).contains("Provided node executable file does not exist: mynode. Fallback to using 'node' from path.");
-  }
 }
diff --git a/sonar-css-plugin/src/test/resources/executables/invalidNodeVersion.js b/sonar-css-plugin/src/test/resources/executables/invalidNodeVersion.js
deleted file mode 100644
index 772b9f3..0000000
--- a/sonar-css-plugin/src/test/resources/executables/invalidNodeVersion.js
+++ /dev/null
@@ -1 +0,0 @@
-console.log("Invalid version");