Skip to content

Commit

Permalink
Improve coverage & fix code smells (#4188)
Browse files Browse the repository at this point in the history
  • Loading branch information
@saberduck
saberduck authored Sep 21, 2023
1 parent 814732f commit ab29cc8
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 74 deletions.
6 changes: 3 additions & 3 deletions ...nar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ public class EmbeddedNode {
public static final String VERSION_FILENAME = "version.txt";
private static final long EXTRACTION_LOCK_WAIT_TIME_MILLIS = 10000;
private static final Logger LOG = Loggers.get(EmbeddedNode.class);
private Path deployLocation;
private final Path deployLocation;
private final Platform platform;
private boolean isAvailable;
private Environment env;
private final Environment env;

enum Platform {
WIN_X64,
Expand Down Expand Up @@ -151,7 +151,7 @@ public boolean isAvailable() {
*/
public void deploy() throws IOException {
LOG.debug("Detected os: {} arch: {} platform: {}", env.getOsName(), env.getOsArch(), platform);
if (platform == UNSUPPORTED || isAvailable) {
if (platform == UNSUPPORTED) {
return;
}
var is = getClass().getResourceAsStream(platform.archivePathInJar());
Expand Down
8 changes: 0 additions & 8 deletions ...onar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,4 @@ public String toString() {
public Version getActualNodeVersion() {
return actualNodeVersion;
}

public static NodeCommandBuilder builder() {
return builder(new ProcessWrapperImpl());
}

static NodeCommandBuilder builder(ProcessWrapper processWrapper) {
return new NodeCommandBuilderImpl(processWrapper);
}
}
11 changes: 4 additions & 7 deletions ...ript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import java.util.function.Consumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import org.sonar.api.config.Configuration;
import org.sonar.api.utils.Version;
import org.sonar.api.utils.log.Logger;
Expand All @@ -53,7 +52,7 @@ public class NodeCommandBuilderImpl implements NodeCommandBuilder {
"package/node_modules/run-node/run-node";

private static final String NODE_EXECUTABLE_PROPERTY = "sonar.nodejs.executable";
final String NODE_FORCE_HOST_PROPERTY = "sonar.nodejs.forceHost";
private static final String NODE_FORCE_HOST_PROPERTY = "sonar.nodejs.forceHost";

private static final Pattern NODEJS_VERSION_PATTERN = Pattern.compile(
"v?(\\d+)\\.(\\d+)\\.(\\d+)"
Expand Down Expand Up @@ -254,9 +253,9 @@ private String getVersion(String nodeExecutable) throws NodeCommandException {
* @throws NodeCommandException
* @throws IOException
*/
private String retrieveNodeExecutable(@Nullable Configuration configuration)
private String retrieveNodeExecutable(Configuration configuration)
throws NodeCommandException, IOException {
if (configuration != null && configuration.hasKey(NODE_EXECUTABLE_PROPERTY)) {
if (configuration.hasKey(NODE_EXECUTABLE_PROPERTY)) {
String nodeExecutable = configuration.get(NODE_EXECUTABLE_PROPERTY).get();
File file = new File(nodeExecutable);
if (file.exists()) {
Expand Down Expand Up @@ -295,9 +294,7 @@ private String locateNode(boolean isForceHost) throws IOException {
}

private boolean isForceHost(Configuration configuration) {
return (
configuration != null && configuration.getBoolean(NODE_FORCE_HOST_PROPERTY).orElse(false)
);
return configuration.getBoolean(NODE_FORCE_HOST_PROPERTY).orElse(false);
}

private String locateNodeOnMac() throws IOException {
Expand Down
15 changes: 11 additions & 4 deletions ...script-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgramRequest;
import org.sonar.plugins.javascript.nodejs.NodeCommand;
import org.sonar.plugins.javascript.nodejs.NodeCommandBuilder;
import org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl;
import org.sonar.plugins.javascript.nodejs.NodeCommandException;
import org.sonar.plugins.javascript.nodejs.ProcessWrapperImpl;

class BridgeServerImplTest {

Expand Down Expand Up @@ -654,7 +656,7 @@ void should_skip_metrics_on_sonarlint() throws Exception {
void should_use_default_timeout() {
bridgeServer =
new BridgeServerImpl(
NodeCommand.builder(),
builder(),
mock(Bundle.class),
mock(RulesBundles.class),
deprecationWarning,
Expand Down Expand Up @@ -716,7 +718,7 @@ public void execute(SensorContext context) {}
assertThat(monitoring.isMonitoringEnabled()).isTrue();
bridgeServer =
new BridgeServerImpl(
NodeCommand.builder(),
builder(),
TEST_TIMEOUT_SECONDS,
new TestBundle(START_SERVER_SCRIPT),
emptyRulesBundles,
Expand Down Expand Up @@ -748,7 +750,7 @@ void test_ucfg_bundle_version() throws Exception {

bridgeServer =
new BridgeServerImpl(
NodeCommand.builder(),
builder(),
TEST_TIMEOUT_SECONDS,
new TestBundle(START_SERVER_SCRIPT),
rulesBundles,
Expand All @@ -765,7 +767,7 @@ void test_ucfg_bundle_version() throws Exception {

private BridgeServerImpl createBridgeServer(String startServerScript) {
return new BridgeServerImpl(
NodeCommand.builder(),
builder(),
TEST_TIMEOUT_SECONDS,
new TestBundle(startServerScript),
emptyRulesBundles,
Expand Down Expand Up @@ -808,4 +810,9 @@ public String resolve(String relativePath) {
return new File(file.getAbsoluteFile(), relativePath).getAbsolutePath();
}
}

private static NodeCommandBuilder builder() {
return new NodeCommandBuilderImpl(new ProcessWrapperImpl())
.configuration(new MapSettings().asConfig());
}
}
59 changes: 50 additions & 9 deletions ...javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.DARWIN_ARM64;
import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.LINUX_X64;
import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.UNSUPPORTED;
import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.WIN_X64;

import java.io.IOException;
import java.nio.file.Files;
Expand All @@ -12,6 +16,7 @@
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.event.Level;
import org.sonar.api.testfixtures.log.LogTesterJUnit5;
import org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform;

class EmbeddedNodeTest {

Expand All @@ -31,6 +36,7 @@ void should_extract_if_deployLocation_contains_a_different_version() throws Exce
Files.write(runtimeFolder.resolve("version.txt"), "a-different-version".getBytes());
en.deploy();
assertThat(en.binary()).exists();
assertThat(en.isAvailable()).isTrue();
}

@Test
Expand All @@ -44,6 +50,14 @@ void should_not_extract_if_deployLocation_contains_the_same_version() throws Exc
);
en.deploy();
assertThat(en.binary()).doesNotExist();
assertThat(en.isAvailable()).isTrue();
}

@Test
void should_not_be_available() throws Exception {
var en = new EmbeddedNode(createUnsupportedEnvironment());
en.deploy();
assertThat(en.isAvailable()).isFalse();
}

@Test
Expand All @@ -55,29 +69,55 @@ void should_extract_if_deployLocation_has_no_version() throws Exception {

@Test
void should_detect_platform_for_windows_environment() {
var platform = EmbeddedNode.Platform.detect(createWindowsEnvironment());
assertThat(platform).isEqualTo(EmbeddedNode.Platform.WIN_X64);
var platform = Platform.detect(createWindowsEnvironment());
assertThat(platform).isEqualTo(WIN_X64);
assertThat(platform.archivePathInJar()).isEqualTo("/win-x64/node.exe.xz");
}

@Test
void should_detect_platform_for_mac_os_environment() {
var platform = EmbeddedNode.Platform.detect(createMacOSEnvironment());
assertThat(platform).isEqualTo(EmbeddedNode.Platform.DARWIN_ARM64);
var platform = Platform.detect(createMacOSEnvironment());
assertThat(platform).isEqualTo(DARWIN_ARM64);
assertThat(platform.archivePathInJar()).isEqualTo("/darwin-arm64/node.xz");
}

@Test
void should_detect_platform_for_linux_environment() {
var linux = mock(Environment.class);
when(linux.getOsName()).thenReturn("linux");
when(linux.getOsArch()).thenReturn("amd64");
var platform = Platform.detect(linux);
assertThat(platform).isEqualTo(LINUX_X64);
assertThat(platform.archivePathInJar()).isEqualTo("/linux-x64/node.xz");
}

@Test
void should_return_unsupported_for_unknown_environment() {
var platform = EmbeddedNode.Platform.detect(createUnsupportedEnvironment());
assertThat(platform).isEqualTo(EmbeddedNode.Platform.UNSUPPORTED);
var platform = Platform.detect(createUnsupportedEnvironment());
assertThat(platform).isEqualTo(UNSUPPORTED);
assertThat(platform.archivePathInJar()).isEqualTo("node.xz");
}

@Test
void test_unsupported_archs() {
var win = mock(Environment.class);
when(win.getOsName()).thenReturn("Windows");
when(win.getOsArch()).thenReturn("unknown");
assertThat(Platform.detect(win)).isEqualTo(UNSUPPORTED);

var linux = mock(Environment.class);
when(linux.getOsName()).thenReturn("linux");
when(linux.getOsArch()).thenReturn("unknown");
assertThat(Platform.detect(linux)).isEqualTo(UNSUPPORTED);

var macos = mock(Environment.class);
when(macos.getOsName()).thenReturn("mac os");
when(macos.getOsArch()).thenReturn("unknown");
assertThat(Platform.detect(macos)).isEqualTo(UNSUPPORTED);
}

private byte[] extractCurrentVersion(Environment env) throws IOException {
return getClass()
.getResourceAsStream(EmbeddedNode.Platform.detect(env).versionPathInJar())
.readAllBytes();
return getClass().getResourceAsStream(Platform.detect(env).versionPathInJar()).readAllBytes();
}

private Environment createTestEnvironment() {
Expand Down Expand Up @@ -106,6 +146,7 @@ private Environment createUnsupportedEnvironment() {
Environment mockEnvironment = mock(Environment.class);
when(mockEnvironment.getOsName()).thenReturn("");
when(mockEnvironment.getOsArch()).thenReturn("");
when(mockEnvironment.getUserHome()).thenReturn(tempDir.toString());
return mockEnvironment;
}
}
Loading

0 comments on commit ab29cc8

Please sign in to comment.