Rewrite JUnit Assertions.assertThrows() to AssertJ assertThatThrownBy

[oxkitsune]

Problem

We prefer to use AssertJ for our assertions. So we could use a Refaster rule that automatically prefers this over the assertions library built into JUnit!

Description of the proposed new feature

• Support a stylistic preference.
• Avoid a common gotcha, or potential problem.

I would like to rewrite the following code:

org.junit.jupiter.api.Assertions.assertThrows(
    RuntimeException.class,
    () -> {
      throw new RuntimeException("bar");
    });

to:

assertThatThrownBy(() -> {
	throw new RuntimeException("bar");
})
.isInstanceOf(RuntimeException.class):

[Stephan202]

Good idea! I would like to suggest that we expand the scope of this issue a bit to cover "some sizable subset" of the org.junit.jupiter.api.Assertions API. Having a separate PR for each method in that class is not efficient 😅.

[giall]

Interested in picking this up! Below are the methods in org.junit.jupiter.api.Assertions and the equivalent AssertJ code. There are quite a few but quite straight-forward so they could all be part of one big PR.

Conditions:
assertTrue(condition) -> assertThat(condition).isTrue()
assertFalse(condition) -> assertThat(condition).isFalse()
assertNull(obj) -> assertThat(obj).isNull()
assertNotNull(obj) -> assertThat(obj).isNotNull()
assertInstanceOf(clazz, obj) -> assertThat(actual).isInstanceOf(clazz) (or isExactlyInstanceOf?)

Equality:
assertEquals(expected, actual) -> assertThat(actual).isEqualTo(expected)
assertNotEquals(expected, actual) -> assertThat(actual).isNotEqualTo(expected)
assertSame(expected, actual) -> assertThat(actual).isSameAs(expected)
assertNotSame(expected, actual) -> assertThat(actual).isNotSameAs(expected)
assertArrayEquals(expected, actual) -> assertThat(actual).containsExactly(expected)
assertIterableEquals(expected, actual) -> assertThat(actual).containsExactly(expected)
assertLinesMatch -> ?

Exceptions:
assertThrows(clazz, () -> ...) -> assertThatThrownBy(() -> ...).isInstanceOf(clazz)
assertThrowsExactly(clazz, () -> ...) -> assertThatThrownBy(() -> ...).isExactlyInstanceOf(clazz)
assertDoesNotThrow(() -> ...) -> assertThatCode(() -> ...).doesNotThrowAnyException()

Other:
assertAll -> ?
assertTimeout -> ?
assertTimeoutPreemptively -> ?

Each method also has overloads with a String message or Supplier<String> message supplier, so we'd also need templates for these. Not sure if there's a nice way to group these or we'd need to define separate templates for each.
assertEquals(expected, actual, message) -> assertThat(actual).isEqualTo(expected).withFailMessage(message)
assertEquals(expected, actual, messageSupplier) -> assertThat(actual).isEqualTo(expected).withFailMessage(messageSupplier)

Left a question mark for a few I'm not sure about, but I'm guessing those are used much less frequently.

[Stephan202]

Thanks for the overview @giall! To go over your points:

• The String and Supplier<String> overloads would require their own rules, which does explode the scope of the work a bit. if you're up for it: great! If not: also completely understandable!
• What I would do (which also eases reviewing), is to add a line comment for each public method in org.junit.jupiter.api.Assertions, in order, and replace the comments (methods) for which a rule is implemented. This will ease reviewing and makes clear what (future) work remains. (Longer term we should have a bug checker which validates an annotation such as @MigratesAllOf(type = Assertions.class, skipped = {"method-signature-1", "method-signature-2"}). ✨)
• Looking at the code, assertInstanceOf(clazz, obj) maps onto assertThat(actual).isInstanceOf(clazz).
• (Do note that assertInstanceOf(clazz, obj) casts and returns obj, which AssertJ doesn't do. This can theoretically cause some failures. Wouldn't worry about this for now.)
• assertLinesMatch has odd semantics; I doubt AssertJ has a direct equivalent.
• For the other three I'd have to look a bit deeper; can get back to that later (perhaps only during PR review 😉.)

Thanks for having a crack at this, and if you have further questions: don't hesitate to ask! 🚀

[rickie]

Nice @giall! Indeed it makes sense to bundle these rules.

Maybe we can also include the .fail method(s):

• fail() -> this one doesn't have a direct equivalent... (Any ideas on how to approach this one?)
• fail(String) -> fail(String)

Not sure if we should tackle timeout now, I haven't yet found a nice AssertJ equivalent. I think it would make more sense to rewrite it to JUnit's timeout annotation. This sounds like out of scope for this ticket however 😄.

W.r.t. assertAll, maybe this would be better suited in an Error Prone check. Let's defer that one as well. It is an interesting one, see examples.

---

which does explode the scope of the work a bit.

Sounds to me like it'd be better to split that in two PRs then 😉.

[giall]

@rickie Looking at TestNGToAssertJRules, we can do the same for fail and replace it with throw new AssertionError().

[oxkitsune]

Resolved in #417