The Absent Test Body

Screen Shot 2018-10-20 at 18.24.39

Consider the following tests:

JUnit

@Test
public void hasCorrectFoo() {
  checkFoo();
}

private static checkFoo() {
  Foo foo = service.getFoo();
  assertThat(foo.getBar()).isEqualTo("baz");
}

Mocha

const checkFoo = () => {
   const foo = getFoo();
   expect(foo.bar).to.equal('baz');
};

it('has correct foo', () => {
  // makes you wonder why this test even has {} in its body
  checkFoo();
});

As you can see from both examples, there’s a microscopic one line test case, which delegates its entire work to a completely different method. Why would you do that?

As you might imagine from the existence of this article, you shouldn’t do that, but here are some possible intents that might lead to the above:

  • Several tests have a similar body, so there’s been some DRYing out
  • The body of the test was getting large, so it was shunted out of the way to make the tests read better
  • It was like that in other examples, so we copied

There seem to be no positive reasons for writing tests this way. It’s more the sort of thing that might happen.

I’ve worked with coding standards that specifically prevent this, and I recommend such a coding standard. It’s got to be an anti-pattern. In practice there’s a fine line to draw between two extremes:

  • Stop rewriting the same test code, at length over and over again
  • Make a test method pretty self contained and easy to follow – more of a script than perfectly Once and Once Only implementation

The line comes in at readability. A file that’s long and very repetitive, with hard to spot nuances between the repetition would benefit from a bit more refactoring, so that the test bodies look a little more declarative. On average, hold back on the refactoring compared to production code.

Where you have several tests all using the same function, slightly differently, what you really have is a parameterised test and it should be replaced with one.

E.g.

it('works for en', () => {
  check('en');
});

it('works for fr', () => {
  check('fr');
});

// replaced with

// the parameterised test pattern from mocha
['en','fr'].forEach((lang) => {
  it(`works for ${lang}`, () => {
     // inline the check here, using lang
  };
});
Advertisements

One comment

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 )

Google+ photo

You are commenting using your Google+ 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 )

Connecting to %s