Further Hamcrest to AssertJ migration recipes

[timtebeek]

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]

Not quite done yet.

[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]

Another case to cover, as seen in spring-batch

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

Done in #88

[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

[TobiX]

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

[timtebeek]

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]

@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!

[timo-abele]

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);

[timtebeek]

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. :)

[motlin]

Another matcher I just found that's not converted:

assertThat(object, instanceOf(MyType.class))

[timtebeek]

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]

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