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

Hamcrest to JUnit Jupiter migration recipes #212

Closed
yeikel opened this issue Apr 2, 2022 · 6 comments
Closed

Hamcrest to JUnit Jupiter migration recipes #212

yeikel opened this issue Apr 2, 2022 · 6 comments
Labels
assertj good first issue Good for newcomers recipe Recipe request

Comments

@yeikel
Copy link
Contributor

yeikel commented Apr 2, 2022

While the Hamcrest repo is still active, the latest release is from Oct 16, 2019

I think that this is a potential good use case for a migration recipe

We could have two different types of migration recipes (and issues)

  1. Hamcrest to JUnit Jupiter migration recipes #212
  2. Further Hamcrest to AssertJ migration recipes #357

Relevant discussion : hamcrest/JavaHamcrest#353

@tkvangorder
Copy link
Contributor

For context, we had outlined some specifics around this migration. We ended up closing the issue because we did not have the resources to work on it at the time.

#16

@tkvangorder tkvangorder added enhancement New feature or request help wanted labels Apr 4, 2022
@tkvangorder tkvangorder moved this to Backlog in OpenRewrite Apr 4, 2022
@tkvangorder tkvangorder moved this from Backlog to Ice Box in OpenRewrite Apr 8, 2022
@tkvangorder tkvangorder moved this from Request for help to Future in OpenRewrite Apr 18, 2022
@tkvangorder tkvangorder added recipe Recipe request and removed help wanted enhancement New feature or request labels Apr 19, 2022
@rovarga
Copy link

rovarga commented Dec 29, 2022

One thing that might make sense is the straightforward migration of assertThat(foo, instanceOf(Foo.class)) to JUnit5 assertInstanceOf(Foo.class, foo).
That would provide a framework-independent migration, potentially lowering dependencies on Hamcrest for JUnit5 users.

@timtebeek
Copy link
Contributor

@rovarga That would indeed make sense as a starting point; is that something you'd be willing to contribute with some guidance?
I suspect there's quite a few more in #16 that are easy to tackle once an initial sample recipe is available.

@rovarga
Copy link

rovarga commented Jan 2, 2023

@rovarga That would indeed make sense as a starting point; is that something you'd be willing to contribute with some guidance? I suspect there's quite a few more in #16 that are easy to tackle once an initial sample recipe is available.

Yes I would certainly be interested, unfortunately I do not have the cycles right now. Perhaps in a couple of weeks (I will keep this on my radar).

@timtebeek
Copy link
Contributor

Glad to hear you're interested in helping out! Let us know how we can best support you. I'll pitch a quick outline right now.

Looking through the list in #16 I'm seeing quite a few matchers that have one to one replacements in AssertJ, which might be the easiest to tackle. Assuming here that people picked Hamcrest for the fluent matchers, and then AssertJ is the most direct replacement to that.

Alternatively we can (only) migrate to JUnit 5, and then optionally after that chain the existing migration from JUnit 5 to AssertJ; that way we can have both, provided there's equivalents for all matchers in both JUnit 5 and AssertJ, and recipes to go between those.

Either way I suppose you can start by creating a recipe that takes in two options:

  1. a String method pattern of a Matchers method to replace, for instance org.hamcrest.Matchers equalTo.
  2. the replacement method pattern; so for AssertJ that would be org.assertj.core.api.AbstractAssert isEqualTo

The recipe can then match on the first method pattern, and replace the surrounding org.hamcrest.MatcherAssert.assertThat with a call to org.assertj.core.api.Assertions.assertThat with the same first argument, and using the hamcrest matcher argument in a chained call to the replacement method.

-org.hamcrest.MatcherAssert.assertThat(actual, equalTo(expected));
+org.assertj.core.api.Assertions.assertThat(actual).isEqualTo(expected);

or alternatively for JUnit 5

-org.hamcrest.MatcherAssert.assertThat(actual, equalTo(expected));
+org.junit.jupiter.api.Assertions.assertEquals(expected, actual);

Of course with appropriate new imports, removing the old imports, and using JavaTemplate to select the correct overloaded method where applicable for AssertJ. There should be plenty of examples to see how to achieve that, even if it can be daunting at first.

With the above you'd then be able to convert quite a few similar Hamcrest matchers using a single recipe, by configuring the options in a yaml recipe file. That way you'll have maximum output from a single Java recipe.

Anything the generic recipe can not handle, can then be picked up separately in a dedicated recipe for that use case. We prefer to maintain multiple relatively simple recipes, rather than one recipe to migrate all use cases.

Hope this helps!

@timtebeek timtebeek added the good first issue Good for newcomers label May 18, 2023
@timtebeek timtebeek changed the title Recipes to Migrate from Hamcrest Hamcrest to JUnit Jupiter migration recipes Jun 18, 2023
timtebeek added a commit that referenced this issue Jun 22, 2023
* Migrate Hamcrest Matchers to AssertJ Assertions

For #212

* Add AssertJ dependency if needed

* Make HamcrestMatcherToAssertJAssertionTest ParameterizedTest

* Demonstrate a few more matchers we could add to Yaml

And limitations that don't yet work

* Handle even more cases for String and Object

* Do not convert methods `is` and `not` that take in Matchers

* Swap tests and convert passing recipes into yaml

* Remove Hamcrest `is(Matcher)` ahead of migration

* Ignore any sub matcher and support primitive numbers

* Capture behaviour around Iterable and varargs

* Support replacements for lists

* Add support for `in` and `isIn`

* Handle third argument for reason

* Remove condition that's never true for three arguments

* Add replacements for maps

* Separate and add more String matchers

* Convert instanceOf and isA

* Remove calls to `MatcherAssert.assertThat(String, boolean)`

* Rename class to HamcrestMatcherToAssertJ

* Add document examples and link to issue

* Correctly limit application of recipe with three arguments

* Use a Precondition to limit recipe execution early

* Fix test class issues

* Add HamcrestNotMatcherToAssertJ for `not(Matcher)` replacements

* Add more `not(Matcher)` cases from AbstractCharSequenceAssert
@timtebeek
Copy link
Contributor

Thanks again @matusmatokpt !

@github-project-automation github-project-automation bot moved this from Recipes Wanted to Done in OpenRewrite Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assertj good first issue Good for newcomers recipe Recipe request
Projects
Archived in project
Development

No branches or pull requests

4 participants