openrewrite / rewrite-testing-frameworks

OpenRewrite recipes that perform common Java testing migration tasks.
Apache License 2.0
68 stars 58 forks source link

Migrating from Fest 2.x to AssertJ to avoid ambiguous references to `assertThat` #463

Open timo-abele opened 5 months ago

timo-abele commented 5 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

<plugin>
    <groupId>org.openrewrite.maven</groupId>
    <artifactId>rewrite-maven-plugin</artifactId>
    <version>5.18.0</version>
    <configuration>
        <activeRecipes>
            <recipe>org.openrewrite.java.testing.assertj.Assertj</recipe>
        </activeRecipes>
    </configuration>
    <dependencies>
        <dependency>
            <groupId>org.openrewrite.recipe</groupId>
            <artifactId>rewrite-testing-frameworks</artifactId>
            <version>2.2.0</version>
        </dependency>
    </dependencies>
</plugin>

What is the smallest, simplest way to reproduce the problem?

import static org.fest.assertions.Assertions.assertThat;
import org.assertj.core.api.Assertions;

class A {
    @Test
    void foo() {
        Assertions.assertThat(1 + 1).isEqualTo(2); //assertJ
        assertThat(1+1).isEqualTo(2);    //fest
    }
}

What did you expect to see?

import static org.assertj.core.api.Assertions.assertThat;

class A {
    @Test
    void foo() {
        assertThat(1 + 1).isEqualTo(2);
        assertThat(1+1).isEqualTo(2);
    }
}

What did you see instead?

import static org.assertj.core.api.Assertions.assertThat;
import static org.fest.assertions.Assertions.assertThat;

class A {
    @Test
    void foo() {
        assertThat(1 + 1).isEqualTo(2);
        assertThat(1+1).isEqualTo(2);
    }
}

What is the full stack trace of any errors you encountered?

Project won't compile, job testCompile says

reference to assertThat is ambiguous

Are you interested in contributing a fix to OpenRewrite?

No

timtebeek commented 2 months ago

Thanks for pointing this out! Wasn't aware of Fest, and understand how that leads to ambiguity here. It seems the AssertJ recipes that we have are hardwired to use static imports, as for instance seen here. https://github.com/openrewrite/rewrite-testing-frameworks/blob/368384a174bd92d49880c19a879fb9e14afcfa27/src/main/java/org/openrewrite/java/testing/assertj/JUnitAssertEqualsToAssertThat.java#L69-L74

Looks like we can first migrate Fest to AssertJ as seen here: https://joel-costigliola.github.io/assertj/assertj-core-migrating-from-fest.html This would be very similar to what we already do here. https://github.com/openrewrite/rewrite-testing-frameworks/blob/964e677739350ce16cc0b4ee55fb47d152a2baf0/src/main/resources/META-INF/rewrite/assertj.yml#L32-L46

Should be fairly straightforward especially for the 2.x version of Fest. I'd expect no more than a handful of Yaml recipes to get the bulk of the usages migrated, which would then also avoid these conflicts here.