Should //given //when and //then comments be included in unit tests?

Question

I've joined a new team of great colleagues and a difference of opinion has come up concerning including //given //when and //then comment blocks in unit tests.

For example

public void mapToitemList_MapsCarsToitemsCorrectly_ForSingleFurrari() {
    // given
    String commercialProductName = "Furrari";
    String productNumber = "12423";
    BigDecimal numberOfScrews = BigDecimal.TEN;
    ValveType valveType = ValveType.GOOD;

    Car car = new Car() //
                    .withCommercialProductName(commercialProductName) //
                    .withProductCarNumber(productNumber) //
                    .withNumberOfScrewsAmount(new NumberOfScrewsAmount().withValue(numberOfScrews)) //
                    .withBalances(new Engine().withValves(new ValveType().withValue(valveType)));
    List<Car> carList = Collections.singletonList(car);

    // when
    List<Item> mappedListOfCars = CarMapper.mapToItemList(carList);

    // then 
    assertThat(mappedListOfCars).isNotEmpty();
    Item carAsItem = mappedListOfCars.get(0);

    assertThat(carAsItem.getCommercialProductName()).isEqualTo(commercialProductName);
    assertThat(carAsItem.getNumberOfSetsOfScrews().getValue()).isEqualTo(Convertor.numbersOfScrewsToNumberOfSets(numberOfScrews).getValue());

}

Currently my main reason to include them is because I'm used to it and the main counter-argument I'm getting is that comments should be shunned.

Does the added structure of these comments merit the use of comments? Martin Fowler mentions some people use them, but passes no judgment on it.

Answer 1

Code comments are usually taken as a bad practice because:

• They add new artifacts to maintain in the future.
• The code should be as simple and well written to be self-explanatory.

That said, as usual, there are no absolute truths. Personally, this kind of comments is positive to me as they add additional semantics to the code and let me know which section of the test I'm working on. Besides, it also enforces following a certain test coding structure.

So, to sum up:

• Code comments that add no semantic to the code like // Iterate over the list elements before a foreach loop: Bad.
• Code comments that add good semantic and enforces good practices: Good.

But I want to remark this is rather a personal/organizational preference than an absolute truth.

Answer 2

Clearly structuring your tests in a given–when–then or arrange–act–assert schema is a good idea – but this structure should be evident in your code, even without comments. Often, a blank line between each section is entirely sufficient to communicate this division. Extra comments would then just be noise.

Things become more complicated when you also assert preconditions, when the action contains multiple distinct steps, or when describing the expected state requires extra preparation. Many good test cases do not fit into a five line test method. In these more complicated scenarios, it may be sensible to clarify the intent of each section with a comment.

Answer 3

I’ve seen tests with 3 lines of code accompanied by 3 lines of given/when/then comments.

I see it as a lame attempt to improve test codebase when it becomes hard to read/maintain. Short/simple tests do not need it. Long/complex tests need refactoring and meaningful comments instead of throwing in 3 extra lines of placeholders and calling it done. Try removing all of those placeholders and see which tests are not so much readable; those are the ones that truly need your attention.

In another perspective, that practice conditions the developers to an artificial aid that is not available in most places. It may look nice and pleasing to the eyes; however, do you really need it everywhere to be able to comprehend a test? If no, it’s merely a decoration that is not essential to thought process. To bring this a bit further, my theory is that it is even a kind of distraction that gives you a false sense of understanding (the 3-part structure) when you read a test, and that actually hampers your code conprehension.

And obviously, it’s such a chore/time waster to force yourself and fellow developers to add those extra 3 lines where code is perfectly readable.

In general, it’s not so much useful or harmful. It can be a personal preference/technique.

Answer 4

Code changes, comments ain't.

Instead of commenting the code rather use commit messages that describe the state of the code while it evolves. It's a known detail, messages sent through comments could also be sent by method names, so comments shouldn't be included in the code. There might be some acceptable cases when a tight schedule prevails and commenting the code makes it to keep up with the schedule, with the side note comments should be short-term solution to be improved / replaced / removed.

Sure all these should be included in the coding style guide the team uses that is a team choice.

Answer 5

The main problem I have with this sort of convention is that it's so proscriptive. It effectively mandates that a test must follow the arrange-act-assert pattern, regardless of the test. And, as a result, you can end up with code such as in your example:

// when
List<Item> mappedListOfCars = CarMapper.mapToItemList(carList);

// then 
assertThat(mappedListOfCars).isNotEmpty();
Item carAsItem = mappedListOfCars.get(0);

Why is List<Item> mappedListOfCars = ... in "when" rather than "given"? Why are you doing Item carAsItem = ... in "then"? The simple answer is because you have to shoehorn the test into those three steps.

So get rid of those comments. They serve no useful purpose and the test is easier to read without them.

Answer 6

I want to come in with a different answer here, in favor of adding these comments.

TL;DR Use these comments when they make sense. I'm all for it.

But first, I want to be very clear here that this answer is only scoped to the specific given/when/then (or act/arrange/assert, same difference) comments. I agree with the people who are against them when we're talking about code comments in general (e.g. used to fix badly structured code, code should be evident to read, ...), but I think there is a special case for the GWT comments that is not being accounted for in their answers.

GWT (or AAA) is a known concept because it so accurately describes the vast majority of test implementations. You set up some scenario (given/arrange), you let it play out (when/act), you confirm that it behaved the way it needed to in the given scenario (then/assert).

These three sections of the testing process are discrete and should be done one after the other. It would be illogical to e.g. change something about the scenario after you've already acted on it (because then you've just invalidated the first act) or test something before you've acted on the scenario (because what is there to test if not the act itself?).

These three steps are also discrete parts of the developer's mindset when working on a test. Whenever you want to make any change you a test, or you're writing it, you need to be innately aware of whether you're arranging, acting or asserting, because you need to always respect the previous paragraph (i.e. never doing anything out of order).

Having established these points, there is still one other solution: use discrete submethods. Something along the lines of:

public void MyTest()
{
    Arrange();
    Act();
    Assert();
}

This follows the usual guideline on code comments that it's better to structure your code readably than it is to add comments to aid difficult to read code. I actually agree with this advice for real code, but not for unit tests. Tests can get very hairy very quickly when any of its steps require more than a handful of lines, and the submethods introduce several negatives for a testing context:

• The test is no longer readable from top to bottom, you need to go and jump to different submethods to understand it. Sure, you could maybe put your submethods in the same order, but then you've still introduced additional fluff (method signature and braces) inbetween the relevant bits - at best this approach would be equally as bad as adding a // when comment.
• Scopes/closures are making it hard for variables/data to flow from one part of the test to the next. Not that it can't be done, but in order to make it work now, it requires additional complexity. Because a test is a very restricted scope, it's reasonable for it to define all of its content in a "global" scope (i.e. scoped to the whole test), but the submethods make this harder to achieve.

So it detracts from readability (first bullet point), and it adds complexity (second bullet point). That's not great.

This is why I consider given/when/then (or act/arrange/assert) comments valid additions to a test body. To summarize the benefits:

• It can help a developer quickly navigate to the correct section of a test.
• It adds an easy visual gauge to spot if you've put some logic in the wrong section
• It does not meaningfully decrease readability (compared to other possible solutions to the first two bullet points)
• It does not introduce logical complexity (compared to other possible solutions to the first two bullet point)

So yeah, use these comments when they make sense. I'm all for it.

---

Just to point out some edge cases that people might point out in the comments:

• Integration tests are a different breed of test where GWT/AAA is not as easy to maintain, as integration tests often entail a more complex orchestration, e.g. GWGWGWT or GWTGWTGWT
• I'm not saying that a dev would be unable to navigate the code without these comments. I'm saying that it adds more value than it adds problems, so it's a net positive.
• If there is a specific case where these comments don't add a net positive, then don't use them there. That's not an argument against not using these comments in general, though.