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.

Sign up for GitHub

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

Jump to bottom

Enable JUnitAssertXXXToAssertThat for JUnit 4 #672

Open
kthoms opened this issue Feb 10, 2025 · 2 comments · May be fixed by #673
Open

Enable JUnitAssertXXXToAssertThat for JUnit 4 #672

kthoms opened this issue Feb 10, 2025 · 2 comments · May be fixed by #673
Labels
enhancement New feature or request question Further information is requested

Comments

@kthoms
Copy link
Contributor

kthoms commented Feb 10, 2025

What problem are you trying to solve?

I want to migrate JUnit 4 assertions to AssertJ. The existing recipes migrate only JUnit 5 assertions. From the recipe's description it is also not clear that "JUnit assertEquals to AssertJ" refer to JU 5 only.

Describe the solution you'd like

The existing recipes might be enhanced to cover both cases by trying to match methods for org.junit.jupiter.api.Assertions or org.junit.Assert and resolve the parameters differently. The assertions are quite similar except for the order of arguments.

Have you considered any alternatives or workarounds?

There could be also dedicated JUnit4AssertXXXToAssertThat recipes. This would likely produce more duplicated code.

Migrating to JUnit 5 is the ultimate goal, but I would like to do it stepwise and migrate the assertions before migrating the tests themselves.

Are you interested in contributing this feature to OpenRewrite?

Yes, I have a working draft for one recipe already. I could contribute this already, but it feels as if all JUnitAssertXXXToAssertThat recipses should be enhanced together. Might be surprising if only one recipe behaves different.
@kthoms kthoms added the enhancement New feature or request label Feb 10, 2025
@timtebeek
Copy link
Contributor

hi @kthoms ; thanks for pointing out that it wasn't immediately clear the recipe indeed also upgrades you to JUnit 5. I feel it might be complicated to update all recipes to also support JUnit 4, since there's also steps invoked that first apply JUnit 5 best practices to achieve a more idiomatic migration to AssertJ assertions.

rewrite-testing-frameworks/src/main/resources/META-INF/rewrite/assertj.yml

Lines 453 to 482 in fc9d8b4

name: org.openrewrite.java.testing.assertj.JUnitToAssertj
displayName: Migrate JUnit asserts to AssertJ
description: >-
AssertJ provides a rich set of assertions, truly helpful error messages, improves test code readability.
Converts assertions from `org.junit.jupiter.api.Assertions` to `org.assertj.core.api.Assertions`.
Will convert JUnit 4 to JUnit Jupiter if necessary to match and modify assertions.
tags:
- testing
- assertj
recipeList:
# First improve the assertions for JUnit, to fix inverted expected/actual values
- org.openrewrite.java.testing.junit5.JUnit5BestPractices
- org.openrewrite.java.testing.assertj.JUnitAssertArrayEqualsToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertEqualsToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertFalseToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertNotEqualsToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertNotNullToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertNullToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertSameToAssertThat
- org.openrewrite.java.testing.assertj.JUnitAssertTrueToAssertThat
- org.openrewrite.java.testing.assertj.JUnitFailToAssertJFail
- org.openrewrite.java.testing.assertj.JUnitAssertThrowsToAssertExceptionType
- org.openrewrite.java.testing.assertj.JUnitAssertInstanceOfToAssertThat
- org.openrewrite.java.dependencies.AddDependency:
groupId: org.assertj
artifactId: assertj-core
version: 3.x
onlyIfUsing: org.assertj.core.api.Assertions
acceptTransitive: true
- tech.picnic.errorprone.refasterrules.JUnitToAssertJRulesRecipes

It's been 7,5 years since JUnit 5 came out, and JUnit 6 is on the horizon for the end of this year. Since you're already eyeing JUnit 5, perhaps we should then not complicate the recipes here? Note that you can still develop and run those recipes yourself, but adoption into OpenRewrite might complicate what we have to maintain going forward.

@timtebeek timtebeek added the question Further information is requested label Feb 10, 2025
kthoms pushed a commit to kthoms/rewrite-testing-frameworks that referenced this issue Feb 11, 2025
Enhance JUnitAssertEqualsToAssertThat to support JUnit4 also (openrew…
d0127a7 
…rite#672)
@kthoms kthoms linked a pull request Feb 11, 2025 that will close this issue
Open
3 tasks
@kthoms
Copy link
Contributor Author

kthoms commented Feb 11, 2025

I leave the decision up to you. For me it is important to migrate the assertions before actually upgrading to JUnit 5. The recipe that converts to JUnit 5 simply breaks too much stuff in our code base. So I want to take this step first. But I can use the recipes for our purpose now, of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request question Further information is requested
Projects
OpenRewrite
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Junit4 to assertj kthoms/rewrite-testing-frameworks
2 participants
@kthoms @timtebeek