Friday, June 30, 2017

Book Review: Working Effectively with Unit Tests

I recently finished reading Working Effectively with Unit Tests by Jay Fields (Amazon link). I'm a bit mixed on whether I would recommend this book. There are some good unit testing tips, but it wasn't especially memorable, and there were a few things that I didn't care for too much.

The short version is that my main recommendation for unit testing techniques will remain The Art of Unit Testing by Roy Osherove (Jeremy's review).

Things I Liked
Keep What Works
There were a few general messages that I really liked. The first is try stuff and keep what works. I always prefer a non-dogmatic approach because not every technique works in every environment. I really appreciate that Fields talks about some of the things that he's done in the past, liked them, but then later stopped using them because they no longer fit his situation.

Descriptive Tests
Another message that I really liked is to keep tests DAMP (Descriptive And Maintainable Procedures) as opposed to DRY (Don't Repeat Yourself). Keeping common code centralized is a good practice for our production code, but testing code is a bit different. Tests are really meant to be independent, isolated, and not interact with each other, whereas our production code needs to be cohesive and collaborative.

This really follows my general advice to make sure that tests are readable. It's good to be a bit more verbose and explicit so that they are very easy to approach when we need to look at the actual test code.

Setup Methods
As far as specific recommendations, there are several that I found useful. First is generally avoiding setup methods. By keeping setup (the "Arrange" step) local to the test, it enhances readability and helps make sure we are only using the things that we need for a particular test.

For example, in a test class-level setup method, we may have multiple objects instantiated with various states that we can use in different tests. This means that we could have objects that are instantiated in a setup but are not actually used for a test. This means that we've got some wasted code, and it also makes it a bit harder for us to determine exactly what's important for a particular test.

I've explored something along these lines (Unit Testing: Setup Methods or Not?), although I tended to use factory methods that are explicitly called to get some of the non-critical bits out of the tests themselves. Fields' technique is definitely worth exploring some more.

Assertions
There are a couple pieces of good advice regarding assertions, including one assertion per test. This is particularly important because most testing frameworks use exceptions for tests. So when the first assertion fails, no code after it will run. If we have several assertions, we don't know if we have a single failure or multiple failures. If we keep one assertion per test, then we can tell where our problems are. This also goes along with the DAMP approach; we don't need to be afraid of duplicating behavior in unit tests.

Another piece of advice is to assert last. The thing that we are actually verifying should be at the very end of the test method. This makes it really easy to find what we're testing. And this also goes along with the "Arrange/Act/Assert" layout which I really like.

Fields also spends time talking about testing for exceptions and some of the weirdness that is caused by using a try/catch block. When using a try/catch block, the assertion is in the middle of code (usually in the catch block), and we also need to have a "fail" in the middle of the code if an exception is not thrown.

To get around this, Fields suggests making a custom assertion method, Assert.Throws(), that can be used to check for exceptions without a try/catch block, and can also be put at the end of the test. That way we can follow the "assert last" advice. This is similar to the "Assert.Throws()" that is provided with NUnit (and other frameworks that provide custom assertions). I wrote a bit about this in Testing for Exceptions with NUnit.

Things I Have Mixed Feelings About
Solitary and Sociable
There were a few things that I had mixed feelings about. One of them had to do with Solitary Unit Tests vs. Sociable Unit Tests.

A solitary unit test is a test where only 1 object is "new"ed up. Everything else is some sort of test double, like a stub or mock. A sociable unit test has multiple objected "new"ed and checks the interaction between them.

I don't really like the definition of sociable unit test because to me that steps outside of the world of unit testing and starts moving into integration testing. Fields does mention integration testing, but he looks at that as more of an end-to-end type thing. I've generally looked at integration testing as checking that the objects work together at different levels -- from localized to end-to-end.

This isn't really important in the grand scheme of things. I just seem to put all of my "unit tests" into the "solitary unit test" bucket.

Fields does make the recommendation of separating the solitary unit tests and sociable unit tests. The solitary ones are generally very fast and we want to run those very often, and the sociable ones may take a bit longer (for example, if there are I/O operations) and we probably run those less frequently. This is very good advice.

Sample Scenario
Another thing that I was mixed about is the sample scenario. I do like that the same application code was used throughout the entire book. But the scenario was a video rental store. Since the book is from 2014, this example was a bit out of date when it was published. As someone who is well over 40, I have no problems remembering what it was like to rent movies from a physical store. But I'm sure there are lots of developers today who have not had that experience.

Again, not a big deal. The samples could easily be updated to use a video kiosk rather than store.

Things I Didn't Like
The First Test
While I like that the same scenario is used throughout the book and that there was a focus on continuously improving the tests, I really did not like the first example.

The reason is that Fields states, "Let's get straight to code" and then shows a rather-difficult-to-read example and says "you don't need to understand this." From my perspective, that defeats the purpose of going straight to code.

Motivators
Another thing that left a bit of a bad taste had to do with test motivators. Fields spends some time telling us that it is important to understand our motivation for writing tests in order to write effective tests that match the motivation. He also spends quite a bit of time listing different motivators. The problem is that these motivators are not used in the rest of the book (unless I missed it). So it looks like we're being told that it's important to understand the motivation, but doesn't tell us how that impacts things in a practical way.

Naming
Another thing I don't agree with is Fields' opinion on naming tests. He likens test names to comments. Since the test methods are never called directly, the test names are unnecessary and at worst can add confusion. I agree that test names are comments, but I have seen usefulness in that. When a test has a good name, when it fails I can tell what happened simply by looking at the test explorer; I don't need to dig into details to see what went wrong (at least, I don't need to dig into the test details nearly as often).

I would liken test names more to variable names than comments. When we have useful variable names, it enhances the readability of our code. This is why I'll often include an intermediate variable. Then I can include some "comments" about what it is doing by giving it a good name, even if the code itself is not that hard to understand.

The Last Test
While I like most of the techniques presented, I wasn't a big fan of the final test state. His use of Test Data Builders throughout the book were interesting, and they gave me some ideas that I would like to work through. But the conclusion was a bit of a logical extreme:


Here's the text

public class CustomerTest {
  @Test
  public void chargeForTwoRentals() {
    assertMoney(
      5.7,
      a.customer.build().addRentals(
        create(
          stub(Rental class)
            returning(a.money.w(2.2).build())
            .from().getCharge()),
        create(
          stub(Rental class)
            returning(a.money.w(3.5).build())
            .from().getCharge()))
      .getTotalCharge());
  }
}

For someone walking up to this test for the first time, it is a bit confusing. I think it's because the Arrange, Act, and Assert all get mixed together. Fields promotes this as a good choice because it does follow "assert last" (although it's basically crammed the entire test into the assertion).

To pick this apart a bit, the behavior that we're testing (the "Act") is at the very end: "getTotalCharge()". The "Arrange" is really everything after "a.customer.build()..." which creates an object with test data so that we can call "getTotalCharge()" on a populated object. The "Assert" is the last step, but also the first step (which is weird in my mind).

I think that the test data builders can be very useful, but I would really like to see a standard "Arrange/Act/Assert" layout with some intermediate variables for readability.

Wrap Up
While Working Effectively with Unit Tests by Jay Fields does have some good advice, I think the drawbacks keep me from making it a recommendation. For general testing advice, I'd still go with The Art of Unit Testing by Roy Osherove.

Happy Coding!

No comments:

Post a Comment