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

JUnit5's assertInstanceOf is not converted to AssertJ equivalent #607

Open
timo-abele opened this issue Sep 21, 2024 · 2 comments
Open

JUnit5's assertInstanceOf is not converted to AssertJ equivalent #607

timo-abele opened this issue Sep 21, 2024 · 2 comments
Labels
assertj bug Something isn't working

Comments

@timo-abele
Copy link
Contributor

timo-abele commented Sep 21, 2024

What version of OpenRewrite are you using?

I am using the latest version as of 2024-09-21

How are you running OpenRewrite?

originally gradle build file, also verified via PR:

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

full example: https://github.com/openrewrite/rewrite-testing-frameworks/pull/606/files#diff-bfebe6e8e82e88df037959cc55562fae9a10b6b050dd49036be913fd07f93295R45-R70

assertInstanceOf(Integer.class, 4);

What did you expect to see?

assertThat(4).isInstanceOf(Integer.class);

What did you see instead?

no change

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

https://github.com/openrewrite/rewrite-testing-frameworks/actions/runs/10971347200/job/30466378864?pr=606#step:5:1500

AssertJBestPracticesTest > convertsIsInstanceOf() FAILED
    java.lang.AssertionError: Recipe was expected to make a change but made no changes.
        at org.openrewrite.test.LargeSourceSetCheckingExpectedCycles.afterCycle(LargeSourceSetCheckingExpectedCycles.java:119)
        at org.openrewrite.RecipeScheduler.runRecipeCycles(RecipeScheduler.java:98)
        at org.openrewrite.RecipeScheduler.scheduleRun(RecipeScheduler.java:41)
        at org.openrewrite.Recipe.run(Recipe.java:376)
        at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:371)
        at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:130)
        at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:125)
        at org.openrewrite.java.testing.assertj.AssertJBestPracticesTest.convertsIsInstanceOf(AssertJBestPracticesTest.java:48)

Are you interested in contributing a fix to OpenRewrite?

No

@timo-abele timo-abele added the bug Something isn't working label Sep 21, 2024
@timtebeek timtebeek moved this to Backlog in OpenRewrite Sep 21, 2024
@timtebeek timtebeek added the good first issue Good for newcomers label Sep 21, 2024
@Paulitos
Copy link

Any hint @timtebeek? I saw the pull request from @timo-abele but seems like it didn't work.

@timtebeek timtebeek removed the good first issue Good for newcomers label Sep 21, 2024
@timtebeek
Copy link
Contributor

hi @Paulitos ! Thanks for the offer to help; there's (at least) one of two ways we could fix this issue.

Refaster style recipe

The best would probably be to extend rewrite-templating to support this particular style of Refaster rule:
https://github.com/PicnicSupermarket/error-prone-support/blob/6a13efded8f5c64e3b0f09d89e36ece02de36e6d/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java#L732-L743

  static final class AssertThatIsInstanceOf<T> {
    @BeforeTemplate
    void before(Object actual, Class<T> clazz) {
      assertInstanceOf(clazz, actual);
    }

    @AfterTemplate
    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
    void after(Object actual, Class<T> clazz) {
      assertThat(actual).isInstanceOf(clazz);
    }
  }

We already include the refaster rules generated there in our migration from JUnit to AssertJ:

- tech.picnic.errorprone.refasterrules.JUnitToAssertJRulesRecipes

This particular recipe is not converted because of a limitation to handling type parameters in rewrite-templating
https://github.com/openrewrite/rewrite-templating/blob/44e803358b1ee3c08562a8a497aa1f1fd9cb90e8/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java#L729-L732

I haven't yet explored what would be needed for a fix there; I suppose it's more involved and @knutwannheden might have some hints.

Imperative recipe support

If expanding Refaster style recipe support is tough, then we could add an imperative recipe to do the same replacement, similar to the examples seen here: https://github.com/openrewrite/rewrite-testing-frameworks/tree/v2.18.0/src/main/java/org/openrewrite/java/testing/assertj
While certainly manageable with the existing examples, that would also mean we need to support that recipe going forward, whereas the Refaster style recipe is easier to maintain and support.

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
Projects
Status: Backlog
Development

No branches or pull requests

3 participants