rewrite-testing-frameworks icon indicating copy to clipboard operation
rewrite-testing-frameworks copied to clipboard

Further Hamcrest to AssertJ migration recipes

Open timtebeek opened this issue 2 years ago • 12 comments

What problem are you trying to solve?

After https://github.com/openrewrite/rewrite-testing-frameworks/pull/355 we already support some ~50 Hamcrest Matcher to AssertJ migrations. Not all Hamcrest Matchers are covered however, for a potentially incomplete (but non-breaking) migration. Below is a quick overview of cases we might want to cover still, depending on how common they are, and the effort needed to convert those automatically.

Sub matchers allOf(Matcher...), anyOf(Matcher...), ...

Matchers that take subsequent matchers as argument are not yet covered, and likely need a dedicated recipe for each case. Here's a likely quick overview of such Matchers:

allOf(Iterable<Matcher<? super T>> matchers)
anyOf(Iterable<Matcher<? super T>> matchers)
both(Matcher<? super LHS> matcher)
either(Matcher<? super LHS> matcher)
everyItem(Matcher<U> itemMatcher)
hasItem(Matcher<? super T> itemMatcher)
hasItems(Matcher<? super T>... itemMatchers)
aMapWithSize(Matcher<? super Integer> sizeMatcher)
hasSize(Matcher<? super Integer> sizeMatcher)
contains(Matcher<? super E> itemMatcher)
contains(Matcher<? super E>... itemMatchers)
containsInAnyOrder(Matcher<? super T>... itemMatchers)
containsInRelativeOrder(Matcher<? super E>... itemMatchers)
iterableWithSize(Matcher<? super Integer> sizeMatcher)
hasEntry(Matcher<? super K> keyMatcher, Matcher<? super V> valueMatcher)
hasKey(Matcher<? super K> keyMatcher)
hasValue(Matcher<? super V> valueMatcher)
hasToString(Matcher<? super String> toStringMatcher)

More inverted matches with not

There's likely more direct replacement options in AssertJ for some of the inverted not(Matcher) that we don't already cover. Not all will have direct replacements, and for those cases we might need more complicated constructs.

not(Matcher<T> matcher)

Matchers for arrays

array(Matcher<? super T>... elementMatchers)
arrayContaining(List itemMatchers)
arrayContaining(Matcher<? super E>... itemMatchers)
arrayContainingInAnyOrder(Collection itemMatchers)
arrayContainingInAnyOrder(Matcher<? super E>... itemMatchers)
arrayWithSize(Matcher<? super Integer> sizeMatcher)
hasItemInArray(Matcher<? super T> elementMatcher)

Any additional context

There's an additional issue to convert to JUnit Jupiter, which could serve as inspiration.

timtebeek avatar Jun 18 '23 11:06 timtebeek

Not quite done yet.

timtebeek avatar Jul 12 '23 09:07 timtebeek

Still not done, although we're getting closer after #375 ; For anyOf(..) we could look towards org.assertj.core.api.AbstractAssert#satisfiesAnyOf(org.assertj.core.api.ThrowingConsumer<? super ACTUAL>...), which can similarly take multiple throwing consumers, which at first could be calls to Hamcrest still, before the other recipes in hamcrest.yml convert those over to AssertJ Assertions.

In terms of pseudo code we could write a recipe that first and only converts

assertThat("str1 and str2 should be equal", str1, allOf(equalTo(str2), hasLength(12)));

into

assertThat(str) // AssertJ
  .as("str1 and str2 should be equal")
  .satisfiesAnyOf(
    arg -> assertThat(arg, hasLength(12)), // Hamcrest
    arg -> assertThat(arg, equalTo("str2")) // Hamcrest
  );

NOTE: the comments indicate that library is used per method invocation

The subsequent recipes in hamcrest.yml can then turn the lambda's into AssertJ Assertions to complete the migration.

Done in https://github.com/openrewrite/rewrite-testing-frameworks/pull/391

timtebeek avatar Jul 20 '23 10:07 timtebeek

Another case to cover, as seen in spring-batch

String item = this.reader.read();
assertThat(item, is("val2"));

Done in #88

timtebeek avatar Jul 20 '23 22:07 timtebeek

Might need to additionally clean up the import of MatcherAssert

MatcherAssert.assertThat(message, Matchers.containsString("Undeclared general entity \"entityex\""));
assertThat(message).contains("Undeclared general entity \"entityex\"");

Done in #389

timtebeek avatar Jul 20 '23 22:07 timtebeek

How hard would it be to support org.hamcrest.CoreMatchers as an alias for org.hamcrest.Matchers?

TobiX avatar Nov 14 '23 10:11 TobiX

How hard would it be to support org.hamcrest.CoreMatchers as an alias for org.hamcrest.Matchers?

Likely this easy; thanks for the suggestion!

  • https://github.com/openrewrite/rewrite-testing-frameworks/pull/425

timtebeek avatar Nov 14 '23 14:11 timtebeek

@TobiX The above PR has been merged and is now available through our snapshot versions. Hope that helps! If you have any further suggestions do let us know!

timtebeek avatar Nov 20 '23 11:11 timtebeek

Since this got closed 3 times already, I'm not sure which of the requirements in the original post are already considered done. I just observed that a combination of two of them is not covered yet:

        assertThat(Set.of(0), not(hasItem(1)));
        assertThat(List.of(0), not(hasItem(1)));

should become

        assertThat(Set.of(0)).doesNotContain(1);
        assertThat(List.of(0)).doesNotContain(1);

timo-abele avatar Mar 13 '24 10:03 timo-abele

Turns out that's pretty easy; fixed in https://github.com/openrewrite/rewrite-testing-frameworks/commit/7fb2c9d4e8158df1649e816d8e2ccd18db775e2e . As you can see we have a recipe that can convert any negated matcher to an AssertJ replacement, so if you find any others feel free to add a similar change to the project. :)

timtebeek avatar Mar 13 '24 10:03 timtebeek

Another matcher I just found that's not converted:

assertThat(object, instanceOf(MyType.class))

motlin avatar Apr 13 '24 23:04 motlin

Another matcher I just found that's not converted:

assertThat(object, instanceOf(MyType.class))

That's another good addition indeed; is that something you'd like to contribute? I'm finding I'm not getting to a direct implementation.

timtebeek avatar Apr 25 '24 14:04 timtebeek

Turns out we do have a recipe for that instanceOf conversion, but it might fail due to missing types. https://github.com/openrewrite/rewrite-testing-frameworks/blob/dbd67ec864df31b0b65bb35251b7a866231e390e/src/main/resources/META-INF/rewrite/hamcrest.yml#L99-L104

timtebeek avatar May 21 '24 14:05 timtebeek