Skip to content

Commit 1c346ba

Browse files
janikmuellerjanikmueller
authored
Merge pull request #21 from cqse/45170_add_logging_if_timing_out
45170 improve logging when waiting for analysis to finish
2 parents 5270c8a + 2db2c04 commit 1c346ba

File tree

7 files changed

+993
-61
lines changed

7 files changed

+993
-61
lines changed

‎pom.xml‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,6 @@
3030
<artifactId>okhttp</artifactId>
3131
<version>3.14.2</version>
3232
</dependency>
33-
<dependency>
34-
<groupId>com.fasterxml.jackson.core</groupId>
35-
<artifactId>jackson-databind</artifactId>
36-
<version>2.9.10.5</version>
37-
</dependency>
3833
<dependency>
3934
<groupId>info.picocli</groupId>
4035
<artifactId>picocli</artifactId>
@@ -56,9 +51,16 @@
5651
<version>2.4.0</version>
5752
</dependency>
5853
<dependency>
59-
<groupId>com.google.code.gson</groupId>
60-
<artifactId>gson</artifactId>
61-
<version>2.8.6</version>
54+
<groupId>org.junit.jupiter</groupId>
55+
<artifactId>junit-jupiter</artifactId>
56+
<version>5.7.2</version>
57+
<scope>test</scope>
58+
</dependency>
59+
<dependency>
60+
<groupId>com.squareup.okhttp3</groupId>
61+
<artifactId>mockwebserver</artifactId>
62+
<version>3.14.2</version>
63+
<scope>test</scope>
6264
</dependency>
6365
<dependency>
6466
<groupId>org.assertj</groupId>
@@ -72,18 +74,6 @@
7274
<version>1.7.28</version>
7375
<scope>compile</scope>
7476
</dependency>
75-
<dependency>
76-
<groupId>com.squareup.moshi</groupId>
77-
<artifactId>moshi</artifactId>
78-
<version>1.11.0</version>
79-
<scope>test</scope>
80-
</dependency>
81-
<dependency>
82-
<groupId>com.sparkjava</groupId>
83-
<artifactId>spark-core</artifactId>
84-
<version>2.5</version>
85-
<scope>test</scope>
86-
</dependency>
8777
</dependencies>
8878

8979
<build>
@@ -199,8 +189,10 @@
199189
<goal>process-asciidoc</goal>
200190
</goals>
201191
<configuration>
202-
<sourceHighlighter>coderay</sourceHighlighter>
203192
<backend>html5</backend>
193+
<attributes>
194+
<source-highlighter>coderay</source-highlighter>
195+
</attributes>
204196
</configuration>
205197
</execution>
206198
<execution>
@@ -210,8 +202,10 @@
210202
<goal>process-asciidoc</goal>
211203
</goals>
212204
<configuration>
213-
<sourceHighlighter>coderay</sourceHighlighter>
214205
<backend>manpage</backend>
206+
<attributes>
207+
<source-highlighter>coderay</source-highlighter>
208+
</attributes>
215209
</configuration>
216210
</execution>
217211
</executions>

‎src/main/java/com/teamscale/buildbreaker/commandline/BuildBreaker.java‎

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.teamscale.buildbreaker.evaluation.FindingsEvaluator;
1414
import com.teamscale.buildbreaker.evaluation.MetricViolation;
1515
import com.teamscale.buildbreaker.evaluation.MetricsEvaluator;
16+
import com.teamscale.buildbreaker.teamscale_client.AnalysisState;
1617
import com.teamscale.buildbreaker.teamscale_client.TeamscaleClient;
1718
import com.teamscale.buildbreaker.teamscale_client.exceptions.CommitCouldNotBeResolvedException;
1819
import com.teamscale.buildbreaker.teamscale_client.exceptions.HttpRedirectException;
@@ -39,6 +40,7 @@
3940
import java.time.Duration;
4041
import java.time.LocalDateTime;
4142
import java.time.ZoneOffset;
43+
import java.time.chrono.ChronoLocalDateTime;
4244
import java.time.format.DateTimeFormatter;
4345
import java.util.List;
4446
import java.util.Objects;
@@ -176,7 +178,7 @@ public Integer call() throws Exception {
176178
fail("Could not resolve revision " + e.getRevision() +
177179
" to a valid commit known to Teamscale (too many commits returned): " + e.getCommitDescriptorsJson());
178180
} catch (IOException e) {
179-
fail("Encountered an error while communicating with Teamscale: ");
181+
fail("Encountered an error while communicating with Teamscale: " + e.getMessage());
180182
} finally {
181183
// we must shut down OkHttp as otherwise it will leave threads running and
182184
// prevent JVM shutdown
@@ -281,34 +283,60 @@ private void initDefaultOptions() {
281283
}
282284

283285
private void waitForAnalysisToFinish(String branchAndTimestampToWaitFor) throws IOException, InterruptedException, HttpRedirectException, HttpStatusCodeException {
286+
String[] split = branchAndTimestampToWaitFor.split(":", 2);
287+
String branch = split[0];
288+
long requestedTimestamp = Long.parseLong(split[1]);
284289
LocalDateTime timeout = LocalDateTime.now().plus(waitForAnalysisTimeoutDuration);
285-
boolean teamscaleAnalysisFinished = teamscaleClient.isTeamscaleAnalysisFinished(branchAndTimestampToWaitFor);
286-
if (!teamscaleAnalysisFinished) {
290+
AnalysisState analysisState = teamscaleClient.fetchAnalysisState(branch);
291+
boolean analysisFinished = analysisState.timestamp >= requestedTimestamp;
292+
if (!analysisFinished) {
287293
System.out.println(
288294
"The commit that should be evaluated has not yet been analyzed on the Teamscale instance. Triggering Teamscale commit hook on repository.");
289-
try {
290-
teamscaleClient.triggerCommitHookEvent(remoteRepositoryUrl);
291-
System.out.println("Commit hook triggered successfully.");
292-
} catch (RepositoryNotFoundException e) {
293-
System.out.println(
294-
"Failed to automatically detect the remote repository URL. Please specify it manually via --repository-url to enable sending a commit hook event to Teamscale.");
295-
} catch (IOException e) {
296-
System.out.println("Failure when trying to send the commit hook event to Teamscale: " + e);
297-
}
295+
triggerCommitHook();
296+
System.out.println("Start querying the analysis state for '" + branchAndTimestampToWaitFor + "' every ten seconds until it has been analyzed or the timeout is reached at " +
297+
DateTimeFormatter.RFC_1123_DATE_TIME.format(timeout.atZone(ZoneOffset.UTC)) +
298+
". You can change this timeout using --wait-for-analysis-timeout.");
299+
analysisState = waitForCommitBeingAnalyzed(analysisState, branch, requestedTimestamp, timeout);
300+
analysisFinished = analysisState.timestamp >= requestedTimestamp;
301+
}
298302

303+
if (!analysisFinished) {
304+
String timeoutMessage = "The result for the commit with timestamp " + requestedTimestamp
305+
+ " could not be retrieved because the last processed commit has the timestamp "
306+
+ analysisState.timestamp + " and the analysis was in state " + analysisState.state + ".";
307+
if (analysisState.rollbackId != null) {
308+
timeoutMessage += " (rollback id: " + analysisState.rollbackId + ")";
309+
}
310+
throw new AnalysisNotFinishedException(timeoutMessage);
299311
}
300-
while (!teamscaleAnalysisFinished && LocalDateTime.now().isBefore(timeout)) {
312+
}
313+
314+
private void triggerCommitHook() throws HttpRedirectException, HttpStatusCodeException {
315+
try {
316+
teamscaleClient.triggerCommitHookEvent(remoteRepositoryUrl);
317+
System.out.println("Commit hook triggered successfully.");
318+
} catch (RepositoryNotFoundException e) {
301319
System.out.println(
302-
"The commit that should be evaluated has not yet been analyzed on the Teamscale instance. Will retry in ten seconds until the timeout is reached at " +
303-
DateTimeFormatter.RFC_1123_DATE_TIME.format(timeout.atZone(ZoneOffset.UTC)) +
304-
". You can change this timeout using --wait-for-analysis-timeout.");
305-
Thread.sleep(Duration.ofSeconds(10).toMillis());
306-
teamscaleAnalysisFinished = teamscaleClient.isTeamscaleAnalysisFinished(branchAndTimestampToWaitFor);
320+
"Failed to automatically detect the remote repository URL. Please specify it manually via --repository-url to enable sending a commit hook event to Teamscale.");
321+
} catch (IOException e) {
322+
System.out.println("Failure when trying to send the commit hook event to Teamscale: " + e);
307323
}
308-
if (!teamscaleAnalysisFinished) {
309-
throw new AnalysisNotFinishedException(
310-
"The commit that should be evaluated was not analyzed by Teamscale in time before the analysis timeout.");
324+
}
325+
326+
private AnalysisState waitForCommitBeingAnalyzed(AnalysisState analysisState, String branch, long requestedTimestamp, ChronoLocalDateTime<?> timeout) throws InterruptedException, HttpRedirectException, HttpStatusCodeException, IOException {
327+
boolean analysisFinished = false;
328+
while (!analysisFinished && LocalDateTime.now().isBefore(timeout)) {
329+
Thread.sleep(Duration.ofSeconds(10).toMillis());
330+
analysisState = teamscaleClient.fetchAnalysisState(branch);
331+
analysisFinished = analysisState.timestamp >= requestedTimestamp;
332+
String logMessage = "Current analysis state: state=" + analysisState.state
333+
+ ", last processed timestamp=" + analysisState.timestamp;
334+
if (analysisState.rollbackId != null) {
335+
logMessage += ", rollback id=" + analysisState.rollbackId;
336+
}
337+
System.out.println(logMessage);
311338
}
339+
return analysisState;
312340
}
313341

314342
private String determineBranchAndTimestamp() throws IOException, TooManyCommitsException, HttpRedirectException, HttpStatusCodeException, CommitCouldNotBeResolvedException {

‎src/main/java/com/teamscale/buildbreaker/teamscale_client/AnalysisState.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.teamscale.buildbreaker.teamscale_client;
2+
3+
/**
4+
* Mirrors the response of the {@code /api/projects/{project}/branch-analysis-state/{branch}} endpoint.
5+
*/
6+
public class AnalysisState {
7+
public final long timestamp;
8+
public final String state;
9+
public final String rollbackId;
10+
11+
public AnalysisState(long timestamp, String state, String rollbackId) {
12+
this.timestamp = timestamp;
13+
this.state = state;
14+
this.rollbackId = rollbackId;
15+
}
16+
}

‎src/main/java/com/teamscale/buildbreaker/teamscale_client/TeamscaleClient.java‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -217,19 +217,7 @@ public void triggerCommitHookEvent(String remoteRepositoryUrl) throws IOExceptio
217217
sendRequest(request);
218218
}
219219

220-
/**
221-
* @return whether the analysis has reached the given timestamp on the given branch already
222-
* @throws HttpRedirectException if a redirect is encountered
223-
* @throws HttpStatusCodeException if an HTTP error code was returned by Teamscale
224-
*/
225-
public boolean isTeamscaleAnalysisFinished(String branchAndTimestampToWaitFor) throws IOException, HttpRedirectException, HttpStatusCodeException {
226-
String[] branchAndTimestamp = branchAndTimestampToWaitFor.split(":", 2);
227-
String branch = branchAndTimestamp[0];
228-
long timestamp = Long.parseLong(branchAndTimestamp[1]);
229-
return isAnalysisFinished(branch, timestamp);
230-
}
231-
232-
private boolean isAnalysisFinished(String branch, long timestamp) throws IOException, HttpRedirectException, HttpStatusCodeException {
220+
public AnalysisState fetchAnalysisState(String branch) throws IOException, HttpRedirectException, HttpStatusCodeException {
233221
HttpUrl.Builder builder =
234222
teamscaleServerUrl.newBuilder()
235223
.addPathSegments("api/projects")
@@ -239,14 +227,21 @@ private boolean isAnalysisFinished(String branch, long timestamp) throws IOExcep
239227
HttpUrl url = builder.build();
240228
Request request = createAuthenticatedGetRequest(url);
241229
String analysisStateJson = sendRequest(request);
242-
DocumentContext analysisState = JsonPath.parse(analysisStateJson);
230+
DocumentContext context = JsonPath.parse(analysisStateJson);
231+
long lastFinishedTimestamp;
243232
try {
244-
Integer lastFinishedTimestamp = analysisState.read("$.timestamp");
245-
return lastFinishedTimestamp >= timestamp;
233+
lastFinishedTimestamp = context.<Integer>read("$.timestamp");
246234
} catch (ClassCastException e) {
247-
Long lastFinishedTimestamp = analysisState.read("$.timestamp");
248-
return lastFinishedTimestamp >= timestamp;
235+
lastFinishedTimestamp = context.read("$.timestamp");
236+
}
237+
String state = context.read("$.state");
238+
String rollbackId = null;
239+
try {
240+
rollbackId = context.read("$.rollbackId");
241+
} catch (PathNotFoundException e) {
242+
// rollbackId is optional
249243
}
244+
return new AnalysisState(lastFinishedTimestamp, state, rollbackId);
250245
}
251246

252247
private Pair<List<Finding>, List<Finding>> parseCommitFindingResponse(String response, String uniformPath) throws ParserException {

‎src/test/java/com/teamscale/buildbreaker/evaluation/FindingsEvaluatorTest.java‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package com.teamscale.buildbreaker.evaluation;
2+
3+
import org.conqat.lib.commons.collections.Pair;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.util.Collections;
7+
import java.util.List;
8+
9+
import static com.teamscale.buildbreaker.evaluation.ProblemCategory.ERROR;
10+
import static com.teamscale.buildbreaker.evaluation.ProblemCategory.NO_PROBLEM;
11+
import static com.teamscale.buildbreaker.evaluation.ProblemCategory.WARNING;
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
14+
class FindingsEvaluatorTest {
15+
16+
private final FindingsEvaluator evaluator = new FindingsEvaluator();
17+
18+
@Test
19+
void emptyFindingsProduceNoViolations() {
20+
EvaluationResult result = evaluator.evaluate(
21+
Pair.createPair(Collections.emptyList(), Collections.emptyList()), false, false);
22+
assertThat(result.toStatusCode()).isEqualTo(0);
23+
}
24+
25+
@Test
26+
void errorFindingIsAlwaysReported() {
27+
Finding error = createFinding("err-1", ERROR);
28+
EvaluationResult result = evaluator.evaluate(
29+
Pair.createPair(List.of(error), Collections.emptyList()), false, false);
30+
assertThat(result.toStatusCode()).isEqualTo(1);
31+
assertThat(result.toString()).contains("err-1");
32+
}
33+
34+
@Test
35+
void warningFindingIsSkippedWhenFailOnYellowIsFalse() {
36+
Finding warning = createFinding("warn-1", WARNING);
37+
EvaluationResult result = evaluator.evaluate(
38+
Pair.createPair(List.of(warning), Collections.emptyList()), false, false);
39+
assertThat(result.toStatusCode()).isEqualTo(0);
40+
}
41+
42+
@Test
43+
void warningFindingIsReportedWhenFailOnYellowIsTrue() {
44+
Finding warning = createFinding("warn-1", WARNING);
45+
EvaluationResult result = evaluator.evaluate(
46+
Pair.createPair(List.of(warning), Collections.emptyList()), true, false);
47+
assertThat(result.toStatusCode()).isEqualTo(2);
48+
assertThat(result.toString()).contains("warn-1");
49+
}
50+
51+
@Test
52+
void noProblemFindingDoesNotAffectStatusCode() {
53+
Finding noProblem = createFinding("np-1", NO_PROBLEM);
54+
EvaluationResult result = evaluator.evaluate(
55+
Pair.createPair(List.of(noProblem), Collections.emptyList()), true, false);
56+
assertThat(result.toStatusCode()).isEqualTo(0);
57+
}
58+
59+
@Test
60+
void changedCodeFindingsExcludedWhenIncludeChangedCodeIsFalse() {
61+
Finding changedCodeError = createFinding("changed-err", ERROR);
62+
EvaluationResult result = evaluator.evaluate(
63+
Pair.createPair(Collections.emptyList(), List.of(changedCodeError)), false, false);
64+
assertThat(result.toStatusCode()).isEqualTo(0);
65+
}
66+
67+
@Test
68+
void changedCodeFindingsIncludedWhenIncludeChangedCodeIsTrue() {
69+
Finding changedCodeError = createFinding("changed-err", ERROR);
70+
EvaluationResult result = evaluator.evaluate(
71+
Pair.createPair(Collections.emptyList(), List.of(changedCodeError)), false, true);
72+
assertThat(result.toStatusCode()).isEqualTo(1);
73+
assertThat(result.toString()).contains("changed-err");
74+
}
75+
76+
@Test
77+
void newAndChangedCodeFindingsCombinedWhenIncludeChangedCodeIsTrue() {
78+
Finding newCodeError = createFinding("new-err", ERROR);
79+
Finding changedCodeWarning = createFinding("changed-warn", WARNING);
80+
EvaluationResult result = evaluator.evaluate(
81+
Pair.createPair(List.of(newCodeError), List.of(changedCodeWarning)), true, true);
82+
assertThat(result.toStatusCode()).isEqualTo(1);
83+
assertThat(result.toString()).contains("new-err");
84+
assertThat(result.toString()).contains("changed-warn");
85+
}
86+
87+
@Test
88+
void onlyNewCodeFindingsEvaluatedWhenIncludeChangedCodeIsFalse() {
89+
Finding newCodeError = createFinding("new-err", ERROR);
90+
Finding changedCodeError = createFinding("changed-err", ERROR);
91+
EvaluationResult result = evaluator.evaluate(
92+
Pair.createPair(List.of(newCodeError), List.of(changedCodeError)), false, false);
93+
assertThat(result.toString()).contains("new-err");
94+
assertThat(result.toString()).doesNotContain("changed-err");
95+
}
96+
97+
@Test
98+
void errorAndWarningMixWithFailOnYellowFalse() {
99+
Finding error = createFinding("err-1", ERROR);
100+
Finding warning = createFinding("warn-1", WARNING);
101+
EvaluationResult result = evaluator.evaluate(
102+
Pair.createPair(List.of(error, warning), Collections.emptyList()), false, false);
103+
assertThat(result.toStatusCode()).isEqualTo(1);
104+
assertThat(result.toString()).contains("err-1");
105+
assertThat(result.toString()).doesNotContain("warn-1");
106+
}
107+
108+
@Test
109+
void errorAndWarningMixWithFailOnYellowTrue() {
110+
Finding error = createFinding("err-1", ERROR);
111+
Finding warning = createFinding("warn-1", WARNING);
112+
EvaluationResult result = evaluator.evaluate(
113+
Pair.createPair(List.of(error, warning), Collections.emptyList()), true, false);
114+
assertThat(result.toStatusCode()).isEqualTo(1);
115+
assertThat(result.toString()).contains("err-1");
116+
assertThat(result.toString()).contains("warn-1");
117+
}
118+
119+
@Test
120+
void multipleWarningsWithFailOnYellowTrue() {
121+
Finding warning1 = createFinding("warn-1", WARNING);
122+
Finding warning2 = createFinding("warn-2", WARNING);
123+
EvaluationResult result = evaluator.evaluate(
124+
Pair.createPair(List.of(warning1, warning2), Collections.emptyList()), true, false);
125+
assertThat(result.toStatusCode()).isEqualTo(2);
126+
assertThat(result.toString()).contains("warn-1");
127+
assertThat(result.toString()).contains("warn-2");
128+
}
129+
130+
private static Finding createFinding(String id, ProblemCategory assessment) {
131+
return new Finding(id, "TestGroup", "TestCategory", "Test message", "path/to/File.java", assessment);
132+
}
133+
}

0 commit comments

Comments
 (0)