openrewrite / openrewrite/rewrite-testing-frameworks
JUnit 4 to 5 migration discards the one-time setup a `TestSetup` decorator performed, breaking tests at run time
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 100
- Forks
- 105
- Avg merge
- 2h 25m
- Merged PRs (30d)
- 9
Description
What happens
JUnit4to5Migration removes any static suite() method returning junit.framework.Test. When that
method returned a junit.extensions.TestSetup decorator, the removal also discards the class's
one-time setup and teardown, and no JUnit 5 lifecycle annotation takes their place.
Minimal reproduction
Before:
import junit.extensions.TestSetup;
import junit.framework.Test;
import junit.framework.TestCase;
import junit.framework.TestSuite;
public class MathTest extends TestCase {
private static String resource;
public static Test suite() {
return new TestSetup(new TestSuite(MathTest.class)) {
@Override
protected void setUp() throws Exception {
setUpOnce();
}
@Override
protected void tearDown() throws Exception {
tearDownOnce();
}
};
}
static void setUpOnce() throws Exception {
resource = "open";
}
static void tearDownOnce() throws Exception {
resource = null;
}
public void testResourceIsOpen() {
assertEquals("open", resource);
}
}
After org.openrewrite.java.testing.junit5.JUnit4to5Migration:
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class MathTest {
private static String resource;
static void setUpOnce() throws Exception {
resource = "open";
}
static void tearDownOnce() throws Exception {
resource = null;
}
@Test
public void testResourceIsOpen() {
assertEquals("open", resource);
}
}
Everything else is migrated correctly — superclass removed, @Test added, assertEquals
static-imported, JUnit 3 imports cleaned up. Only the fixture is wrong: setUpOnce and tearDownOnce
survive with no caller, so resource stays null and testResourceIsOpen fails.
The loss is larger when the decorator holds its statements inline rather than delegating to a named
method.
Before:
import junit.extensions.TestSetup;
import junit.framework.Test;
import junit.framework.TestCase;
import junit.framework.TestSuite;
public class MathTest extends TestCase {
private static String resource;
private static int counter;
public static Test suite() {
return new TestSetup(new TestSuite(MathTest.class)) {
@Override
protected void setUp() throws Exception {
resource = "open";
counter = 1;
System.setProperty("mode", "test");
}
@Override
protected void tearDown() throws Exception {
resource = null;
System.clearProperty("mode");
}
};
}
public void testResourceIsOpen() {
assertEquals("open", resource);
assertEquals(1, counter);
}
}
After:
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class MathTest {
private static String resource;
private static int counter;
@Test
public void testResourceIsOpen() {
assertEquals("open", resource);
assertEquals(1, counter);
}
}
All five statements are gone, and unlike the first reproduction nothing is left behind to show that
the class ever had a fixture.
Why this matters
This is a silent behaviour change rather than a build break, so it is harder to catch than a compile
error. The output compiles, and CleanupJUnitImports removes the now-unused
junit.extensions.TestSetup import, so the diff reads as an ordinary tidy-up: JUnit 3 imports gone,
extends TestCase gone, suite() gone, @Test added. Nothing indicates that a fixture was dropped.
The failure appears only when the tests run, and some distance from the change. In a real codebase the
discarded statement is usually a client or connection, so what surfaces is a NullPointerException in
the test body:
java.lang.NullPointerException: Cannot invoke
"AlertServiceClient.getAlertsForCoverage(int)" because "AlertServiceTest._client" is null
The two shapes differ in how much evidence they leave behind:
| Decorator body | After migration |
|---|---|
calls a named method, setUpOnce() |
method survives, now uncalled — some evidence remains |
| holds statements inline | statements deleted — no evidence at all |
TestSetup has an exact JUnit 5 counterpart. TestSetup.setUp runs once before the wrapped suite and
TestSetup.tearDown once after, which is @BeforeAll and @AfterAll, so the information needed to
migrate it is present in the source being removed.
Why the existing handling doesn't cover it
MigrateJUnitTestCase removes suite() on the strength of its signature alone, without looking at
what it returns:
if ("suite".equals(md.getSimpleName()) &&
md.hasModifier(J.Modifier.Type.Static) &&
md.getMethodType() != null &&
(TypeUtils.isOfClassType(md.getMethodType().getReturnType(), "junit.framework.Test") ||
TypeUtils.isOfClassType(md.getMethodType().getReturnType(), "junit.framework.TestSuite"))) {
maybeRemoveImport("junit.framework.Test");
maybeRemoveImport("junit.framework.TestSuite");
return null;
}
That is correct for a plain new TestSuite(MathTest.class), which only enumerates tests JUnit 5
discovers by itself, and MigrateJUnitTestCaseTest.suiteMethodIsRemoved covers exactly that case:
public static Test suite() {
return new TestSuite(AppTest.class);
}
There is nothing behind that suite() to lose, so removal is right and the test passes. TestSetup
appears nowhere in the test sources, and a decorator is the one case where suite() carries behaviour
rather than just listing tests.
A fix cannot simply keep suite(): JUnit 5 has no equivalent, and the method would not compile once
junit.framework.Test is gone. The decorator's setUp and tearDown bodies would need to move onto
the class as @BeforeAll and @AfterAll. Where the body is a single call to an existing static
method, annotating that method is enough; where it holds statements inline, they need a generated
static method. Both annotations require a static method unless the class is
@TestInstance(PER_CLASS), which matches TestSetup's own semantics.
Environment
- rewrite-testing-frameworks
3.45.0 - Recipe:
org.openrewrite.java.testing.junit5.JUnit4to5Migration
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in MigrateJUnitTestCase, where suite() is removed based only on its return type, and compare it with MigrateJUnitTestCaseTest.suiteMethodIsRemoved. Add focused coverage for TestSetup decorators with delegated and inline lifecycle bodies, then verify the migration preserves the one-time setup and teardown semantics through JUnit 5 lifecycle annotations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100