Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve coverage & fix code smells #4188

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Improve coverage & fix code smells #4188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...nar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java
View file
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove isAvailable altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isAvailable is needed to know that deployment was successful and embeddednode can be used. We rely on it in NodeCommandBuilder

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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