PicnicSupermarket/error-prone-support

BugChecker to validate JUnit 5's parameterized test cases

Opened this issue · 1 comments

werli commented

Problem

Using JUnit 5's Arguments API performs validation of its parameters at runtime which can lead to three scenarios:

  1. Mismatching types - found when executing the tests (i.e. at runtime).
  2. Insufficient number of parameters - found at runtime.
  3. Definition of superfluous parameters - not found at compile nor runtime.

Description of the proposed new feature

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

I would like to warn about the following code:

  private static Stream<Arguments> sumTestCases() {
    /* { value1, value2, result } */
    return Stream.of(
        arguments(1, 1, 2), // Correct test case.
        arguments(
            1,
            1,
            // (1) Wrong type; results in a `ParameterResolutionException`, i.e. runtime exception.
            "Irrelevant parameter"),
        arguments(
            1,
            1,
            2,
            // (3) Superfluous last parameter does not result in an error.
            Optional.empty()));
  }

  @ParameterizedTest
  // (1) Wrong type; results in a `ParameterResolutionException`, i.e. runtime exception.
  @EnumSource(java.time.DayOfWeek.class)
  // (2) Insufficient number of parameters.
  @ValueSource(ints = {1, 2, 3})
  @MethodSource("sumTestCases")
  void sum(int value1, int value2, int result) {
    assertThat(value1 + value2).isEqualTo(result);
  }

Considerations

(1) and (2) are caught when executing the tests themselves, but (3) is not. Such superfluous parameters result in misleading test cases. Out of the three scenarios, (3) is IMO the one that should be tackled first. However, when adding parameter validation, why only do (3) when the other two are probably not much more work.

Participation

  • I am willing to submit a pull request to implement this improvement.

I most likely won't get to this in the near future.

Tnx for filing this issue, @werli! To whomever picks this up: case (3) is not always an issue, since a method source may be used by multiple @ParameterizedTest methods, some that take more arguments than others.