From 45c1d35f0a132b257c8aa419eb981d0b9198bd2f Mon Sep 17 00:00:00 2001 From: Elena Vilchik Date: Fri, 7 Jun 2019 17:35:05 +0200 Subject: 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 --- sonar-css-plugin/pom.xml | 18 +++- .../main/java/org/sonar/css/plugin/CssPlugin.java | 10 +- .../java/org/sonar/css/plugin/CssRuleSensor.java | 99 ++++++------------- .../css/plugin/ExternalProcessStreamConsumer.java | 68 ------------- .../sonar/css/plugin/LinterCommandProvider.java | 6 +- .../sonar/css/plugin/StylelintCommandProvider.java | 52 ++++------ .../java/org/sonar/css/plugin/CssPluginTest.java | 8 +- .../org/sonar/css/plugin/CssRuleSensorTest.java | 109 ++++++--------------- .../css/plugin/StylelintCommandProviderTest.java | 42 ++------ .../resources/executables/invalidNodeVersion.js | 1 - 10 files changed, 109 insertions(+), 304 deletions(-) delete mode 100644 sonar-css-plugin/src/main/java/org/sonar/css/plugin/ExternalProcessStreamConsumer.java delete mode 100644 sonar-css-plugin/src/test/resources/executables/invalidNodeVersion.js (limited to 'sonar-css-plugin') diff --git a/sonar-css-plugin/pom.xml b/sonar-css-plugin/pom.xml index 4a6a213..a1645e5 100644 --- a/sonar-css-plugin/pom.xml +++ b/sonar-css-plugin/pom.xml @@ -36,6 +36,10 @@ org.sonarsource.analyzer-commons sonar-analyzer-commons + + org.sonarsource.javascript + nodejs-utils + junit junit @@ -127,8 +131,8 @@ - 6500000 - 7000000 + 6800000 + 8000000 ${project.build.directory}/${project.build.finalName}.jar @@ -166,6 +170,16 @@ + + maven-surefire-plugin + + + + target${file.separator}node${path.separator}${env.PATH} + + + + maven-assembly-plugin 3.0.0 diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssPlugin.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssPlugin.java index 6f78be1..9e78205 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssPlugin.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssPlugin.java @@ -37,7 +37,7 @@ public class CssPlugin implements Plugin { public static final String STYLELINT_REPORT_PATHS = "sonar.css.stylelint.reportPaths"; public static final String STYLELINT_REPORT_PATHS_DEFAULT_VALUE = ""; - public static final String NODE_EXECUTABLE = "sonar.css.node"; + public static final String FORMER_NODE_EXECUTABLE = "sonar.css.node"; private static final String CSS_CATEGORY = "CSS"; private static final String LINTER_SUBCATEGORY = "Popular Rule Engines"; @@ -66,14 +66,6 @@ public class CssPlugin implements Plugin { .category(CSS_CATEGORY) .onQualifiers(Qualifiers.PROJECT) .multiValues(true) - .build(), - - PropertyDefinition.builder(NODE_EXECUTABLE) - .name("Node.js executable") - .description("Path to the Node.js executable that will be used to run the analysis of CSS files. When not set, expects 'node' to be in the path.") - .subCategory(GENERAL_SUBCATEGORY) - .category(CSS_CATEGORY) - .hidden() .build() ); diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java index 2b268cd..82e6306 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/CssRuleSensor.java @@ -31,7 +31,6 @@ import java.util.Collections; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; -import org.apache.commons.io.IOUtils; import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.rule.CheckFactory; @@ -47,33 +46,36 @@ import org.sonar.css.plugin.CssRules.StylelintConfig; import org.sonar.css.plugin.StylelintReport.Issue; import org.sonar.css.plugin.StylelintReport.IssuesPerFile; import org.sonar.css.plugin.bundle.BundleHandler; +import org.sonarsource.nodejs.NodeCommand; +import org.sonarsource.nodejs.NodeCommandException; public class CssRuleSensor implements Sensor { private static final Logger LOG = Loggers.get(CssRuleSensor.class); - private static final int MIN_NODE_VERSION = 6; - private static final String WARNING_PREFIX = "CSS files were not analyzed. "; private final BundleHandler bundleHandler; private final CssRules cssRules; private final LinterCommandProvider linterCommandProvider; @Nullable private final AnalysisWarningsWrapper analysisWarnings; - private final ExternalProcessStreamConsumer externalProcessStreamConsumer = new ExternalProcessStreamConsumer(); - public CssRuleSensor(BundleHandler bundleHandler, - CheckFactory checkFactory, - LinterCommandProvider linterCommandProvider, - @Nullable AnalysisWarningsWrapper analysisWarnings) { + public CssRuleSensor( + BundleHandler bundleHandler, + CheckFactory checkFactory, + LinterCommandProvider linterCommandProvider, + @Nullable AnalysisWarningsWrapper analysisWarnings + ) { this.bundleHandler = bundleHandler; this.linterCommandProvider = linterCommandProvider; this.cssRules = new CssRules(checkFactory); this.analysisWarnings = analysisWarnings; } - public CssRuleSensor(BundleHandler bundleHandler, - CheckFactory checkFactory, - LinterCommandProvider linterCommandProvider) { + public CssRuleSensor( + BundleHandler bundleHandler, + CheckFactory checkFactory, + LinterCommandProvider linterCommandProvider + ) { this(bundleHandler, checkFactory, linterCommandProvider, null); } @@ -86,39 +88,38 @@ public class CssRuleSensor implements Sensor { @Override public void execute(SensorContext context) { + // fixme add log and UI warn when old property is provided + if (cssRules.isEmpty()) { LOG.warn("No rules are activated in CSS Quality Profile"); return; } - if (!checkCompatibleNodeVersion(context)) { - return; - } - File deployDestination = context.fileSystem().workDir(); - bundleHandler.deployBundle(deployDestination); - String[] commandParts = linterCommandProvider.commandParts(deployDestination, context); try { - ProcessBuilder processBuilder = new ProcessBuilder(commandParts); + bundleHandler.deployBundle(deployDestination); createLinterConfig(deployDestination); - Process process = processBuilder.start(); StringBuilder output = new StringBuilder(); - externalProcessStreamConsumer.consumeStream(process.getInputStream(), output::append); - externalProcessStreamConsumer.consumeStream(process.getErrorStream(), LOG::error); - if (isSuccessful(process)) { + + NodeCommand nodeCommand = linterCommandProvider.nodeCommand(deployDestination, context, output::append, LOG::error); + LOG.debug("Starting process: " + nodeCommand.toString()); + nodeCommand.start(); + + if (isSuccessful(nodeCommand.waitFor())) { saveIssues(context, output.toString()); } + } catch (NodeCommandException e) { + LOG.error(e.getMessage() + " No CSS files will be analyzed.", e); + if (analysisWarnings != null) { + analysisWarnings.addUnique("CSS files were not analyzed. " + e.getMessage()); + } } catch (Exception e) { - LOG.error("Failed to run external linting process " + String.join(" ", commandParts), e); - } finally { - externalProcessStreamConsumer.shutdownNow(); + LOG.error("Failed to run external linting process", e); } } - private boolean isSuccessful(Process process) throws InterruptedException { - int exitValue = process.waitFor(); - externalProcessStreamConsumer.await(); + private boolean isSuccessful(int exitValue) { // exit codes 0 and 2 are expected. 0 - means no issues were found, 2 - means that at least one "error-level" rule found issue // see https://github.com/stylelint/stylelint/blob/master/docs/user-guide/cli.md#exit-codes boolean isSuccessful = exitValue == 0 || exitValue == 2; @@ -128,48 +129,6 @@ public class CssRuleSensor implements Sensor { return isSuccessful; } - private boolean checkCompatibleNodeVersion(SensorContext context) { - String nodeExecutable = linterCommandProvider.nodeExecutable(context.config()); - LOG.debug("Checking node version"); - String messageSuffix = "No CSS files will be analyzed."; - - String version; - try { - Process process = Runtime.getRuntime().exec(nodeExecutable + " -v"); - version = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8).trim(); - } catch (Exception e) { - LOG.error("Failed to get Node.js version. " + messageSuffix, e); - if (analysisWarnings != null) { - analysisWarnings.addUnique(WARNING_PREFIX + "Node.js version could not be detected using command: " + nodeExecutable + " -v"); - } - return false; - } - - Pattern versionPattern = Pattern.compile("v?(\\d+)\\.\\d+\\.\\d+"); - Matcher versionMatcher = versionPattern.matcher(version); - if (versionMatcher.matches()) { - int major = Integer.parseInt(versionMatcher.group(1)); - if (major < MIN_NODE_VERSION) { - String message = String.format("Only Node.js v%s or later is supported, got %s.", MIN_NODE_VERSION, version); - LOG.error(message + ' ' + messageSuffix); - if (analysisWarnings != null) { - analysisWarnings.addUnique(WARNING_PREFIX + message); - } - return false; - } - } else { - String message = String.format("Failed to parse Node.js version, got '%s'.", version); - LOG.error(message + ' ' + messageSuffix); - if (analysisWarnings != null) { - analysisWarnings.addUnique(WARNING_PREFIX + message); - } - return false; - } - - LOG.debug(String.format("Using Node.js %s", version)); - return true; - } - private void createLinterConfig(File deployDestination) throws IOException { String configPath = linterCommandProvider.configPath(deployDestination); StylelintConfig config = cssRules.getConfig(); diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/ExternalProcessStreamConsumer.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/ExternalProcessStreamConsumer.java deleted file mode 100644 index ef843c4..0000000 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/ExternalProcessStreamConsumer.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * SonarCSS - * Copyright (C) 2018-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.css.plugin; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; - -class ExternalProcessStreamConsumer { - - private static final Logger LOG = Loggers.get(ExternalProcessStreamConsumer.class); - private ExecutorService executorService; - - ExternalProcessStreamConsumer() { - executorService = Executors.newCachedThreadPool(r -> { - Thread thread = new Thread(r); - thread.setName("nodejs-stream-consumer"); - thread.setDaemon(true); - return thread; - }); - } - - void consumeStream(InputStream inputStream, Consumer consumer) { - executorService.submit(() -> { - try (BufferedReader errorReader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) { - errorReader.lines().forEach(consumer); - } catch (IOException e) { - LOG.error("Error while reading stream", e); - } - }); - } - - void await() throws InterruptedException { - executorService.shutdown(); - if (!executorService.awaitTermination(5, TimeUnit.MINUTES)) { - LOG.error("External process stream consumer timed out"); - } - } - - void shutdownNow() { - executorService.shutdownNow(); - } -} diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/LinterCommandProvider.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/LinterCommandProvider.java index 133e9a4..74a343c 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/LinterCommandProvider.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/LinterCommandProvider.java @@ -20,14 +20,14 @@ package org.sonar.css.plugin; import java.io.File; +import java.util.function.Consumer; import org.sonar.api.batch.sensor.SensorContext; -import org.sonar.api.config.Configuration; +import org.sonarsource.nodejs.NodeCommand; public interface LinterCommandProvider { - String[] commandParts(File deployDestination, SensorContext context); + NodeCommand nodeCommand(File deployDestination, SensorContext context, Consumer output, Consumer error); String configPath(File deployDestination); - String nodeExecutable(Configuration configuration); } diff --git a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/StylelintCommandProvider.java b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/StylelintCommandProvider.java index 0265c20..90d3ae8 100644 --- a/sonar-css-plugin/src/main/java/org/sonar/css/plugin/StylelintCommandProvider.java +++ b/sonar-css-plugin/src/main/java/org/sonar/css/plugin/StylelintCommandProvider.java @@ -21,38 +21,43 @@ package org.sonar.css.plugin; import java.io.File; import java.nio.file.Paths; -import java.util.Optional; +import java.util.function.Consumer; import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.sensor.SensorContext; -import org.sonar.api.config.Configuration; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; +import org.sonarsource.nodejs.NodeCommand; +import org.sonarsource.nodejs.NodeCommandException; @ScannerSide public class StylelintCommandProvider implements LinterCommandProvider { - private static final Logger LOG = Loggers.get(StylelintCommandProvider.class); - private static final String CONFIG_PATH = "css-bundle/stylelintconfig.json"; - private static final String NODE_EXECUTABLE_DEFAULT = "node"; - - private String nodeExecutable = null; @Override - public String[] commandParts(File deployDestination, SensorContext context) { + public NodeCommand nodeCommand(File deployDestination, SensorContext context, Consumer output, Consumer error) { String projectBaseDir = context.fileSystem().baseDir().getAbsolutePath(); String[] suffixes = context.config().getStringArray(CssPlugin.FILE_SUFFIXES_KEY); String filesGlob = "**" + File.separator + "*{" + String.join(",", suffixes) + "}"; String filesToAnalyze = Paths.get(projectBaseDir, "TOREPLACE").toString(); filesToAnalyze = filesToAnalyze.replace("TOREPLACE", filesGlob); - return new String[]{ - nodeExecutable(context.config()), + String[] args = { new File(deployDestination, "css-bundle/node_modules/stylelint/bin/stylelint").getAbsolutePath(), filesToAnalyze, "--config", new File(deployDestination, CONFIG_PATH).getAbsolutePath(), "-f", "json" }; + + try { + return NodeCommand.builder() + .outputConsumer(output) + .errorConsumer(error) + .minNodeVersion(6) + .configuration(context.config()) + .nodeJsArgs(args) + .build(); + } catch (IllegalArgumentException e) { + throw new NodeCommandException(e.getMessage(), e); + } } @Override @@ -60,27 +65,4 @@ public class StylelintCommandProvider implements LinterCommandProvider { return new File(deployDestination, CONFIG_PATH).getAbsolutePath(); } - @Override - public String nodeExecutable(Configuration configuration) { - if (nodeExecutable == null) { - nodeExecutable = retrieveNodeExecutableFromConfig(configuration); - } - - return nodeExecutable; - } - - private static String retrieveNodeExecutableFromConfig(Configuration configuration) { - Optional nodeExecutableOptional = configuration.get(CssPlugin.NODE_EXECUTABLE); - if (nodeExecutableOptional.isPresent()) { - String nodeExecutable = nodeExecutableOptional.get(); - File file = new File(nodeExecutable); - if (file.exists()) { - return nodeExecutable; - } - - LOG.warn("Provided node executable file does not exist: " + file + ". Fallback to using 'node' from path."); - } - - return NODE_EXECUTABLE_DEFAULT; - } } 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 output, Consumer 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 output, Consumer 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 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"); -- cgit v1.2.3