Skip to content

Commit 65ffcef

Browse files
C0urantetimja
C0urante
and
timja
authored
[JENKINS-27931] Replace keepLongStdio with stdioRetention, allow test output to be retained for failed tests only (#601)
Co-authored-by: Tim Jacomb <[email protected]>
1 parent 14bf914 commit 65ffcef

17 files changed

+367
-102
lines changed

‎src/main/java/hudson/tasks/junit/CaseResult.java‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,12 @@ public CaseResult(
167167
this.properties = Collections.emptyMap();
168168
}
169169

170+
@Deprecated
170171
CaseResult(SuiteResult parent, Element testCase, String testClassName, boolean keepLongStdio, boolean keepProperties) {
172+
this(parent, testCase, testClassName, StdioRetention.fromKeepLongStdio(keepLongStdio), keepProperties);
173+
}
174+
175+
CaseResult(SuiteResult parent, Element testCase, String testClassName, StdioRetention stdioRetention, boolean keepProperties) {
171176
// schema for JUnit report XML format is not available in Ant,
172177
// so I don't know for sure what means what.
173178
// reports in http://www.nabble.com/difference-in-junit-publisher-and-ant-junitreport-tf4308604.html#a12265700
@@ -201,8 +206,8 @@ public CaseResult(
201206
skippedMessage = getSkippedMessage(testCase);
202207
@SuppressWarnings("LeakingThisInConstructor")
203208
Collection<CaseResult> _this = Collections.singleton(this);
204-
stdout = fixNULs(possiblyTrimStdio(_this, keepLongStdio, testCase.elementText("system-out")));
205-
stderr = fixNULs(possiblyTrimStdio(_this, keepLongStdio, testCase.elementText("system-err")));
209+
stdout = fixNULs(possiblyTrimStdio(_this, stdioRetention, testCase.elementText("system-out")));
210+
stderr = fixNULs(possiblyTrimStdio(_this, stdioRetention, testCase.elementText("system-err")));
206211

207212
// parse properties
208213
Map<String, String> properties = new HashMap<String, String>();
@@ -223,11 +228,13 @@ public CaseResult(
223228
this.properties = properties;
224229
}
225230

226-
static String possiblyTrimStdio(Collection<CaseResult> results, boolean keepLongStdio, String stdio) { // HUDSON-6516
231+
static String possiblyTrimStdio(Collection<CaseResult> results, StdioRetention stdioRetention, String stdio) { // HUDSON-6516
227232
if (stdio == null) {
228233
return null;
229234
}
230-
if (keepLongStdio) {
235+
boolean keepAll = stdioRetention == StdioRetention.ALL
236+
|| (stdioRetention == StdioRetention.FAILED && hasFailures(results));
237+
if (keepAll) {
231238
return stdio;
232239
}
233240
int len = stdio.length();
@@ -244,14 +251,17 @@ static String fixNULs(String stdio) { // JENKINS-71139
244251
}
245252

246253
/**
247-
* Flavor of {@link #possiblyTrimStdio(Collection, boolean, String)} that doesn't try to read the whole thing into memory.
254+
* Flavor of {@link #possiblyTrimStdio(Collection, StdioRetention, String)} that doesn't try to read the whole thing into memory.
248255
*/
249256
@SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "Expected behavior")
250-
static String possiblyTrimStdio(Collection<CaseResult> results, boolean keepLongStdio, File stdio) throws IOException {
257+
static String possiblyTrimStdio(Collection<CaseResult> results, StdioRetention stdioRetention, File stdio) throws IOException {
251258
long len = stdio.length();
252-
if (keepLongStdio && len < 1024 * 1024) {
259+
boolean keepAll = stdioRetention == StdioRetention.ALL
260+
|| (stdioRetention == StdioRetention.FAILED && hasFailures(results));
261+
if (keepAll && len < 1024 * 1024) {
253262
return FileUtils.readFileToString(stdio);
254263
}
264+
255265
int halfMaxSize = halfMaxSize(results);
256266

257267
long middle = len - halfMaxSize * 2;
@@ -278,12 +288,11 @@ static String possiblyTrimStdio(Collection<CaseResult> results, boolean keepLong
278288
private static final int HALF_MAX_SIZE = 500;
279289
private static final int HALF_MAX_FAILING_SIZE = 50000;
280290
private static int halfMaxSize(Collection<CaseResult> results) {
281-
for (CaseResult result : results) {
282-
if (result.errorStackTrace != null) {
283-
return HALF_MAX_FAILING_SIZE;
284-
}
285-
}
286-
return HALF_MAX_SIZE;
291+
return hasFailures(results) ? HALF_MAX_FAILING_SIZE : HALF_MAX_SIZE;
292+
}
293+
294+
private static boolean hasFailures(Collection<CaseResult> results) {
295+
return results.stream().anyMatch(r -> r.errorStackTrace != null);
287296
}
288297

289298
@Override

‎src/main/java/hudson/tasks/junit/JUnitParser.java‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public class JUnitParser extends TestResultParser {
5252

5353
private static final Logger LOGGER = Logger.getLogger(JUnitParser.class.getName());
5454

55-
private final boolean keepLongStdio;
55+
private final StdioRetention stdioRetention;
5656
private final boolean keepProperties;
5757
private final boolean allowEmptyResults;
5858

@@ -83,8 +83,13 @@ public JUnitParser(boolean keepLongStdio, boolean allowEmptyResults) {
8383
this(keepLongStdio, false, allowEmptyResults, false);
8484
}
8585

86+
@Deprecated
8687
public JUnitParser(boolean keepLongStdio, boolean keepProperties, boolean allowEmptyResults, boolean skipOldReports) {
87-
this.keepLongStdio = keepLongStdio;
88+
this(StdioRetention.fromKeepLongStdio(keepLongStdio), keepProperties, allowEmptyResults, skipOldReports);
89+
}
90+
91+
public JUnitParser(StdioRetention stdioRetention, boolean keepProperties, boolean allowEmptyResults, boolean skipOldReports) {
92+
this.stdioRetention = stdioRetention;
8893
this.keepProperties = keepProperties;
8994
this.allowEmptyResults = allowEmptyResults;
9095
this.skipOldReports = skipOldReports;
@@ -117,14 +122,14 @@ public TestResult parseResult(String testResultLocations, Run<?,?> build, FilePa
117122
public TestResult parseResult(String testResultLocations, Run<?,?> build, PipelineTestDetails pipelineTestDetails,
118123
FilePath workspace, Launcher launcher, TaskListener listener)
119124
throws InterruptedException, IOException {
120-
return workspace.act(new DirectParseResultCallable(testResultLocations, build, keepLongStdio, keepProperties, allowEmptyResults,
125+
return workspace.act(new DirectParseResultCallable(testResultLocations, build, stdioRetention, keepProperties, allowEmptyResults,
121126
pipelineTestDetails, listener, skipOldReports));
122127
}
123128

124129
public TestResultSummary summarizeResult(String testResultLocations, Run<?,?> build, PipelineTestDetails pipelineTestDetails,
125130
FilePath workspace, Launcher launcher, TaskListener listener, JunitTestResultStorage storage)
126131
throws InterruptedException, IOException {
127-
return workspace.act(new StorageParseResultCallable(testResultLocations, build, keepLongStdio, keepProperties, allowEmptyResults,
132+
return workspace.act(new StorageParseResultCallable(testResultLocations, build, stdioRetention, keepProperties, allowEmptyResults,
128133
pipelineTestDetails, listener, storage.createRemotePublisher(build), skipOldReports));
129134
}
130135

@@ -137,7 +142,7 @@ private static abstract class ParseResultCallable<T> extends MasterToSlaveFileCa
137142
private final long buildTimeInMillis;
138143
private final String testResults;
139144
private final long nowMaster;
140-
private final boolean keepLongStdio;
145+
private final StdioRetention stdioRetention;
141146
private final boolean keepProperties;
142147
private final boolean allowEmptyResults;
143148
private final PipelineTestDetails pipelineTestDetails;
@@ -146,14 +151,14 @@ private static abstract class ParseResultCallable<T> extends MasterToSlaveFileCa
146151
private boolean skipOldReports;
147152

148153
private ParseResultCallable(String testResults, Run<?,?> build,
149-
boolean keepLongStdio, boolean keepProperties, boolean allowEmptyResults,
154+
StdioRetention stdioRetention, boolean keepProperties, boolean allowEmptyResults,
150155
PipelineTestDetails pipelineTestDetails, TaskListener listener,
151156
boolean skipOldReports) {
152157
this.buildStartTimeInMillis = build.getStartTimeInMillis();
153158
this.buildTimeInMillis = build.getTimeInMillis();
154159
this.testResults = testResults;
155160
this.nowMaster = System.currentTimeMillis();
156-
this.keepLongStdio = keepLongStdio;
161+
this.stdioRetention = stdioRetention;
157162
this.keepProperties = keepProperties;
158163
this.allowEmptyResults = allowEmptyResults;
159164
this.pipelineTestDetails = pipelineTestDetails;
@@ -177,7 +182,7 @@ public T invoke(File ws, VirtualChannel channel) throws IOException {
177182
+ ",buildTimeInMillis:" + buildTimeInMillis + ",filesTimestamp:" + filesTimestamp + ",nowSlave:"
178183
+ nowSlave + ",nowMaster:" + nowMaster);
179184
}
180-
result = new TestResult(filesTimestamp, ds, keepLongStdio, keepProperties, pipelineTestDetails, skipOldReports);
185+
result = new TestResult(filesTimestamp, ds, stdioRetention, keepProperties, pipelineTestDetails, skipOldReports);
181186
result.tally();
182187
} else {
183188
if (this.allowEmptyResults) {
@@ -197,9 +202,9 @@ public T invoke(File ws, VirtualChannel channel) throws IOException {
197202

198203
private static final class DirectParseResultCallable extends ParseResultCallable<TestResult> {
199204

200-
DirectParseResultCallable(String testResults, Run<?,?> build, boolean keepLongStdio, boolean keepProperties, boolean allowEmptyResults,
205+
DirectParseResultCallable(String testResults, Run<?,?> build, StdioRetention stdioRetention, boolean keepProperties, boolean allowEmptyResults,
201206
PipelineTestDetails pipelineTestDetails, TaskListener listener, boolean skipOldReports) {
202-
super(testResults, build, keepLongStdio, keepProperties, allowEmptyResults, pipelineTestDetails, listener, skipOldReports);
207+
super(testResults, build, stdioRetention, keepProperties, allowEmptyResults, pipelineTestDetails, listener, skipOldReports);
203208
}
204209

205210
@Override
@@ -213,9 +218,9 @@ private static final class StorageParseResultCallable extends ParseResultCallabl
213218

214219
private final RemotePublisher publisher;
215220

216-
StorageParseResultCallable(String testResults, Run<?,?> build, boolean keepLongStdio, boolean keepProperties, boolean allowEmptyResults,
221+
StorageParseResultCallable(String testResults, Run<?,?> build, StdioRetention stdioRetention, boolean keepProperties, boolean allowEmptyResults,
217222
PipelineTestDetails pipelineTestDetails, TaskListener listener, RemotePublisher publisher, boolean skipOldReports) {
218-
super(testResults, build, keepLongStdio, keepProperties, allowEmptyResults, pipelineTestDetails, listener, skipOldReports);
223+
super(testResults, build, stdioRetention, keepProperties, allowEmptyResults, pipelineTestDetails, listener, skipOldReports);
219224
this.publisher = publisher;
220225
}
221226

‎src/main/java/hudson/tasks/junit/JUnitResultArchiver.java‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import hudson.tasks.test.PipelineTestDetails;
4747
import hudson.util.DescribableList;
4848
import hudson.util.FormValidation;
49+
import hudson.util.ListBoxModel;
4950
import io.jenkins.plugins.junit.checks.JUnitChecksPublisher;
5051
import io.jenkins.plugins.junit.storage.FileJunitTestResultStorage;
5152
import io.jenkins.plugins.junit.storage.JunitTestResultStorage;
@@ -80,10 +81,9 @@ public class JUnitResultArchiver extends Recorder implements SimpleBuildStep, JU
8081
private final String testResults;
8182

8283
/**
83-
* If true, retain a suite's complete stdout/stderr even if this is huge and the suite passed.
84-
* @since 1.358
84+
* Whether to complete test stdout/stderr even if this is huge.
8585
*/
86-
private boolean keepLongStdio;
86+
private String stdioRetention;
8787

8888
private boolean keepProperties;
8989
/**
@@ -156,7 +156,7 @@ private static TestResult parse(@NonNull JUnitTask task, PipelineTestDetails pip
156156
String expandedTestResults, Run<?,?> run, @NonNull FilePath workspace,
157157
Launcher launcher, TaskListener listener)
158158
throws IOException, InterruptedException {
159-
return new JUnitParser(task.isKeepLongStdio(), task.isKeepProperties(), task.isAllowEmptyResults(), task.isSkipOldReports())
159+
return new JUnitParser(task.getParsedStdioRetention(), task.isKeepProperties(), task.isAllowEmptyResults(), task.isSkipOldReports())
160160
.parseResult(expandedTestResults, run, pipelineTestDetails, workspace, launcher, listener);
161161
}
162162

@@ -255,7 +255,7 @@ public static TestResultSummary parseAndSummarize(@NonNull JUnitTask task, Pipel
255255
summary = null; // see below
256256
} else {
257257
result = new TestResult(storage.load(build.getParent().getFullName(), build.getNumber())); // irrelevant
258-
summary = new JUnitParser(task.isKeepLongStdio(), task.isKeepProperties(), task.isAllowEmptyResults(), task.isSkipOldReports())
258+
summary = new JUnitParser(task.getParsedStdioRetention(), task.isKeepProperties(), task.isAllowEmptyResults(), task.isSkipOldReports())
259259
.summarizeResult(testResults, build, pipelineTestDetails, workspace, launcher, listener, storage);
260260
}
261261

@@ -382,20 +382,33 @@ public List<TestDataPublisher> getTestDataPublishers() {
382382
}
383383

384384
/**
385-
* @return the keepLongStdio.
385+
* @param keepLongStdio Whether to keep long stdio.
386+
*
387+
* @since 1.2-beta-1
386388
*/
387-
@Override
389+
@Deprecated
390+
@DataBoundSetter public final void setKeepLongStdio(boolean keepLongStdio) {
391+
this.stdioRetention = StdioRetention.fromKeepLongStdio(keepLongStdio).name();
392+
}
393+
394+
@Deprecated
388395
public boolean isKeepLongStdio() {
389-
return keepLongStdio;
396+
return StdioRetention.ALL == getParsedStdioRetention();
390397
}
391398

392399
/**
393-
* @param keepLongStdio Whether to keep long stdio.
394-
*
395-
* @since 1.2-beta-1
400+
* @return the stdioRetention
396401
*/
397-
@DataBoundSetter public final void setKeepLongStdio(boolean keepLongStdio) {
398-
this.keepLongStdio = keepLongStdio;
402+
@Override
403+
public String getStdioRetention() {
404+
return stdioRetention == null ? StdioRetention.DEFAULT.name() : stdioRetention;
405+
}
406+
407+
/**
408+
* @param stdioRetention How to keep long stdio.
409+
*/
410+
@DataBoundSetter public final void setStdioRetention(String stdioRetention) {
411+
this.stdioRetention = stdioRetention;
399412
}
400413

401414
/**
@@ -507,5 +520,26 @@ public FormValidation doCheckHealthScaleFactor(@QueryParameter double value) {
507520
(int) (100.0 - Math.max(0.0, Math.min(100.0, 5 * value)))
508521
));
509522
}
523+
524+
public ListBoxModel doFillStdioRetentionItems(@QueryParameter("stdioRetention") String value) {
525+
ListBoxModel result = new ListBoxModel();
526+
527+
StdioRetention selectedOption;
528+
try {
529+
selectedOption = StdioRetention.parse(value);
530+
} catch (IllegalArgumentException e) {
531+
selectedOption = StdioRetention.DEFAULT;
532+
}
533+
534+
for (StdioRetention option : StdioRetention.values()) {
535+
result.add(new ListBoxModel.Option(
536+
option.getDisplayName(),
537+
option.name(),
538+
option == selectedOption
539+
));
540+
}
541+
542+
return result;
543+
}
510544
}
511545
}

‎src/main/java/hudson/tasks/junit/JUnitTask.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ public interface JUnitTask {
99

1010
List<TestDataPublisher> getTestDataPublishers();
1111

12+
String getStdioRetention();
13+
14+
default StdioRetention getParsedStdioRetention() {
15+
return StdioRetention.parse(getStdioRetention());
16+
}
17+
18+
@Deprecated
1219
boolean isKeepLongStdio();
1320

1421
boolean isKeepProperties();

‎src/main/java/hudson/tasks/junit/StdioRetention.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package hudson.tasks.junit;
2+
3+
import java.util.Locale;
4+
import java.util.stream.Collectors;
5+
import java.util.stream.Stream;
6+
7+
public enum StdioRetention {
8+
9+
ALL(Messages.StdioRetention_All_DisplayName()),
10+
FAILED(Messages.StdioRetention_Failed_DisplayName()),
11+
NONE(Messages.StdioRetention_None_DisplayName());
12+
13+
private final String displayName;
14+
15+
StdioRetention(String displayName) {
16+
this.displayName = displayName;
17+
}
18+
19+
public String getDisplayName() {
20+
return displayName;
21+
}
22+
23+
public static final StdioRetention DEFAULT = NONE;
24+
25+
public static StdioRetention fromKeepLongStdio(boolean value) {
26+
return value ? ALL : NONE;
27+
}
28+
29+
public static StdioRetention parse(String value) {
30+
if (value == null || value.isEmpty()) {
31+
return DEFAULT;
32+
}
33+
try {
34+
return valueOf(value.toUpperCase(Locale.ROOT));
35+
} catch (IllegalArgumentException e) {
36+
throw new IllegalArgumentException(
37+
"Unrecognized value '" + value + "'; must be one of the following: "
38+
+ Stream.of(values()).map(Enum::name).collect(Collectors.joining(", ")),
39+
e
40+
);
41+
}
42+
}
43+
44+
}

0 commit comments

Comments
 (0)