Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
timo-abele opened this issue Jan 18, 2024 · 1 comment
Labels
assertj bug Something isn't working good first issue Good for newcomers recipe Recipe request

Comments

@timo-abele
Copy link
Contributor

timo-abele commented Jan 18, 2024

What version of OpenRewrite are you using?

I am using

  • Maven plugin v5.18.0
  • rewrite-testing-frameworks v2.2.0

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

@timo-abele timo-abele added the bug Something isn't working label Jan 18, 2024
@timtebeek timtebeek moved this to Backlog in OpenRewrite Apr 27, 2024
@timtebeek
Copy link
Contributor

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.

return JavaTemplate.builder("assertThat(#{any()}).isEqualTo(#{any()});")
.contextSensitive()
.staticImports("org.assertj.core.api.Assertions.assertThat")
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "assertj-core-3.24"))
.build()
.apply(getCursor(), method.getCoordinates().replace(), actual, expected);

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.

name: org.openrewrite.java.testing.assertj.StaticImports
displayName: Statically import AssertJ's `assertThat`
description: Consistently use a static import rather than inlining the `Assertions` class name in tests.
tags:
- testing
- assertj
recipeList:
- org.openrewrite.java.ChangeMethodTargetToStatic:
methodPattern: "org.assertj.core.api.AssertionsForClassTypes assertThat(..)"
fullyQualifiedTargetTypeName: "org.assertj.core.api.Assertions"
- org.openrewrite.java.ChangeMethodTargetToStatic:
methodPattern: "org.assertj.core.api.AssertionsForInterfaceTypes assertThat(..)"
fullyQualifiedTargetTypeName: "org.assertj.core.api.Assertions"
- org.openrewrite.java.UseStaticImport:
methodPattern: "org.assertj.core.api.Assertions *(..)"

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.

@timtebeek timtebeek added good first issue Good for newcomers recipe Recipe request labels Apr 27, 2024
@timtebeek timtebeek changed the title assertj.Assertj does not remove org.fest assertThat import leading to "reference to assertThat is ambiguous" Migrating from Fest 2.x to AssertJ to avoid ambiguous references to assertThat Apr 27, 2024
@timtebeek timtebeek moved this from Backlog to Recipes Wanted in OpenRewrite Apr 27, 2024
@Laurens-W Laurens-W self-assigned this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assertj bug Something isn't working good first issue Good for newcomers recipe Recipe request
Projects
Status: Recipes Wanted
Development

No branches or pull requests

3 participants