AssertJ skips `is`

[timo-abele]

How are you running OpenRewrite?

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

    <profile>
        <id>openrewrite</id>
        <build>
            <plugins>
                <plugin>
                    <groupId>org.openrewrite.maven</groupId>
                    <artifactId>rewrite-maven-plugin</artifactId>
                    <version>5.23.1</version>
                    <configuration>
                        <activeRecipes>
                            <recipe>org.openrewrite.java.testing.assertj.Assertj</recipe>
                        </activeRecipes>
                        <failOnDryRunResults>true</failOnDryRunResults>
                    </configuration>
                    <dependencies>
                        <dependency>
                            <groupId>org.openrewrite.recipe</groupId>
                            <artifactId>rewrite-testing-frameworks</artifactId>
                            <version>2.4.1</version>
                        </dependency>
                    </dependencies>
                </plugin>
            </plugins>
        </build>
    </profile>

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

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import org.junit.jupiter.api.Test;

class DebugTest {

    class Foo {
        int i = 8;
    }

    @Test
    void ba() {
        assertThat(System.out, is(System.out));
        assertThat(new Foo(), is(new Foo()));
    }

}

What did you expect to see?

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import org.junit.jupiter.api.Test;

class DebugTest {

    class Foo {
        int i = 8;
    }

    @Test
    void ba() {
        assertThat(System.out).isEqualTo(System.out);
        assertThat(new Foo()).isEqualTo(new Foo());
    }

}

What did you see instead?

no change

Comments

I'm a bit surprised that this seemingly simple case is not covered. I had added a custom class suspecting that this was the issue but it fails even for jdk classes. The same execution that skipped the case listed above, has successfully converted assertThats with custom types in other test classes... Update: some instances of is on primitives are skipped as well while others are converted. Is it about the imports?

[timtebeek]

Hmm; I'd have expected this to work through the recipe that we have for that migration https://github.com/openrewrite/rewrite-testing-frameworks/blob/7fb2c9d4e8158df1649e816d8e2ccd18db775e2e/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestIsMatcherToAssertJTest.java#L39-L71

Not quite sure what's different in your case, especially as it's one of the first recipes we created and call.

https://github.com/openrewrite/rewrite-testing-frameworks/blob/7fb2c9d4e8158df1649e816d8e2ccd18db775e2e/src/main/resources/META-INF/rewrite/hamcrest.yml#L41-L46

Are you able to see any differences between your usage and what's implemented and tested? Perhaps a new unit test that highlights the issue could help.

[timo-abele]

I think it's the import. They take the is matcher from import static org.hamcrest.core.Is.is; which so far no other test seems to do. The project I noticed this in is using hamcrest 1.3 by the way, maybe the matcher used to be only in this package and is still there for compatibility?

[timtebeek]

Ah yes, thanks for pointing that out! That will indeed trip up the matcher we have here as that's looking for an is method defined on any of the *Matchers classes.

https://github.com/openrewrite/rewrite-testing-frameworks/blob/bcb4449f913f43ca2820ce18214d4399f7617833/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestIsMatcherToAssertJ.java#L42

I think a quick Change method target to static to move from org.hamcrest.core.Is.is to org.hamcrest.Matchers.is executed before any other recipes run should be enough to fix this issue.