diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f544fabace..6148fc8df3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,10 +10,46 @@ We appreciate your effort and to make sure that your pull request is easy to rev * Write one JUnit test class for each assertion method with the following naming convention: `__Test`. * Write unit test assertions with AssertJ ! Let's eat our own dog food. * Unit tests method naming convention is underscore-based (like python) and not camel-case, we find it is much readable for long test names! +* Put GIVEN WHEN THEN steps in each test (you can omit steps when not relevant) +* Use `@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)` and `@DisplayName` on the test class - see `OptionalAssert_containsInstanceOf_Test` as an example. +* Use `AssertionUtil.expectAssertionError` for tests expecting to get an `AssertionError` - see `OptionalAssert_containsInstanceOf_Test` as an example.. * Successful assertion unit test method names should start with: `should_pass_...`. * Failing assertion unit test method names should start with: `should_fail_...`. +* Use static import when it makes the code more readable. * If possible, add a (fun) code example in [assertj-examples](https://github.com/joel-costigliola/assertj-examples) and use it in the javadoc. +A good unit test to use as a reference is `OptionalAssert_containsInstanceOf_Test`. Here's a sample below: + +```java +import static org.assertj.core.util.AssertionsUtil.expectAssertionError; +// other imports not shown for brevity + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +@DisplayName("OptionalAssert containsInstanceOf") +public class OptionalAssert_containsInstanceOf_Test extends BaseTest { + + @Test + public void should_fail_if_optional_is_empty() { + // GIVEN + Optional actual = Optional.empty(); + // WHEN + AssertionError assertionError = expectAssertionError(() -> assertThat(actual).containsInstanceOf(Object.class)); + // THEN + assertThat(assertionError).hasMessage(shouldBePresent(actual).create()); + } + + @Test + public void should_pass_if_optional_contains_required_type() { + // GIVEN + Optional optional = Optional.of("something"); + // THEN + assertThat(optional).containsInstanceOf(String.class) + .containsInstanceOf(Object.class); + } +``` + +It's ok not to follow some of the rules described above if you have a good reason not to (use your best judgement) + [assertj-examples](https://github.com/joel-costigliola/assertj-examples) shows how to efficiently use AssertJ through fun unit test examples, it can be seen as AssertJs living documentation. ## Rebase your PR on master (no merge!) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index d00bc0a108..bfa9253c0e 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -1,6 +1,6 @@ #### Check List: * Fixes #??? * Unit tests : YES / NO / NA -* Javadoc with a code example (API only) : YES / NO / NA +* Javadoc with a code example (on API only) : YES / NO / NA diff --git a/src/test/java/org/assertj/core/api/optional/OptionalAssert_containsInstanceOf_Test.java b/src/test/java/org/assertj/core/api/optional/OptionalAssert_containsInstanceOf_Test.java index 9c1b9dee6d..47097719c5 100644 --- a/src/test/java/org/assertj/core/api/optional/OptionalAssert_containsInstanceOf_Test.java +++ b/src/test/java/org/assertj/core/api/optional/OptionalAssert_containsInstanceOf_Test.java @@ -20,8 +20,13 @@ import java.util.Optional; import org.assertj.core.api.BaseTest; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +@DisplayName("OptionalAssert containsInstanceOf") public class OptionalAssert_containsInstanceOf_Test extends BaseTest { @Test @@ -36,13 +41,19 @@ public void should_fail_if_optional_is_empty() { @Test public void should_pass_if_optional_contains_required_type() { - assertThat(Optional.of("something")).containsInstanceOf(String.class) - .containsInstanceOf(Object.class); + // GIVEN + Optional optional = Optional.of("something"); + // THEN + assertThat(optional).containsInstanceOf(String.class) + .containsInstanceOf(Object.class); } @Test public void should_pass_if_optional_contains_required_type_subclass() { - assertThat(Optional.of(new SubClass())).containsInstanceOf(ParentClass.class); + // GIVEN + Optional optional = Optional.of(new SubClass()); + // THEN + assertThat(optional).containsInstanceOf(ParentClass.class); } @Test