openrewrite / openrewrite/rewrite-testing-frameworks

`MigrateJUnitTestCase` removes the `TestCase(String)` constructor without updating its callers, breaking compilation

Open
#1,107 0 comments 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

MigrateJUnitTestCase deletes a TestCase(String) constructor whose body becomes empty once
super(testName) is removed, but nothing else in the compilation unit — or in any other file — is
updated. Any code that called that constructor is left pointing at something that no longer exists,
and the class fails to compile.

There are two shapes, and neither involves anything exotic.

Minimal reproduction 1 — caller in the same file

Before:

import junit.framework.TestCase;

public class MathTest extends TestCase {
    public MathTest(String testName) {
        super(testName);
    }

    public static void main(String[] args) {
        new MathTest("FOO").testAdd();
    }

    public void testAdd() {
        assertEquals(2, 1 + 1);
    }
}

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

import org.junit.jupiter.api.Test;

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

public class MathTest {

    public static void main(String[] args) {
        new MathTest("FOO").testAdd();
    }

    @Test
    public void testAdd() {
        assertEquals(2, 1 + 1);
    }
}
error: constructor MathTest in class MathTest cannot be applied to given types;
  required: no arguments
  found:    java.lang.String

The constructor was removed correctly. The call to it, six lines below in the same file, was not.

Minimal reproduction 2 — subclass calling the removed base constructor

Before:

import junit.framework.TestCase;

public abstract class BaseTest extends TestCase {
    public BaseTest(String name) {
        super(name);
    }
}
public class MathTest extends BaseTest {
    public MathTest(String name) {
        super(name);
    }

    public void testAdd() {
        assertEquals(2, 1 + 1);
    }
}

After:

public abstract class BaseTest {
}
import org.junit.jupiter.api.Test;

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

public class MathTest extends BaseTest {
    public MathTest(String name) {
        super(name);
    }

    @Test
    public void testAdd() {
        assertEquals(2, 1 + 1);
    }
}

BaseTest is migrated correctly. MathTest is untouched and now calls a constructor that no longer
exists:

error: constructor BaseTest in class BaseTest cannot be applied to given types;
Why this matters

Both shapes are hard compile breaks rather than silent behaviour changes, but that is little comfort
in a large migration: the recipe presents its output as a clean, compiling result and instead leaves
the caller side entirely untouched.

Caller shape Constructor removed? Caller updated? Outcome
Same-file new MathTest("FOO") Yes No does not compile
Subclass super(name) one level down Yes (base only) No does not compile

The removal stops at the first level of the hierarchy and never propagates past it, and it never
looks for in-file callers at all.

Why the existing tests don't cover it

TEST_CASE_SUPER_MATCHER only matches a super(...) whose resolved target is
junit.framework.TestCase itself:

private static final MethodMatcher TEST_CASE_SUPER_MATCHER =
    new MethodMatcher("junit.framework.TestCase <constructor>(..)");
  • In BaseTest, super(name) resolves to TestCase(String), so it matches, the call is removed, the
    body becomes empty, and the constructor is deleted by the check in visitMethodDeclaration.
  • In MathTest, super(name) resolves to BaseTest(String), so it does not match and nothing is
    touched.

MigrateJUnitTestCaseTest.caseWithConstructorCallingSuperTestName covers the removal, but the
fixture contains nothing that calls the constructor. convertExtendedTestCase does cover a
two-level hierarchy (CTest extends TestCase, MathTest extends CTest), but neither class declares
a constructor, so the intersection of "hierarchy" and "constructors" is untested. That intersection
is reproduction 2 above.

A fix needs to check whether the constructor is still referenced before deleting it — rewriting the
call site to the no-arg form where possible — and propagate the removal down the hierarchy, which
also requires dropping the subclass's own now-redundant super(name) call and constructor. Note the
related-but-distinct case where the constructor is kept because its body had other statements,
which leaves a class JUnit 5 cannot instantiate — filed separately.

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 in org.openrewrite.java.testing.junit5.MigrateJUnitTestCase, especially TEST_CASE_SUPER_MATCHER and visitMethodDeclaration. Read MigrateJUnitTestCaseTest.caseWithConstructorCallingSuperTestName and convertExtendedTestCase, then add coverage for same-file calls and constructors in a two-level hierarchy. Done means migrated sources compile without stale constructor calls and the regression tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
testing-qa, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.