Programming

Counterargument: Don’t put logic in tests

I consider test suites to be first class code. They need to be clean, adaptable, and frequently refactored just like the primary code. Thus I was quite bothered to see the article Don’t Put Logic in Tests that wants to reduce tests to spreadsheets of input and output variables. If I followed this approach my tests suites would be monstrosities!

The example

The article uses the below example as a “bad” test case:

1
2
3
4
5
6
@Test public void shouldNavigateToPhotosPage() {
    String baseUrl = "http://plus.google.com/";
    Navigator nav = new Navigator(baseUrl);
    nav.goToPhotosPage();
    assertEquals(baseUrl + "/u/0/photos", nav.getCurrentUrl());
}

And gives this better version:

1
2
3
4
5
@Test public void shouldNavigateToPhotosPage() {
    Navigator nav = new Navigator("http://plus.google.com/");
    nav.goToPhotosPage();
    assertEquals("http://plus.google.com//u/0/photos", nav.getCurrentUrl());
}

The primary reason for this version is that the double slash //u defect is clearer. I don’t know, but I suspect it’d also be clear from the error message that assertEquals should be producing:

1
"http://plus.google.com//u/0/photos" != "http://plus.google.com/u/0/photos"

I could quickly fix the test and move on. But this isn’t the basis for my dislike of this “improved” test.

Distraction

I find the second version of the test less clear. Consider if I wrote a test like this instead, which assumes the test suite has a baseURL variable:

1
2
3
4
5
@Test public void shouldNavigateToPhotosPage() {
  Navigator nav = new Navigator( baseURL );
  nav.goToPhotosPage();
  assertEquals( baseURL + "u/0/photos", nav.getCurrentUrl());
}

The precise value of baseURL is not an important detail of this test. It does have to be equal, but that’s it. In the version that has the complete string, twice, I’m left wondering if it is somehow a key part of the test.

I even consider the new Navgiator part to be superfluous. My test isn’t about testing creation of this object, it’s named shouldNavigateToPhotosPage. I would refactor to get the test looking like this:

1
2
3
4
5
@Test public void shouldNavigateToPhotosPage() {
  Navigator nav = CreateNavigatorAtHome();
  nav.goToPhotosPage();
  assertOnPage( "u/0/photos", nav );
}

I’ve elevated my test case one level higher. CreateNavigatorAtBase just says I need a Navigator starting at the home page, but the detail of how to get that isn’t relevant. The assertOnPage also makes it clear that I’m trying to test navigation to a proper page, not just a URL. The URL is merely an indicator of being on the proper page, it isn’t actually what I want to test.

Not reusable

The second problem of hardcoding the URL is the test is no longer reusable. Web sites often have multiple URLs, in particular an HTTPS version, or a mobile friendly site. I expect a test like shouldNavigateToPhotosPage to perform the same on any version of the site. By abstracting away details I can do exactly that: call this function multiple times, each with a different version of the site.

I write tests to be compliance suites, and abstraction really helps. By making this test use only high-level concepts I get more places to hook in and do further testing. It also lets me write new feature tests without getting bogged down in details first.

For example, on one of my projects my assertOnPage logic became essentially a distinct test suite. We checked the titles, URLs, menus, and other content. Where possible user customizations were also checked. The verifier worked in a hierarchy of pages to ensure minimal code for maximum coverage. We were able to improve the quality of our test suite without always having to touch the individual high-level tests.

Bad Example?

I really hope the intent of the original article’s author was different than their blog presented. Perhaps the code example just misrepresents what they are trying to say about clarity.

I want my tests to be focused and clear. When I read a test I want to be able to identify what feature is actually being tested without being distracted by details. I don’t want to have to touch tests just because some minor details in the program changed. As much as possible I’d like to equate changes in the highest-level tests as being changes in functionality. A change in the test is then a good trigger for doing a manual review of the feature.

If you follow the advice of the original article you’ll eventually have massive, hard to maintain, and tedious to program suites of dubious quality. Instead focus on high level intent, refactoring the tests as necessary to get it. You’ll be able to get better coverage and code that people are happy to work on.

Categories: Programming, Response

Tagged as: , ,

2 replies »

  1. You did exactly what was suggested in the article you’re responding to:

    > When tests do need their own logic, such logic should often be moved out of the test bodies and into utilities and helper functions.

  2. Twins game is about to start, so I’ll have to come back and read the cited article, but the cited example you provided hugely violates a key principle for me: Never duplicate constants, especially string constants.

    While I’ll grant the double-slash bug would be more noticeable during writing, it’s equally possible a typo results in two distinct URLs (which is exactly why you don’t duplicate string constants). Depending on how far away the two strings were in the source, the difference might not be noticeable, and might be pretty hard to spot during testing. As you point out, the double-slash issue will become immediately apparent.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s