UPDATE: I updated this to include updates to the BDD example’s test function name.  I’m starting to dislike having the called interface in the test name.  It’s inflexible and unnecessary and ultimately doesn’t help the reader all that much.

It’s always good when there are people on your team whom you can both learn from and teach things to. Such is the case with my current team. A couple team members have never done unit testing as it’s known in the industry today. Mostly just “pound at the interface until I’m comfortable and throw it over to testing to deal with” unit testing.

During our first code review there were a lot of issues with unit tests. I actually prefer to let it pan out this way; this way people actually get in there and try it out, to later see what works, what didn’t, and (hopefully) get some ideas from the rest of the team about how to improve it. One recurring theme was that there was a lot of redundant code throughout test suites. Setup and teardown were outright missing! That’s good, because now they’ve seen the problem, and part of the solution is to use those. Another thing was how the test cases themselves were structured. I’ve come across two widely accepted ways of structuring unit tests:

Arrange – Act – Assert
Given – When – Then

I’ve personally used both in the same codebase when it makes sense, but I’m wondering if there’s more to it than just semantics and readability. With AAA you are more likely to interact with the class under test directly inside your test function:

  1. void interface_context_somethingHappens()
  2. {
  3.     //arrange
  4.     mock1->setCallShouldSucceed(false);
  5.     mock2->addFakeValue(“Value”);

  6.     //act
  7.     out->interface();

  8.     //assert
  9.     CPPUNIT_ASSERT(somethingHappened);
  10. }

However, I’ve started to notice that if you read your tests, I mean really read them, then using the Behavior-Driven-Development (BDD) Given-When-Then structuring will actually nudge you towards factoring the real test preparation and calls to the class under test out of your test case:

  1. void SomethingShouldHappenInSomeContext()
  2. {
  3.     givenSomeContext();

  4.     whenActionPerformed();
  5.     thenSomethingShouldHappen();
  6. }

  7. void givenSomeContext()
  8. {
  9.     //configure context
  10.     mock1->setCallShouldSucceed(false);
  11.     mock2->addFakeValue(“Value”);
  12. }
  13. void whenActionPerformed()
  14. {
  15.     //execute action
  16.     out->interface();
  17. }
  18. void thenSomethingShouldHappen()
  19. {
  20.     //check that what should happen happened
  21.     CPPUNIT_ASSERT(didSomething);
  22. }

Yes, it’s more code, and yes, it may just be semantics, but I see something more. The naming alone has suggested that maybe I should remove the details from the test. Not only does this produce well-factored code but, but it pulls communication with the class under test to the boundary of my test suite and away from my test cases. Now, if the usage of this class changes for some reason, I only have to update it in one or two places in my test suite, rather than in every single test case. Obviously this approach may not be suited for every single situation, but like I said, I use both where it feels right. Granted, this is considered a good practice when using AAA as well, but you’ve got to name those newly extracted functions something don’t you?

I’d love to get some feedback on this. What convention does your team follow? Are my observations valid or can it be chalked up to something else?