Thursday, July 31, 2014

Testing on the Toilet: Don't Put Logic in Tests

by Erik Kuefler

This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.

Programming languages give us a lot of expressive power. Concepts like operators and conditionals are important tools that allow us to write programs that handle a wide range of inputs. But this flexibility comes at the cost of increased complexity, which makes our programs harder to understand.

Unlike production code, simplicity is more important than flexibility in tests. Most unit tests verify that a single, known input produces a single, known output. Tests can avoid complexity by stating their inputs and outputs directly rather than computing them. Otherwise it's easy for tests to develop their own bugs.

Let's take a look at a simple example. Does this test look correct to you?

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

The author is trying to avoid duplication by storing a shared prefix in a variable. Performing a single string concatenation doesn't seem too bad, but what happens if we simplify the test by inlining the variable?

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

After eliminating the unnecessary computation from the test, the bug is obvious—we're expecting two slashes in the URL! This test will either fail or (even worse) incorrectly pass if the production code has the same bug. We never would have written this if we stated our inputs and outputs directly instead of trying to compute them. And this is a very simple example—when a test adds more operators or includes loops and conditionals, it becomes increasingly difficult to be confident that it is correct.

Another way of saying this is that, whereas production code describes a general strategy for computing outputs given inputs, tests are concrete examples of input/output pairs (where output might include side effects like verifying interactions with other classes). It's usually easy to tell whether an input/output pair is correct or not, even if the logic required to compute it is very complex. For instance, it's hard to picture the exact DOM that would be created by a Javascript function for a given server response. So the ideal test for such a function would just compare against a string containing the expected output HTML.

When tests do need their own logic, such logic should often be moved out of the test bodies and into utilities and helper functions. Since such helpers can get quite complex, it's usually a good idea for any nontrivial test utility to have its own tests.

13 comments:

  1. Although I agree with you alot, this example introduces some problems dealing with multiple enviroments.

    What if I want to move my site to a new domain. Do I need to change all my tests? What about dev-enviroment, test-enviroment and production-enviroment?

    Defining the base url at the top of the file, or even better, in a config file, would make my life alot easier.

    ReplyDelete
    Replies
    1. This is a unit test, not a functional test. The logic doesn't change based on the parameters. This code would work in all these environments w/o actually testing it in each environment. Your argument would suggest that to fully test this method you would test it against every domain.

      Delete
  2. I like the way article is explained. Simple and crisp!

    ReplyDelete
  3. I agree with the concept of not putting a lot of logic in the test but not quite with the example. This makes the test limited to just one instance of the App

    JumpToUrl(ConstantsConfig.hhtp_protocol + BeanTestHelper.App + "csr." + ConstantsConfig.homepage_url );

    protocol - http/https
    App- dev,test,stage,prod
    homepage - google.ca , google.sg , google.com

    ReplyDelete
  4. "Since such helpers can get quite complex, it's usually a good idea for any nontrivial test utility to have its own tests."
    Let's test the tests :)

    ReplyDelete
  5. One way to look at this example is to say "In an attempt to apply DRY, we broke the test. In a unit test, obviousness is more important than DRY".

    I mostly agree, but I think we can do better.

    In unit tests, obviousness is more important than DRY, but DRY still matters. Maybe I can refactor my production code to keep things obvious *and* dry. If not, maybe I can factor out a test-only utility, which is itself unit-testable.

    ReplyDelete
  6. What strange advise. This runs completely counter to my experience. Unit tests should be written with the same high maintainability (and therefore Don't Repeat Yourself) requirements as production code. If you don't you end up with buggy, verbose and unmaintainable unit tests that put a drag on the project. I've written a blog post explaining why:

    http://andrewtrumper.blogspot.ca/2014/08/complex-logic-in-unit-tests.html

    ReplyDelete
  7. I'm usually the guy on my team that resists the temptation to extract out common test setup, but I would regard your example as just too trivial not to pull up.

    ReplyDelete
  8. I agree with the arguments of having the tests configurable between environments. For me I write all my tests in reference of running it locally, but when deployed on CI, they run against a deployed environment.

    But I do agree with reducing the amount of logic in your tests as possible. But where I draw a line is usually at branching instead of simple string concatenation. When you see a 'if' or 'switch' statement, that's when a test starts to smell bad.

    ReplyDelete
  9. I think this is a bad example. If you do not run your test to ensure it's green when written, then you do not need to write a test at all. This "simple" bug would be found instantly after the first test, whats the big deal?

    ReplyDelete
  10. I think a better example would be the use of many if/else statements to handle the differences between a mobile view and a desktop view of the same page within the page object. Vs. using a higher level abstraction level such as abstract factories.

    ReplyDelete

The comments you read and contribute here belong only to the person who posted them. We reserve the right to remove off-topic comments.