Skip to content

Commit 5a21d94

Browse files
timbrown5timja
andauthored
Fix 'getConsoleOutput' + add tests (#230)
Co-authored-by: Tim Jacomb <[email protected]>
1 parent 4e3820f commit 5a21d94

File tree

9 files changed

+273
-51
lines changed

9 files changed

+273
-51
lines changed

src/main/frontend/pipeline-console-view/pipeline-console/main/ConsoleLogCard.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export class ConsoleLogCard extends React.Component<ConsoleLogCardProps> {
100100
let mib = 1024 * 1024;
101101
let gib = 1024 * 1024 * 1024;
102102
if (size < kib) {
103-
return `${size}b`;
103+
return `${size}B`;
104104
} else if (size < mib) {
105105
return `${(size / kib).toFixed(2)}KiB`;
106106
} else if (size < gib) {

src/main/frontend/pipeline-console-view/pipeline-console/main/PipelineConsole.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export class PipelineConsole extends React.Component<
251251
);
252252
let promise = this.getConsoleTextOffset(stepId, startByte);
253253
promise.then((res) => {
254-
step.consoleLines = res.text.split("\n") || [];
254+
step.consoleLines = res.text.trimEnd().split("\n") || [];
255255
step.consoleStartByte = res.startByte;
256256
step.consoleEndByte = res.endByte;
257257
this.setState({

src/main/java/io/jenkins/plugins/pipelinegraphview/consoleview/PipelineConsoleViewAction.java

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public class PipelineConsoleViewAction extends AbstractPipelineViewAction {
2727
private final WorkflowRun target;
2828
private final PipelineStepApi stepApi;
2929

30+
private static final ObjectMapper MAPPER = new ObjectMapper();
31+
3032
public PipelineConsoleViewAction(WorkflowRun target) {
3133
super(target);
3234
this.target = target;
@@ -55,35 +57,41 @@ public String getIconClassName() {
5557
public HttpResponse getSteps(StaplerRequest req) throws IOException {
5658
String nodeId = req.getParameter("nodeId");
5759
if (nodeId != null) {
58-
logger.debug("getSteps was passed nodeId '" + nodeId + "'.");
59-
ObjectMapper mapper = new ObjectMapper();
60-
PipelineStepList steps = stepApi.getSteps(nodeId);
61-
String stepsJson = mapper.writeValueAsString(steps);
62-
if (logger.isDebugEnabled()) {
63-
logger.debug("Steps: '" + stepsJson + "'.");
64-
}
65-
return HttpResponses.okJSON(JSONObject.fromObject(stepsJson));
60+
return HttpResponses.okJSON(getSteps(nodeId));
6661
} else {
6762
return HttpResponses.errorJSON("Error getting console text");
6863
}
6964
}
7065

66+
private JSONObject getSteps(String nodeId) throws IOException {
67+
logger.debug("getSteps was passed nodeId '" + nodeId + "'.");
68+
PipelineStepList steps = stepApi.getSteps(nodeId);
69+
String stepsJson = MAPPER.writeValueAsString(steps);
70+
if (logger.isDebugEnabled()) {
71+
logger.debug("Steps: '" + stepsJson + "'.");
72+
}
73+
return JSONObject.fromObject(stepsJson);
74+
}
7175
// Return all steps to:
7276
// - reduce number of API calls
7377
// - remove dependency of getting list of stages in frontend.
7478
@GET
7579
@WebMethod(name = "allSteps")
7680
public HttpResponse getAllSteps(StaplerRequest req) throws IOException {
77-
ObjectMapper mapper = new ObjectMapper();
81+
return HttpResponses.okJSON(getAllSteps());
82+
}
83+
84+
// Private method for testing.
85+
protected JSONObject getAllSteps() throws IOException {
7886
PipelineStepList steps = stepApi.getAllSteps();
79-
String stepsJson = mapper.writeValueAsString(steps);
87+
String stepsJson = MAPPER.writeValueAsString(steps);
8088
if (logger.isDebugEnabled()) {
8189
logger.debug("Steps: '" + stepsJson + "'.");
8290
}
83-
return HttpResponses.okJSON(JSONObject.fromObject(stepsJson));
91+
return JSONObject.fromObject(stepsJson);
8492
}
8593

86-
/* Get pre-parsed console output in json format.
94+
/*
8795
* The default behavior of this functions differs from 'getConsoleOutput' in that it will use LOG_THRESHOLD from the end of the string.
8896
* Note: if 'startByte' is negative and falls outside of the console text then we will start from byte 0.
8997
* Example:
@@ -102,10 +110,22 @@ public HttpResponse getConsoleOutput(StaplerRequest req) throws IOException {
102110
return HttpResponses.errorJSON("Error getting console json");
103111
}
104112
logger.debug("getConsoleOutput was passed node id '" + nodeId + "'.");
113+
// This will be a step, so return it's log output.
114+
// startByte to start getting data from. If negative will startByte from end of string with
115+
// LOG_THRESHOLD.
116+
Long startByte = parseIntWithDefault(req.getParameter("startByte"), -LOG_THRESHOLD);
117+
JSONObject data = getConsoleOutputJson(nodeId, startByte);
118+
if (data == null) {
119+
return HttpResponses.errorJSON("Something went wrong - check Jenkins logs.");
120+
}
121+
return HttpResponses.okJSON(data);
122+
}
105123

124+
protected JSONObject getConsoleOutputJson(String nodeId, Long requestStartByte)
125+
throws IOException {
106126
Long startByte = 0L;
107127
long endByte = 0L;
108-
long textLength = 0L;
128+
long textLength;
109129
String text = "";
110130
// If this is an exception, return the exception text (inc. stacktrace).
111131
if (isUnhandledException(nodeId)) {
@@ -114,57 +134,47 @@ public HttpResponse getConsoleOutput(StaplerRequest req) throws IOException {
114134
text = getNodeExceptionText(nodeId);
115135
endByte = text.length();
116136
} else {
117-
// This will be a step, so return it's log output.
118-
// startByte to start getting data from. If negative will startByte from end of string with
119-
// LOG_THRESHOLD.
120-
startByte = parseIntWithDefault(req.getParameter("startByte"), -LOG_THRESHOLD);
121-
122137
AnnotatedLargeText<? extends FlowNode> logText = getLogForNode(nodeId);
123138

124139
if (logText != null) {
125140
textLength = logText.length();
126141
// postitive startByte
127-
if (startByte > textLength) {
142+
if (requestStartByte > textLength) {
128143
// Avoid resource leak.
129144
logger.error("consoleJson - user requested startByte larger than console output.");
130-
return HttpResponses.errorJSON("startByte too large.");
145+
return null;
131146
}
132147
// if startByte is negative make sure we don't try and get a byte before 0.
133-
if (startByte < 0) {
134-
logger.debug(
135-
"consoleJson - requested negative startByte '" + Long.toString(startByte) + "'.");
136-
startByte = textLength + startByte;
137-
if (startByte < 0) {
148+
if (requestStartByte < 0L) {
149+
logger.debug("consoleJson - requested negative startByte '" + requestStartByte + "'.");
150+
startByte = textLength + requestStartByte;
151+
if (startByte < 0L) {
138152
logger.debug(
139153
"consoleJson - requested negative startByte '"
140-
+ Long.toString(startByte)
141-
+ "' out of bounds, setting to 0.");
154+
+ requestStartByte
155+
+ "' out of bounds, starting at 0.");
142156
startByte = 0L;
143157
}
158+
} else {
159+
startByte = requestStartByte;
144160
}
145-
146-
logger.debug(
147-
"Returning '"
148-
+ Long.toString(textLength - startByte)
149-
+ "' bytes from 'getConsoleOutput'.");
150-
} else {
151-
// If there is no text then set set startByte to 0 - as we have read from the start, there
152-
// is just nothing there.
153-
startByte = 0L;
161+
logger.debug("Returning '" + (textLength - startByte) + "' bytes from 'getConsoleOutput'.");
162+
text = PipelineNodeUtil.convertLogToString(logText, startByte);
163+
endByte = textLength;
154164
}
155165
}
156-
HashMap<String, Object> response = new HashMap<String, Object>();
166+
HashMap<String, Object> response = new HashMap<>();
157167
response.put("text", text);
158168
response.put("startByte", startByte);
159169
response.put("endByte", endByte);
160-
return HttpResponses.okJSON(JSONObject.fromObject(response));
170+
return JSONObject.fromObject(response);
161171
}
162172

163173
private AnnotatedLargeText<? extends FlowNode> getLogForNode(String nodeId) throws IOException {
164174
FlowExecution execution = target.getExecution();
165175
if (execution != null) {
166176
logger.debug("getLogForNode found execution.");
167-
PipelineNodeUtil.getLogText(execution.getNode(nodeId));
177+
return PipelineNodeUtil.getLogText(execution.getNode(nodeId));
168178
}
169179
return null;
170180
}
@@ -186,13 +196,13 @@ private boolean isUnhandledException(String nodeId) throws IOException {
186196
return false;
187197
}
188198

189-
private static long parseIntWithDefault(String s, long default_value) {
199+
private static long parseIntWithDefault(String s, long defaultValue) {
190200
try {
191201
logger.debug("Parsing user provided value of '" + s + "'");
192202
return Long.parseLong(s);
193203
} catch (NumberFormatException e) {
194-
logger.debug("Using default value of '" + default_value + "'");
195-
return default_value;
204+
logger.debug("Using default value of '" + defaultValue + "'");
205+
return defaultValue;
196206
}
197207
}
198208
}

src/main/java/io/jenkins/plugins/pipelinegraphview/utils/PipelineNodeUtil.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,6 @@ public static String convertLogToString(
256256
AnnotatedLargeText<? extends FlowNode> log, Long startByte) throws IOException {
257257
Writer stringWriter = new StringBuilderWriter();
258258
// NOTE: This returns the total length of the console log, not the received bytes.
259-
260259
log.writeHtmlTo(startByte, stringWriter);
261260
return stringWriter.toString();
262261
}
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package io.jenkins.plugins.pipelinegraphview.consoleview;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.*;
5+
6+
import hudson.Functions;
7+
import hudson.model.Result;
8+
import io.jenkins.plugins.pipelinegraphview.utils.FlowNodeWrapper;
9+
import io.jenkins.plugins.pipelinegraphview.utils.PipelineStepVisitor;
10+
import io.jenkins.plugins.pipelinegraphview.utils.TestUtils;
11+
import java.util.List;
12+
import net.sf.json.JSONObject;
13+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
14+
import org.junit.Rule;
15+
import org.junit.Test;
16+
import org.jvnet.hudson.test.Issue;
17+
import org.jvnet.hudson.test.JenkinsRule;
18+
19+
public class PipelineConsoleViewActionTest {
20+
@Rule public JenkinsRule j = new JenkinsRule();
21+
22+
private static final String TEXT = "Hello, World!" + System.lineSeparator();
23+
24+
@Issue("GH#224")
25+
@Test
26+
public void getConsoleLogReturnLogText() throws Exception {
27+
WorkflowRun run =
28+
TestUtils.createAndRunJob(
29+
j, "hello_world_scripted", "helloWorldScriptedPipeline.jenkinsfile", Result.SUCCESS);
30+
31+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
32+
String stageId = TestUtils.getNodesByDisplayName(run, "Say Hello").get(0).getId();
33+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
34+
FlowNodeWrapper echoStep = stepNodes.get(0);
35+
36+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
37+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(echoStep.getId(), 0L);
38+
assertThat(consoleJson.getString("endByte"), is(String.valueOf(TEXT.length())));
39+
assertThat(consoleJson.getString("startByte"), is("0"));
40+
assertThat(consoleJson.getString("text"), is(TEXT));
41+
}
42+
43+
@Issue("GH#224")
44+
@Test
45+
public void getConsoleLogReturnsErrorText() throws Exception {
46+
WorkflowRun run =
47+
TestUtils.createAndRunJob(
48+
j, "hello_world_scripted", "simpleError.jenkinsfile", Result.FAILURE);
49+
50+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
51+
String stageId = TestUtils.getNodesByDisplayName(run, "A").get(0).getId();
52+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
53+
FlowNodeWrapper errorStep = stepNodes.get(0);
54+
55+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
56+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(errorStep.getId(), 0L);
57+
assertThat(consoleJson.getString("endByte"), is("16"));
58+
assertThat(consoleJson.getString("startByte"), is("0"));
59+
assertThat(consoleJson.getString("text"), is("This is an error"));
60+
}
61+
62+
@Issue("GH#224")
63+
@Test
64+
public void getConsoleLogReturnLogTextWithOffset() throws Exception {
65+
WorkflowRun run =
66+
TestUtils.createAndRunJob(
67+
j, "hello_world_scripted", "helloWorldScriptedPipeline.jenkinsfile", Result.SUCCESS);
68+
69+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
70+
String stageId = TestUtils.getNodesByDisplayName(run, "Say Hello").get(0).getId();
71+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
72+
FlowNodeWrapper echoStep = stepNodes.get(0);
73+
74+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
75+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(echoStep.getId(), 7L);
76+
assertThat(consoleJson.getString("endByte"), is(String.valueOf(TEXT.length())));
77+
assertThat(consoleJson.getString("startByte"), is("7"));
78+
assertThat(consoleJson.getString("text"), is("World!" + System.lineSeparator()));
79+
}
80+
81+
@Issue("GH#224")
82+
@Test
83+
public void getConsoleLogReturnLogTextWithNegativeOffset() throws Exception {
84+
WorkflowRun run =
85+
TestUtils.createAndRunJob(
86+
j, "hello_world_scripted", "helloWorldScriptedPipeline.jenkinsfile", Result.SUCCESS);
87+
88+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
89+
String stageId = TestUtils.getNodesByDisplayName(run, "Say Hello").get(0).getId();
90+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
91+
FlowNodeWrapper echoStep = stepNodes.get(0);
92+
93+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
94+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(echoStep.getId(), -7L);
95+
// 14-7
96+
assertThat(consoleJson.getString("endByte"), is(String.valueOf(TEXT.length())));
97+
assertThat(
98+
consoleJson.getString("startByte"),
99+
is(String.valueOf(7 + (Functions.isWindows() ? 1 : 0))));
100+
String value = (Functions.isWindows() ? "" : "W") + "orld!" + System.lineSeparator();
101+
assertThat(consoleJson.getString("text"), is(value));
102+
}
103+
104+
@Issue("GH#224")
105+
@Test
106+
public void getConsoleLogReturnLogTextWithLargeNegativeOffset() throws Exception {
107+
WorkflowRun run =
108+
TestUtils.createAndRunJob(
109+
j, "hello_world_scripted", "helloWorldScriptedPipeline.jenkinsfile", Result.SUCCESS);
110+
111+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
112+
String stageId = TestUtils.getNodesByDisplayName(run, "Say Hello").get(0).getId();
113+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
114+
FlowNodeWrapper echoStep = stepNodes.get(0);
115+
116+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
117+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(echoStep.getId(), -1000L);
118+
assertThat(consoleJson.getString("endByte"), is(String.valueOf(TEXT.length())));
119+
assertThat(consoleJson.getString("startByte"), is("0"));
120+
assertThat(consoleJson.getString("text"), is(TEXT));
121+
}
122+
123+
@Issue("GH#224")
124+
@Test
125+
public void getConsoleLogReturnLogTextWithLargeOffset() throws Exception {
126+
WorkflowRun run =
127+
TestUtils.createAndRunJob(
128+
j, "hello_world_scripted", "helloWorldScriptedPipeline.jenkinsfile", Result.SUCCESS);
129+
130+
PipelineStepVisitor builder = new PipelineStepVisitor(run);
131+
String stageId = TestUtils.getNodesByDisplayName(run, "Say Hello").get(0).getId();
132+
List<FlowNodeWrapper> stepNodes = builder.getStageSteps(stageId);
133+
FlowNodeWrapper echoStep = stepNodes.get(0);
134+
135+
PipelineConsoleViewAction consoleAction = new PipelineConsoleViewAction(run);
136+
JSONObject consoleJson = consoleAction.getConsoleOutputJson(echoStep.getId(), 1000L);
137+
assertThat(consoleJson, is(nullValue()));
138+
}
139+
}

0 commit comments

Comments
 (0)