openrewrite / openrewrite/rewrite-testing-frameworks

A retained `TestCase(String)` constructor leaves the migrated test class unrunnable under JUnit 5

Open
#1,108 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Java
Stars
100
Forks
105
Avg merge
2h 25m
Merged PRs (30d)
9

Description

What happens

When a JUnit 3 test class has a TestCase(String name) constructor whose body contains anything
besides super(name), MigrateJUnitTestCase removes the super(name) call and keeps the
constructor. The class compiles, so the migration looks successful, but JUnit 5 can no longer
instantiate it and every test in the class errors at run time.

Minimal reproduction

Before:

import junit.framework.TestCase;

public class MathTest extends TestCase {
    private boolean initialized;

    public MathTest(String testName) {
        super(testName);
        initialized = true;
    }

    public void testAdd() {
        assertTrue(initialized);
    }
}

After org.openrewrite.java.testing.junit5.MigrateJUnitTestCase:

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertTrue;

public class MathTest {
    private boolean initialized;

    public MathTest(String testName) {
        initialized = true;
    }

    @Test
    public void testAdd() {
        assertTrue(initialized);
    }
}

Everything else is migrated correctly. The class compiles. Running it gives:

org.junit.jupiter.api.extension.ParameterResolutionException:
No ParameterResolver registered for parameter [java.lang.String arg0]
in constructor [public MathTest(java.lang.String)]
Why this matters

JUnit 5 constructs a fresh instance of the test class per test method and resolves constructor
parameters through registered ParameterResolver extensions. There is no built-in resolver for a
bare String, so instantiation fails before any test method runs and the whole class errors out.

The String parameter is not arbitrary user API. TestCase(String name) was JUnit 3's own
mechanism: the runner passed the test method name in and getName() returned it. JUnit 5 dropped
that model entirely, so once extends TestCase is gone the parameter is a leftover of a contract
that no longer exists and nothing will ever supply it.

Two properties make this worse than a compile error:

  • It is silent. The build is green. The failure only appears when the tests are executed, and in
    a large migration that can be a long way from the change.
  • Real setup is stranded. initialized = true above — and in practice things like a
    BaseInit.init() call — used to run once per test instance. It is now in a constructor that is
    never invoked. Simply deleting the parameter to make the class instantiable would drop that
    initialisation, so the statements need to move to @BeforeEach.

Whether the migration output fails at compile time or at run time depends on an incidental detail of
the original constructor body:

Original constructor body After super removal Outcome
super(name); empty, constructor deleted callers no longer compile (separate issue)
super(name); doSomething(); non-empty, constructor kept compiles, class cannot be instantiated
Why the existing tests don't cover it

MigrateJUnitTestCaseTest.constructorWithAdditionalStatementsIsKept asserts exactly this
transformation:

public class AppTest extends TestCase {
    private final String name;
    public AppTest(String testName) {
        super(testName);
        this.name = testName;
    }
}

becomes

public class AppTest {
    private final String name;
    public AppTest(String testName) {
        this.name = testName;
    }
}

Keeping user-written statements is clearly the right instinct. The gap is that the fixture has no
@Test methods, so the resulting class is never executed and the ParameterResolutionException is
not observed. Adding a test method to that same fixture is enough to expose it.

When the enclosing class was a TestCase and the retained constructor's only parameter is the
JUnit 3 test name, the migration could move the remaining statements into a @BeforeEach method and
drop the constructor, preserving both the statements and their per-test execution semantics:

@BeforeEach
public void setUp() {
    initialized = true;
}

If that is considered too invasive, leaving the constructor but reporting it would at least stop the
result looking like a clean migration.

Environment
  • rewrite-testing-frameworks 3.44.0 (tag v3.44.0); also reproduces on main
  • Recipe: org.openrewrite.java.testing.junit5.MigrateJUnitTestCase

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with MigrateJUnitTestCase and its existing MigrateJUnitTestCaseTest.constructorWithAdditionalStatementsIsKept fixture. Add an executing test method to reproduce the retained String constructor failure, then inspect the migration's constructor and setup handling. Done means the migrated class runs under JUnit 5, preserves initialization per test, and the regression test passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.