Junit5: Use realistic examples in User Guide

Created on 26 Oct 2018  路  18Comments  路  Source: junit-team/junit5

Overview

Although the examples in the User Guide are technically correct, many of them are not "good examples" for people new to JUnit 5 and JUnit Jupiter in particular.

For example, I hear complaints about examples such as assertEquals(2, 2), examples that use foo and bar, or the fact that the assertThrows(...) example throws an exception directly instead of invoking user code that throws an exception.

Guidelines

  1. Avoid the use of meaningless assertions such as assertEquals(2, 2) or assertTrue(true).

    • Replace such assertions with something more meaningful such as assertEquals(3, calculator.add(1, 2) and assertTrue(isPalindrome("racecar")), respectively.

  2. Focus on a realistic domain model throughout the examples.

    • For example, we already have example.ParameterizedTestDemo.isPalindrome(String) (which could be relocated to something like a StringUtils class) and a Person class.

    • The aforementioned Calculator is from my examples in junit5-demo; a StringUtils class exists there with the aforementioned isPalindrome(String) method.

  3. I propose for starters that we focus on reusing isPalindrome(), Person, and Calculator for most examples.

    • As mentioned by @foobarjet on Twitter, _if necessary_ we can embellish tests using standard JDK libraries (e.g., List<Person>, etc.).

    • If we need something more along the lines of "integration testing" examples, we can consider reusing (and enhancing, if necessary) the existing WebClient demo code.

  4. We should not actually implement anything complex to use as an example.

    • For example, the aforementioned WebClient does not do anything at all, but... it _appears_ to do something realistic in the examples that use it in the User Guide, and that's our primary goal: _for the examples to appear realistic_.

    • Granted, the isPalindrome(String) method does actually do something realistic, _but_ it was not difficult to implement. 馃槆

Deliverables

  • [x] Revise examples in the User Guide (i.e., in the documentation project) so that the examples appear to be "real world" examples.

    • [x] Replace assertions such as assertEquals(2, 2) with something more meaningful such as assertEquals(3, calculator.add(1, 2).

    • [x] Replace assertions such as assertTrue(true) with something more meaningful such as assertTrue(isPalindrome("racecar")).

    • [x] Replace _dummy_ variable names and values (e.g., foo, bar, baz, etc.) with something more meaningful. For example, switching to names of fruits might work out nicely, as was done for an example with @CsvSource (search for "apple").

    • [x] Revise examples for assertThrows(...) so that it appears that user code is being tested instead of an inline lambda expression that throws an exception.

Jupiter Platform Vintage documentation enhancement

Most helpful comment

OK... I added a new Guidelines section to this issue.

@junit-team/junit-lambda, please give a 馃憤 or 馃憥 to let me know if you're OK with the proposed guidelines, and then @psychsane can start on the work.

All 18 comments

Please note that this issue is "up for grabs" for community contribution.

However, please share your ideas here first and gather feedback from the JUnit 5 team before beginning work on this issue.

Would love to work on this @sbrannen

Great idea. Every other software should also do this. For a lot of projects, documentation and examples tend to show a disconnect between users and library developers.

Because of the complex nature of some of the objects I'm working with, I end up with double or triple nested assertAll calls.

val anObject = aVeryBigComputation()

assertAll(
    anObject.someElement.flatMap {
           listOf(
              { assertEquals("some good name", it.name) },
              { validateNested(it.someNestedElement) }
           )
    }
)

fun validateNested(aCollection: Collection<Something>) {
    assertAll(aCollection.map { ....
    ....

This might provide some insights into how assertAll actually gets used.

@psychsane,

Would love to work on this @sbrannen

Glad to hear it!

I'll add some guidelines to this issue's description.

@JLLeitschuh, I think the dependentAssertions() example in the User Guide adequately covers nested use of assertAll().

With regard to nesting assertAll() invocations with streams of Executable, flat mappings, etc., I personally consider that a more advanced use case that is out of scope for the User Guide.

Of course... that's just my opinion. 馃槆

If there is high demand for such an example, the team will consider including something along those lines.

OK... I added a new Guidelines section to this issue.

@junit-team/junit-lambda, please give a 馃憤 or 馃憥 to let me know if you're OK with the proposed guidelines, and then @psychsane can start on the work.

@psychsane,

Would love to work on this @sbrannen

Glad to hear it!

I'll add some guidelines to this issue's description.

Super 馃啋 I am thrilled to be part of this 馃暫

OK... 3 * 馃憤 from team members.

So... @psychsane, go for it!

I've added the in progress label to denote that you are working on it.

If you decide not to work on it, please let us know.

@psychsane, to make the reviews manageable, please rework the examples one _class_ at a time.

In other words, one commit per modified example/demo class.

I think the easiest way is to start at the beginning of the User Guide and work your way down, tackling each example/demo class in the order in which they appear in the User Guide.

If you have any questions, just ask.

Cheers!

@psychsane, feel free to copy the Calculator class and tests for that class from my junit5-demo project. I give you permission. 馃槈

Also, feel free to move all "domain" classes to the src/main/java directory structure in the documentation project.

Thanks for your guidance @sbrannen I am on it!

@sbrannen I have created PR #1664
Should we keep the same PR open until the task is done or one PR/commit?

@sbrannen I have created PR #1664
Should we keep the same PR open until the task is done or one PR/commit?

So far I think it will be manageable in a single PR (based on the commits I've seen thus far from you).

But... I may change my mind if it gets out of hand. So let's just play it by ear. 馃槈

Sorry @sbrannen I haven't yet managed to replace the foo-bars. I will try to get it finished by 1st week of Dec. Thanks!

Sorry @sbrannen I haven't yet managed to replace the foo-bars. I will try to get it finished by 1st week of Dec. Thanks!

No worries.

We just like to keep an eye on who's working on what, so that we know the status of issues as we approach release dates.

Update: I have revised the _Deliverables_.

Closing this issue since all current _deliverables_ have been completed.

Was this page helpful?
0 / 5 - 0 ratings