Skip to content

Migrate tests to JUnit5 #420

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

Open
strangelookingnerd wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Migrate tests to JUnit5 #420

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eb1d805
Migrate tests to JUnit5
strangelookingnerd Jul 28, 2025
ba95370
Update parent and remove jth version
strangelookingnerd Aug 8, 2025
9cd17b9
Merge branch 'master' into migrate_to_junit5
strangelookingnerd Aug 8, 2025
8ea08ed
Merge branch 'master' into migrate_to_junit5
strangelookingnerd Aug 13, 2025
7c76883
Merge branch 'master' into migrate_to_junit5
strangelookingnerd Aug 25, 2025
6d02dfe
Bump org.jenkins-ci.plugins:plugin from 5.24 to 5.26
dependabot[bot] Sep 15, 2025
67badae
Bump org.awaitility:awaitility from 4.2.2 to 4.3.0
dependabot[bot] Feb 24, 2025
8a9a4dd
Disallow soft references in MemoryAssert-related tests in FlowExecuti…
dwnusbaum Sep 30, 2025
cf1feb6
Merge branch 'master' into migrate_to_junit5
strangelookingnerd Oct 2, 2025
3a48717
Merge branch 'master' into migrate_to_junit5
strangelookingnerd Oct 23, 2025
2617bcf
Change LogStorageTestBase class to public
strangelookingnerd Oct 23, 2025
26b795f
Change ArtifactManagerTest class to public
strangelookingnerd Oct 24, 2025
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
52 changes: 31 additions & 21 deletions src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import hudson.AbortException;
import hudson.EnvVars;
Expand All @@ -52,6 +52,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -66,19 +67,27 @@
import jenkins.util.VirtualFile;
import org.apache.commons.io.IOUtils;
import org.jenkinsci.plugins.workflow.flow.StashManager;
import org.junit.Rule;
import org.junit.Test;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.LogRecorder;
import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
import test.ssh_agent.OutboundAgent;

/**
* {@link #artifactArchiveAndDelete} and variants allow an implementation of {@link ArtifactManager} plus {@link VirtualFile} to be run through a standard gantlet of tests.
*/
public class ArtifactManagerTest {
@WithJenkins
class ArtifactManagerTest {
Copy link
Contributor Author

@strangelookingnerd strangelookingnerd Oct 24, 2025

Choose a reason for hiding this comment

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

I'll look into it. On a side-note: Is there a clever way for me to find these kind of dependencies or to you just happen to know them?

Copy link
Member

@jglick jglick Oct 24, 2025

Choose a reason for hiding this comment

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

In this case I wrote the class and some of its subclasses so I just happened to remember that this was being used as a test utility, not only a suite here. And I have had to make matching changes in the past for various reasons, like migrating away from docker-fixtures.

It is not immediately apparently that just restoring public would suffice; you would need to try updating at least one subclass to use the Jupiter version and verify that it still runs as a test (and, of course, passes).

Copy link
Member

@jglick jglick Oct 24, 2025

Choose a reason for hiding this comment

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

https://github.com/search?type=code&q=org%3Ajenkinsci+NOT+is%3Aarchived+ArtifactManagerTest FTR (I use a Firefox bookmarklet for this sort of search).


@Rule public JenkinsRule r = new JenkinsRule();
@Rule public LoggerRule logging = new LoggerRule();
private final LogRecorder logging = new LogRecorder();

private JenkinsRule r;

@BeforeEach
void setUp(JenkinsRule rule) {
r = rule;
}

/**
* Creates an agent, in a Docker container when possible, calls {@link #setUpWorkspace}, then runs some tests.
Expand Down Expand Up @@ -180,7 +189,7 @@ private static void setUpWorkspace(FilePath workspace, boolean weirdCharacters)
}
private static class FindEncoding extends MasterToSlaveCallable<String, Exception> {
@Override public String call() {
return System.getProperty("file.encoding") + " vs. " + System.getProperty("sun.jnu.encoding");
return Charset.defaultCharset().displayName() + " vs. " + System.getProperty("sun.jnu.encoding");
}
}

Expand Down Expand Up @@ -349,9 +358,9 @@ private void test() throws Exception {
*/
private void assertFile(VirtualFile f, String contents) throws Exception {
System.err.println("Asserting file: " + f);
assertTrue("Not a file: " + f, f.isFile());
assertFalse("Unexpected directory: " + f, f.isDirectory());
assertTrue("Does not exist: " + f, f.exists());
assertTrue(f.isFile(), "Not a file: " + f);
assertFalse(f.isDirectory(), "Unexpected directory: " + f);
assertTrue(f.exists(), "Does not exist: " + f);
assertEquals(contents.length(), f.length());
assertThat(f.lastModified(), not(is(0)));
try (InputStream is = f.open()) {
Expand Down Expand Up @@ -381,9 +390,9 @@ private static final class RemoteOpenURL extends MasterToSlaveCallable<String, I
*/
private static void assertDir(VirtualFile f) throws IOException {
System.err.println("Asserting dir: " + f);
assertFalse("Unexpected file: " + f, f.isFile());
assertTrue("Not a directory: " + f, f.isDirectory());
assertTrue("Does not exist: " + f, f.exists());
assertFalse(f.isFile(), "Unexpected file: " + f);
assertTrue(f.isDirectory(), "Not a directory: " + f);
assertTrue(f.exists(), "Does not exist: " + f);
// length & lastModified may or may not be defined
}

Expand All @@ -392,9 +401,9 @@ private static void assertDir(VirtualFile f) throws IOException {
*/
private static void assertNonexistent(VirtualFile f) throws IOException {
System.err.println("Asserting nonexistent: " + f);
assertFalse("Unexpected file: " + f, f.isFile());
assertFalse("Unexpected dir: " + f, f.isDirectory());
assertFalse("Unexpectedly exists: " + f, f.exists());
assertFalse(f.isFile(), "Unexpected file: " + f);
assertFalse(f.isDirectory(), "Unexpected dir: " + f);
assertFalse(f.exists(), "Unexpectedly exists: " + f);
try {
assertEquals(0, f.length());
} catch (IOException x) {
Expand All @@ -408,7 +417,8 @@ private static void assertNonexistent(VirtualFile f) throws IOException {
}

/** Run the standard one, as a control. */
@Test public void standard() throws Exception {
@Test
void standard() throws Exception {
logging.record(StandardArtifactManager.class, Level.FINE);
// Who knows about weird characters on NTFS; also case-sensitivity could confuse things
artifactArchiveAndDelete(r, null, !Functions.isWindows());
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/jenkinsci/plugins/workflow/DirectArtifactManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public DirectArtifactManagerFactory() throws Exception {
HttpServer server = ServerBootstrap.bootstrap().
registerHandler("*", (HttpRequest request, HttpResponse response, HttpContext _context) -> {
String method = request.getRequestLine().getMethod();
String contents = URLDecoder.decode(request.getRequestLine().getUri().substring(1), "UTF-8");
String contents = URLDecoder.decode(request.getRequestLine().getUri().substring(1), StandardCharsets.UTF_8);
switch (method) {
case "GET": {
response.setStatusCode(200);
Expand Down Expand Up @@ -183,7 +183,7 @@ private static final class NoOpenVF extends VirtualFile {
try (InputStream is = delegate.open()) {
contents = IOUtils.toString(is, StandardCharsets.UTF_8);
}
return new URL(null, baseURL + URLEncoder.encode(contents, "UTF-8"), new URLStreamHandler() {
return new URL(null, baseURL + URLEncoder.encode(contents, StandardCharsets.UTF_8), new URLStreamHandler() {
@Override protected URLConnection openConnection(URL u) throws IOException {
throw new IOException("not allowed to open " + u + " from this JVM");
}
Expand Down
89 changes: 52 additions & 37 deletions src/test/java/org/jenkinsci/plugins/workflow/actions/ErrorActionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
package org.jenkinsci.plugins.workflow.actions;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -39,12 +39,9 @@
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.BuildWatcher;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsSessionRule;

import groovy.lang.MissingMethodException;
import hudson.FilePath;
Expand All @@ -65,25 +62,28 @@
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
import org.jenkinsci.plugins.workflow.steps.StepExecution;
import org.jenkinsci.plugins.workflow.steps.StepExecutions;
import org.jvnet.hudson.test.InboundAgentRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.LogRecorder;
import org.jvnet.hudson.test.TestExtension;
import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension;
import org.jvnet.hudson.test.junit.jupiter.InboundAgentExtension;
import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension;
import org.kohsuke.stapler.DataBoundConstructor;

/**
* Tests for {@link ErrorAction}
*/
public class ErrorActionTest {
class ErrorActionTest {

@ClassRule
public static BuildWatcher buildWatcher = new BuildWatcher();
@RegisterExtension
private static final BuildWatcherExtension buildWatcher = new BuildWatcherExtension();

@Rule
public JenkinsSessionRule rr = new JenkinsSessionRule();
@RegisterExtension
private final JenkinsSessionExtension rr = new JenkinsSessionExtension();

@Rule public InboundAgentRule agents = new InboundAgentRule();
@RegisterExtension
private final InboundAgentExtension agents = new InboundAgentExtension();

@Rule public LoggerRule logging = new LoggerRule().record(ErrorAction.class, Level.FINE);
private final LogRecorder logging = new LogRecorder().record(ErrorAction.class, Level.FINE);

private List<ErrorAction> extractErrorActions(FlowExecution exec) {
List<ErrorAction> ret = new ArrayList<>();
Expand All @@ -99,14 +99,15 @@ private List<ErrorAction> extractErrorActions(FlowExecution exec) {
}

@Test
public void simpleException() throws Throwable {
void simpleException() throws Throwable {
rr.then(r -> {
final String EXPECTED = "For testing purpose";
WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p");
job.setDefinition(new CpsFlowDefinition(String.format(
"node {\n"
+ "throw new Exception('%s');\n"
+ "}"
"""
node {
throw new Exception('%s');
}"""
, EXPECTED
), true));
WorkflowRun b = r.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get());
Expand All @@ -121,16 +122,17 @@ public void simpleException() throws Throwable {

@Issue("JENKINS-34488")
@Test
public void unserializableForSecurityReason() throws Throwable {
void unserializableForSecurityReason() throws Throwable {
rr.then(r -> {
final String FAILING_EXPRESSION = "(2 + 2) == 5";
WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "p");
// "assert false" throws org.codehaus.groovy.runtime.powerassert.PowerAssertionError,
// which is rejected by remoting.
job.setDefinition(new CpsFlowDefinition(String.format(
"node {\n"
+ "assert %s;\n"
+ "}",
"""
node {
assert %s;
}""",
FAILING_EXPRESSION
), true));
WorkflowRun b = r.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get());
Expand All @@ -144,7 +146,8 @@ public void unserializableForSecurityReason() throws Throwable {
}

@Issue("JENKINS-39346")
@Test public void wrappedUnserializableException() throws Throwable {
@Test
void wrappedUnserializableException() throws Throwable {
rr.then(r -> {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
Expand All @@ -167,7 +170,8 @@ public void unserializableForSecurityReason() throws Throwable {
}

@Issue("JENKINS-49025")
@Test public void nestedFieldUnserializable() throws Throwable {
@Test
void nestedFieldUnserializable() throws Throwable {
rr.then(r -> {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
Expand All @@ -189,21 +193,25 @@ public static class X extends Exception {
final NullObject nil = NullObject.getNullObject();
}

@Test public void userDefinedError() throws Throwable {
@Test
void userDefinedError() throws Throwable {
rr.then(r -> {
WorkflowJob p = r.createProject(WorkflowJob.class);
p.setDefinition(new CpsFlowDefinition(
"class MyException extends Exception {\n" +
" MyException(String message) { super(message) }\n" +
"}\n" +
"throw new MyException('test')\n",
"""
class MyException extends Exception {
MyException(String message) { super(message) }
}
throw new MyException('test')
""",
true));
WorkflowRun b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0));
assertThat(b.getExecution().getCauseOfFailure(), Matchers.instanceOf(ProxyException.class));
});
}

@Test public void missingPropertyExceptionMemoryLeak() throws Throwable {
@Test
void missingPropertyExceptionMemoryLeak() throws Throwable {
rr.then(r -> {
WorkflowJob p = r.createProject(WorkflowJob.class);
p.setDefinition(new CpsFlowDefinition("FOO", false));
Expand All @@ -212,7 +220,8 @@ public static class X extends Exception {
});
}

@Test public void findOriginOfAsyncErrorAcrossRestart() throws Throwable {
@Test
void findOriginOfAsyncErrorAcrossRestart() throws Throwable {
String name = "restart";
AtomicReference<String> origin = new AtomicReference<>();
rr.then(r -> {
Expand All @@ -232,7 +241,8 @@ public static class X extends Exception {
});
}

@Test public void findOriginOfSyncErrorAcrossRestart() throws Throwable {
@Test
void findOriginOfSyncErrorAcrossRestart() throws Throwable {
String name = "restart";
AtomicReference<String> origin = new AtomicReference<>();
rr.then(r -> {
Expand All @@ -251,7 +261,8 @@ public static class X extends Exception {
});
}

@Test public void findOriginFromBodyExecutionCallback() throws Throwable {
@Test
void findOriginFromBodyExecutionCallback() throws Throwable {
rr.then(r -> {
agents.createAgent(r, "remote");
var p = r.createProject(WorkflowJob.class);
Expand All @@ -263,6 +274,7 @@ public static class X extends Exception {
r.assertLogContains("Found in: fails", b);
});
}

public static final class WrapperStep extends Step {
@DataBoundConstructor public WrapperStep() {}
@Override public StepExecution start(StepContext context) throws Exception {
Expand Down Expand Up @@ -305,6 +317,7 @@ public void onFailure(StepContext context, Throwable t) {
}
}
}

public static final class FailingStep extends Step {
@DataBoundConstructor public FailingStep() {}
@Override public StepExecution start(StepContext context) throws Exception {
Expand Down Expand Up @@ -332,15 +345,17 @@ private static final class Sleep extends MasterToSlaveFileCallable<Void> {
}
}

@Test public void cyclicErrorsAreSupported() throws Throwable {
@Test
void cyclicErrorsAreSupported() throws Throwable {
Exception cyclic1 = new Exception();
Exception cyclic2 = new Exception(cyclic1);
cyclic1.initCause(cyclic2);
assertNotNull(new ErrorAction(cyclic1));
assertNotNull(new ErrorAction(cyclic2));
}

@Test public void unserializableCyclicErrorsAreSupported() throws Throwable {
@Test
void unserializableCyclicErrorsAreSupported() throws Throwable {
Exception unserializable = new MissingMethodException("thisMethodDoesNotExist", String.class, new Object[0]);
Exception cyclic = new Exception(unserializable);
unserializable.initCause(cyclic);
Expand Down
Loading
Loading