Skip to content

Convert try/catch blocks with fail() to assertThatThrownBy #780

@timtebeek

Description

@timtebeek

What problem are you trying to solve?

Many test codebases contain try/catch blocks where the try block ends with a fail() call to indicate that an expected exception was not thrown. This pattern can be replaced with the more concise and readable assertThatThrownBy from AssertJ, which provides better error messages and clearer intent.

What precondition(s) should be checked before applying this recipe?

  • The project should have AssertJ as a test dependency
  • The code should be using JUnit 4 or JUnit 5 (both use fail() methods)
  • The try block should end with a call to fail() or Assertions.fail() or Assert.fail()

Describe the situation before applying the recipe

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.fail;

class MyTest {
    @Test
    void testExceptionIsThrown() {
        try {
            someMethodThatShouldThrow();
            fail("Expected IllegalArgumentException to be thrown");
        } catch (IllegalArgumentException e) {
            // Expected exception
        }
    }
    
    @Test
    void testExceptionWithAssertion() {
        try {
            service.process(null);
            fail("Should have thrown NullPointerException");
        } catch (NullPointerException e) {
            assertEquals("Input cannot be null", e.getMessage());
        }
    }
}

Describe the situation after applying the recipe

import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

class MyTest {
    @Test
    void testExceptionIsThrown() {
        assertThatThrownBy(() -> someMethodThatShouldThrow())
            .isInstanceOf(IllegalArgumentException.class);
    }
    
    @Test
    void testExceptionWithAssertion() {
        assertThatThrownBy(() -> service.process(null))
            .isInstanceOf(NullPointerException.class)
            .hasMessage("Input cannot be null");
    }
}

Have you considered any alternatives or workarounds?

  • JUnit 5's assertThrows could be used instead, but assertThatThrownBy provides a more fluent API for multiple assertions on the exception
  • Manual refactoring is possible but time-consuming for large codebases

Any additional context

  • The recipe should handle various forms of fail() calls: fail(), fail(String), Assertions.fail(), Assert.fail(), etc.
  • If the catch block contains assertions on the exception (e.g., checking the message), these should be converted to appropriate AssertJ methods like hasMessage(), hasMessageContaining(), etc.
  • The recipe should preserve any comments in the catch block
  • Consider cases where the catch block might have multiple statements - only simple assertion patterns should be converted

Are you interested in contributing this recipe to OpenRewrite?

Yes, I would be interested in contributing this recipe. I would appreciate guidance on:

  • The best way to detect the try/catch pattern with LST
  • How to handle the transformation of different assertion types in the catch block
  • Testing strategies for edge cases

Metadata

Metadata

Assignees

Labels

recipeRecipe request

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions