openrewrite / openrewrite/rewrite-testing-frameworks

`ParameterizedRunnerToParameterized` silently skips classes whose `@Parameters` method is inherited, leaving a partially migrated class that

Open
#1,126 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

ParameterizedRunnerToParameterized migrates a @RunWith(Parameterized.class) class only when
the @Parameters-annotated factory method is declared in that same class. When the factory is
inherited from a superclass — a very common way to share a parameter set across a family of test
classes — the recipe makes no change at all and reports no warning.

Minimal reproduction

Three files. SameClassTest declares its own @Parameters; InheritedTest gets it from
ParamBase. Everything else about the two is identical.

ParamBase.java

package com.example;

import java.util.Arrays;
import java.util.Collection;
import org.junit.runners.Parameterized.Parameters;

public class ParamBase {
    protected final int value;

    @Parameters(name = "{0}")
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{1}, {2}});
    }

    protected ParamBase(int value) {
        this.value = value;
    }
}

InheritedTest.java

package com.example;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import static org.junit.Assert.assertTrue;

@RunWith(Parameterized.class)
public class InheritedTest extends ParamBase {

    public InheritedTest(int value) {
        super(value);
    }

    @Test
    public void isPositive() {
        assertTrue(value > 0);
    }
}

SameClassTest.java — the control, with the factory declared locally

package com.example;

import java.util.Arrays;
import java.util.Collection;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import static org.junit.Assert.assertTrue;

@RunWith(Parameterized.class)
public class SameClassTest {
    private final int value;

    @Parameters(name = "{0}")
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{1}, {2}});
    }

    public SameClassTest(int value) {
        this.value = value;
    }

    @Test
    public void isPositive() {
        assertTrue(value > 0);
    }
}

Run org.openrewrite.java.testing.junit5.JUnit4to5Migration.

SameClassTest — migrated correctly:

package com.example;

import java.util.Arrays;
import java.util.Collection;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

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

public class SameClassTest {
    private int value;

    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{1}, {2}});
    }

    public void initSameClassTest(int value) {
        this.value = value;
    }

    @MethodSource("data")
    @ParameterizedTest(name = "{0}")
    public void isPositive(int value) {
        initSameClassTest(value);
        assertTrue(value > 0);
    }
}

InheritedTest@RunWith(Parameterized.class) and both JUnit 4 imports survive:

package com.example;

import org.junit.jupiter.api.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

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

@RunWith(Parameterized.class)
public class InheritedTest extends ParamBase {

    public InheritedTest(int value) {
        super(value);
    }

    @Test
    public void isPositive() {
        assertTrue(value > 0);
    }
}
Why this matters

Because JUnit4to5Migration runs many recipes together, the skipped class doesn't stay untouched:
other sub-recipes in the same run still migrate what they can. Here, org.junit.Test became
org.junit.jupiter.api.Test and Assert.assertTrue became Assertions.assertTrue, while
@RunWith(Parameterized.class) and its org.junit.runner/org.junit.runners imports were left
behind — and the pom has had junit:junit replaced by junit-jupiter, so there is nothing on the
classpath to satisfy them:

[ERROR] InheritedTest.java:[4,24] package org.junit.runner does not exist
[ERROR] InheritedTest.java:[5,25] package org.junit.runners does not exist
[ERROR] ParamBase.java:[5,39] package org.junit.runners.Parameterized does not exist
[ERROR] InheritedTest.java:[9,2] cannot find symbol
[ERROR]   symbol: class RunWith
[ERROR] InheritedTest.java:[9,10] cannot find symbol
[ERROR]   symbol: class Parameterized

The result isn't "unmigrated," it's "partially migrated and now uncompilable" — and nothing in the
recipe run indicates the class was skipped. The failure is silent until compilation.

Why the existing tests don't cover it

In ParameterizedRunnerToParameterized.ParameterizedRunnerVisitor, the parameter metadata for a
class is collected by visitMethodDeclaration, which only ever sees methods in the compilation
unit currently being visited:

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
    J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
    Cursor classDeclCursor = getCursor().dropParentUntil(J.ClassDeclaration.class::isInstance);
    Map<String, Object> params = classDeclCursor.computeMessageIfAbsent(
            ((J.ClassDeclaration) classDeclCursor.getValue()).getId().toString(), v -> new HashMap<>());
    if (m.isConstructor()) {
        params.put(CONSTRUCTOR_ARGUMENTS, m.getParameters());
    }
    for (J.Annotation annotation : service(AnnotationService.class).getAllAnnotations(getCursor())) {
        if (PARAMETERS.matches(annotation)) {
            params.put(PARAMETERS_ANNOTATION_ARGUMENTS, annotation.getArguments());
            params.put(PARAMETERS_METHOD_NAME, method.getSimpleName());
            break;
        }
        ...
    }
    return m;
}

visitClassDeclaration then gates both of its transformation branches on that value being present:

String parametersMethodName = (String) params.get(PARAMETERS_METHOD_NAME);
...
// Constructor Injected Test
if (parametersMethodName != null && constructorParams != null && constructorParams.stream()
        .anyMatch(J.VariableDeclarations.class::isInstance)) {
    doAfterVisit(new ParameterizedRunnerToParameterizedTestsVisitor(...));
}
// Field Injected Test
else if (parametersMethodName != null && fieldInjectionParams != null) {
    doAfterVisit(new ParameterizedRunnerToParameterizedTestsVisitor(...));
}

For InheritedTest there is no @Parameters method in the class body, so parametersMethodName is
null, both branches are false, and visitClassDeclaration returns the class untouched. The
recipe's UsesType<>("org.junit.runners.Parameterized", false) precondition still matches — via the
@RunWith argument — so the recipe runs, it just does nothing.

The type information needed to resolve the inherited method is available: the superclass is on the
classpath, and classDecl.getType().getSupertype() exposes its members. The lookup simply isn't
attempted.

Every fixture in ParameterizedRunnerToParameterizedTest declares @Parameters in the class under
test. One fixture, NestedTests, comes close — it has classes extending an outer hierarchy — but
both still declare their own local @Parameters method; only the constructor and test method are
inherited, not the factory. There is no test in which the factory method itself comes from a
supertype, so the parametersMethodName == null path is never exercised against a class that still
carries @RunWith(Parameterized.class).

At minimum, the recipe should not leave a class in a state that doesn't compile. Two options: walk
JavaType.FullyQualified.getSupertype() for a static method annotated with @Parameters and use it
as parametersMethodName@MethodSource accepts a fully-qualified #-separated reference, so an
inherited factory can be named directly as @MethodSource("com.example.ParamBase#data") — or, if
resolving it is out of scope, fail loudly instead of silently: emit a search marker or leave the
JUnit 4 dependency in place, so the situation is visible rather than surfacing later as a compile
error.

Environment
  • rewrite-testing-frameworks 3.44.0 (also present in 3.46.0-SNAPSHOT)
  • rewrite-maven-plugin 6.41.0
  • Recipe: org.openrewrite.java.testing.junit5.JUnit4to5Migration

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 ParameterizedRunnerToParameterized.ParameterizedRunnerVisitor, especially visitMethodDeclaration and visitClassDeclaration, to trace how parameter metadata is collected and used. Add an inherited-factory fixture to ParameterizedRunnerToParameterizedTest based on ParamBase and InheritedTest, then run the migration tests. Done means the inherited case is handled or reported visibly and no longer leaves an uncompilable partial migration.

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
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.