Skip to content

Commit e9dd6a8

Browse files
henryjuhenryju
committed
Log java --version sanity check in debug only
1 parent 6e43965 commit e9dd6a8

5 files changed

Lines changed: 114 additions & 90 deletions

File tree

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/JavaRunner.java‎

Lines changed: 2 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
*/
2020
package org.sonarsource.scanner.lib.internal;
2121

22-
import com.google.gson.Gson;
23-
import com.google.gson.annotations.SerializedName;
2422
import java.io.BufferedReader;
2523
import java.io.IOException;
2624
import java.io.InputStream;
@@ -50,7 +48,7 @@ public JreCacheHit getJreCacheHit() {
5048
return jreCacheHit;
5149
}
5250

53-
public boolean execute(List<String> args, @Nullable String input) {
51+
public boolean execute(List<String> args, @Nullable String input, Consumer<String> stdOutConsummer) {
5452
try {
5553
List<String> command = new ArrayList<>(args);
5654
command.add(0, javaExecutable.toString());
@@ -61,7 +59,7 @@ public boolean execute(List<String> args, @Nullable String input) {
6159
osw.write(input);
6260
}
6361
}
64-
var stdoutConsummer = new StreamGobbler(process.getInputStream(), this::tryParse);
62+
var stdoutConsummer = new StreamGobbler(process.getInputStream(), stdOutConsummer);
6563
var stdErrConsummer = new StreamGobbler(process.getErrorStream(), stderr -> LOG.error("[stderr] {}", stderr));
6664
stdErrConsummer.start();
6765
stdoutConsummer.start();
@@ -83,54 +81,6 @@ public Path getJavaExecutable() {
8381
return javaExecutable;
8482
}
8583

86-
void tryParse(String stdout) {
87-
try {
88-
var log = new Gson().fromJson(stdout, Log.class);
89-
StringBuilder sb = new StringBuilder();
90-
if (log.message != null) {
91-
sb.append(log.message);
92-
}
93-
if (log.message != null && log.stacktrace != null) {
94-
sb.append("\n");
95-
}
96-
if (log.stacktrace != null) {
97-
sb.append(log.stacktrace);
98-
}
99-
log(log.level, sb.toString());
100-
} catch (Exception e) {
101-
LOG.info("[stdout] {}", stdout);
102-
}
103-
}
104-
105-
private static void log(String level, String msg) {
106-
switch (level) {
107-
case "ERROR":
108-
LOG.error(msg);
109-
break;
110-
case "WARN":
111-
LOG.warn(msg);
112-
break;
113-
case "DEBUG":
114-
LOG.debug(msg);
115-
break;
116-
case "TRACE":
117-
LOG.trace(msg);
118-
break;
119-
case "INFO":
120-
default:
121-
LOG.info(msg);
122-
}
123-
}
124-
125-
private static class Log {
126-
@SerializedName("level")
127-
private String level;
128-
@SerializedName("message")
129-
private String message;
130-
@SerializedName("stacktrace")
131-
private String stacktrace;
132-
}
133-
13484
private static class StreamGobbler extends Thread {
13585
private final InputStream inputStream;
13686
private final Consumer<String> consumer;

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/ScannerEngineLauncher.java‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,21 @@
2222
import com.google.gson.Gson;
2323
import com.google.gson.JsonArray;
2424
import com.google.gson.JsonObject;
25+
import com.google.gson.annotations.SerializedName;
2526
import java.util.ArrayList;
2627
import java.util.Arrays;
2728
import java.util.List;
2829
import java.util.Map;
2930
import java.util.stream.Collectors;
31+
import org.slf4j.Logger;
32+
import org.slf4j.LoggerFactory;
3033
import org.sonarsource.scanner.lib.ScannerProperties;
3134
import org.sonarsource.scanner.lib.internal.cache.CachedFile;
3235

3336
public class ScannerEngineLauncher {
3437

38+
private static final Logger LOG = LoggerFactory.getLogger(ScannerEngineLauncher.class);
39+
3540
private static final String JSON_FIELD_SCANNER_PROPERTIES = "scannerProperties";
3641
private final JavaRunner javaRunner;
3742
private final CachedFile scannerEngineJar;
@@ -42,7 +47,55 @@ public ScannerEngineLauncher(JavaRunner javaRunner, CachedFile scannerEngineJar)
4247
}
4348

4449
public boolean execute(Map<String, String> properties) {
45-
return javaRunner.execute(buildArgs(properties), buildJsonProperties(properties));
50+
return javaRunner.execute(buildArgs(properties), buildJsonProperties(properties), ScannerEngineLauncher::tryParse);
51+
}
52+
53+
static void tryParse(String stdout) {
54+
try {
55+
var log = new Gson().fromJson(stdout, Log.class);
56+
StringBuilder sb = new StringBuilder();
57+
if (log.message != null) {
58+
sb.append(log.message);
59+
}
60+
if (log.message != null && log.stacktrace != null) {
61+
sb.append("\n");
62+
}
63+
if (log.stacktrace != null) {
64+
sb.append(log.stacktrace);
65+
}
66+
log(log.level, sb.toString());
67+
} catch (Exception e) {
68+
LOG.info("[stdout] {}", stdout);
69+
}
70+
}
71+
72+
private static void log(String level, String msg) {
73+
switch (level) {
74+
case "ERROR":
75+
LOG.error(msg);
76+
break;
77+
case "WARN":
78+
LOG.warn(msg);
79+
break;
80+
case "DEBUG":
81+
LOG.debug(msg);
82+
break;
83+
case "TRACE":
84+
LOG.trace(msg);
85+
break;
86+
case "INFO":
87+
default:
88+
LOG.info(msg);
89+
}
90+
}
91+
92+
private static class Log {
93+
@SerializedName("level")
94+
private String level;
95+
@SerializedName("message")
96+
private String message;
97+
@SerializedName("stacktrace")
98+
private String stacktrace;
4699
}
47100

48101
private List<String> buildArgs(Map<String, String> properties) {

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/ScannerEngineLauncherFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public ScannerEngineLauncher createLauncher(ServerConnection serverConnection, F
5757
}
5858

5959
private static void jreSanityCheck(JavaRunner javaRunner) {
60-
javaRunner.execute(Collections.singletonList("--version"), null);
60+
javaRunner.execute(Collections.singletonList("--version"), null, LOG::debug);
6161
}
6262

6363
private static CachedFile getScannerEngine(ServerConnection serverConnection, FileCache fileCache, boolean retry) {

‎lib/src/test/java/org/sonarsource/scanner/lib/internal/JavaRunnerTest.java‎

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.nio.file.Paths;
2323
import java.util.List;
24+
import java.util.concurrent.ConcurrentLinkedDeque;
2425
import org.junit.jupiter.api.Test;
2526
import org.junit.jupiter.api.extension.RegisterExtension;
2627
import org.slf4j.event.Level;
@@ -34,23 +35,34 @@ class JavaRunnerTest {
3435
@RegisterExtension
3536
LogTester logTester = new LogTester().setLevel(Level.TRACE);
3637

38+
private final ConcurrentLinkedDeque<String> stdOut = new ConcurrentLinkedDeque<>();
39+
3740
@Test
38-
void execute_shouldLogProcessOutput() {
41+
void execute_shouldConsummeProcessStdOut() {
3942
JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
4043

41-
runner.execute(List.of("--version"), "test");
42-
assertThat(logTester.logs(Level.INFO)).isNotEmpty().allMatch(s -> s.startsWith("[stdout] "));
44+
assertThat(runner.execute(List.of("--version"), "test", stdOut::add)).isTrue();
45+
46+
assertThat(stdOut).isNotEmpty();
47+
assertThat(logTester.logs(Level.ERROR)).isEmpty();
48+
}
49+
50+
@Test
51+
void execute_shouldLogProcessStdError() {
52+
JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
4353

44-
assertThat(runner.execute(List.of("-version"), null)).isTrue();
54+
// For some reason the java process exit with 0 even with an unsupported parameter
55+
assertThat(runner.execute(List.of("-version"), null, stdOut::add)).isTrue();
4556

57+
assertThat(stdOut).isEmpty();
4658
assertThat(logTester.logs(Level.ERROR)).isNotEmpty().allMatch(s -> s.startsWith("[stderr] "));
4759
}
4860

4961
@Test
5062
void execute_whenInvalidRunner_shouldFail() {
5163
JavaRunner runner = new JavaRunner(Paths.get("invalid-runner"), JreCacheHit.DISABLED);
5264
List<String> command = List.of("--version");
53-
assertThatThrownBy(() -> runner.execute(command, "test"))
65+
assertThatThrownBy(() -> runner.execute(command, "test", stdOut::add))
5466
.isInstanceOf(IllegalStateException.class)
5567
.hasMessageContaining("Failed to run the Java command");
5668
}
@@ -59,35 +71,7 @@ void execute_whenInvalidRunner_shouldFail() {
5971
void execute_shouldReturnFalseWhenNonZeroExitCode() {
6072
JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
6173
List<String> command = List.of("unknown-command");
62-
assertThat(runner.execute(command, "test")).isFalse();
74+
assertThat(runner.execute(command, "test", stdOut::add)).isFalse();
6375
}
6476

65-
@Test
66-
void tryParse_shouldParseLogMessages() {
67-
JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
68-
69-
runner.tryParse("{\n" +
70-
" \"level\": \"ERROR\",\n" +
71-
" \"message\": \"Some error message\",\n" +
72-
" \"stacktrace\": \"exception\"\n" +
73-
"}");
74-
runner.tryParse("{\"level\": \"WARN\", \"message\": \"Some warn message\"}");
75-
runner.tryParse("{\"level\": \"DEBUG\", \"message\": \"Some debug message\"}");
76-
runner.tryParse("{\"level\": \"TRACE\", \"message\": \"Some trace message\"}");
77-
runner.tryParse("{\"level\": \"INFO\", \"message\": \"Some info message\"}");
78-
runner.tryParse("{\"level\": \"UNKNOWN-LEVEL\", \"message\": \"Some unknown level message\"}");
79-
80-
assertThat(logTester.logs(Level.ERROR)).containsOnly("Some error message\nexception");
81-
assertThat(logTester.logs(Level.WARN)).containsOnly("Some warn message");
82-
assertThat(logTester.logs(Level.DEBUG)).containsOnly("Some debug message");
83-
assertThat(logTester.logs(Level.TRACE)).containsOnly("Some trace message");
84-
assertThat(logTester.logs(Level.INFO)).containsOnly("Some info message", "Some unknown level message");
85-
}
86-
87-
@Test
88-
void tryParse_whenCannotParse_shouldLogInfo() {
89-
JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
90-
runner.tryParse("INFO: test");
91-
assertThat(logTester.logs(Level.INFO)).containsOnly("[stdout] INFO: test");
92-
}
9377
}

‎lib/src/test/java/org/sonarsource/scanner/lib/internal/ScannerEngineLauncherTest.java‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,24 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.api.extension.RegisterExtension;
2627
import org.junit.jupiter.api.io.TempDir;
28+
import org.slf4j.event.Level;
2729
import org.sonarsource.scanner.lib.ScannerProperties;
2830
import org.sonarsource.scanner.lib.internal.cache.CachedFile;
31+
import testutils.LogTester;
2932

33+
import static org.assertj.core.api.Assertions.assertThat;
34+
import static org.mockito.ArgumentMatchers.any;
35+
import static org.mockito.ArgumentMatchers.eq;
3036
import static org.mockito.Mockito.mock;
3137
import static org.mockito.Mockito.verify;
3238

3339
class ScannerEngineLauncherTest {
3440

41+
@RegisterExtension
42+
LogTester logTester = new LogTester().setLevel(Level.TRACE);
43+
3544
@TempDir
3645
private Path temp;
3746

@@ -40,6 +49,7 @@ class ScannerEngineLauncherTest {
4049
@Test
4150
void execute() {
4251
var scannerEngine = temp.resolve("scanner-engine.jar");
52+
4353
ScannerEngineLauncher launcher = new ScannerEngineLauncher(javaRunner, new CachedFile(scannerEngine, true));
4454

4555
Map<String, String> properties = Map.of(
@@ -48,7 +58,34 @@ void execute() {
4858
launcher.execute(properties);
4959

5060
verify(javaRunner).execute(
51-
List.of("-Xmx4g", "-Xms1g", "-jar", scannerEngine.toAbsolutePath().toString()),
52-
"{\"scannerProperties\":[{\"key\":\"sonar.host.url\",\"value\":\"http://localhost:9000\"},{\"key\":\"sonar.scanner.javaOpts\",\"value\":\"-Xmx4g -Xms1g\"}]}");
61+
eq(List.of("-Xmx4g", "-Xms1g", "-jar", scannerEngine.toAbsolutePath().toString())),
62+
eq("{\"scannerProperties\":[{\"key\":\"sonar.host.url\",\"value\":\"http://localhost:9000\"},{\"key\":\"sonar.scanner.javaOpts\",\"value\":\"-Xmx4g -Xms1g\"}]}"),
63+
any());
64+
}
65+
66+
@Test
67+
void tryParse_shouldParseLogMessages() {
68+
ScannerEngineLauncher.tryParse("{\n" +
69+
" \"level\": \"ERROR\",\n" +
70+
" \"message\": \"Some error message\",\n" +
71+
" \"stacktrace\": \"exception\"\n" +
72+
"}");
73+
ScannerEngineLauncher.tryParse("{\"level\": \"WARN\", \"message\": \"Some warn message\"}");
74+
ScannerEngineLauncher.tryParse("{\"level\": \"DEBUG\", \"message\": \"Some debug message\"}");
75+
ScannerEngineLauncher.tryParse("{\"level\": \"TRACE\", \"message\": \"Some trace message\"}");
76+
ScannerEngineLauncher.tryParse("{\"level\": \"INFO\", \"message\": \"Some info message\"}");
77+
ScannerEngineLauncher.tryParse("{\"level\": \"UNKNOWN-LEVEL\", \"message\": \"Some unknown level message\"}");
78+
79+
assertThat(logTester.logs(Level.ERROR)).containsOnly("Some error message\nexception");
80+
assertThat(logTester.logs(Level.WARN)).containsOnly("Some warn message");
81+
assertThat(logTester.logs(Level.DEBUG)).containsOnly("Some debug message");
82+
assertThat(logTester.logs(Level.TRACE)).containsOnly("Some trace message");
83+
assertThat(logTester.logs(Level.INFO)).containsOnly("Some info message", "Some unknown level message");
84+
}
85+
86+
@Test
87+
void tryParse_whenCannotParse_shouldLogInfo() {
88+
ScannerEngineLauncher.tryParse("INFO: test");
89+
assertThat(logTester.logs(Level.INFO)).containsOnly("[stdout] INFO: test");
5390
}
5491
}

0 commit comments

Comments
 (0)