TDD anti patterns - Chapter 1

Matheus Marabesi

Matheus Marabesi

See author's bio and posts

Test Driven Development (TDD) has been adopted across developers and teams to deliver quality code through short feedback cycles in a (fail-pass-refactor) fashion. Kent Beck popularized the methodology that became a standard for the industry to follow and build upon. Therefore, starting with TDD is not easy and keeping it sharp so the test suite runs fast is even more complicated. Codebases that tackle business problems require a non-trivial amount of code, and with that, a non-trivial set of test cases. Therefore, as this experience report describes, the industry has an immature test automation process leading to slow feedback.

At Codurance, we are aware of that, and we try to push the industry forward. For example, in Codurance Talks episode 31, Javier Martínez Alcántara, Alasdair Smith and Jose Enrique Rodríguez Huerta discuss building a testing culture.

Along those lines, James Carr came up with a list of anti-patterns to look at and keep under control to avoid the testing trap that extensive codebases might fall into (thus having slow feedback). Dave Farley went through a few of them on his youtube channel, and as such, it made me realize that I had to spread the word on this subject. This post is inspired by Dave Farley's video, a companion for the meetup hosted at Codurance around TDD anti-patterns and a way to share the data from the TDD anti-patterns survey conducted before the meetup.

Survey notes

Along with the meetup, we surveyed to gather data around the TDD-antipatterns in the industry, and from practitioners worldwide. We got 22 answers, and the data shows that practitioners who answered the survey worked for projects in South America: Argentina, Brazil, and Mexico and in Europe: France, Hungary, Ireland, Portugal, Spain and Romania.

We also found that the most popular programming languages that respondents work with professionally are:

  • Javascript
  • PHP
  • Java
  • Typescript
  • Python

 

You will find that most of the examples used in the following sections are in javascript as well. We think this way will be easier to reach a broader audience and give back to those that answered the survey. On the other hand, we also see other languages showing up in the survey that are not as popular: Ruby, Rust and Groovy.

We also found that practitioners usually informally learn TDD, meaning that from the responses, more than 50% of them learned TDD alone through video courses, books or tutorials. Following the same trend, only 50% of the companies understand the pros and cons of TDD and use it as a practice. 

Excessive setup

According to the survey data, the Excessive setup is the 3rd most popular tdd anti-patterns.

I relate to excessive setup due to the non-practice of TDD from the start and also the lack of practising object callisthenics - it would also apply for functional programming. Still, for the time being, I will stay with object-oriented programming.

The classic approach for excessive setup is when you want to test a specific behaviour on your codebase. It becomes difficult due to the many dependencies you have to create beforehand (such as classes, operating system dependencies, databases - basically anything that removes the attention to the testing goal).

The following code depicts the nuxtjs framework test case for this matter, the test file for the server starts with a few mocks, and then it goes on till the method beforeEach that has more setup work (the mocks).

 

jest.mock('compression')

jest.mock('connect')

jest.mock('serve-static')

jest.mock('serve-placeholder')

jest.mock('launch-editor-middleware')

jest.mock('@nuxt/utils')

jest.mock('@nuxt/vue-renderer')

jest.mock('../src/listener')

jest.mock('../src/context')

jest.mock('../src/jsdom')

jest.mock('../src/middleware/nuxt')

jest.mock('../src/middleware/error')

jest.mock('../src/middleware/timing')

The code above points to a few dependencies that the test case is mocking to take more fine-tuned control, then the code follows:

 

describe('server: server', () => {

  const createNuxt = () => ({

    options: {

      dir: {

        static: 'var/nuxt/static'

      },

      srcDir: '/var/nuxt/src',

      buildDir: '/var/nuxt/build',

      globalName: 'test-global-name',

      globals: { id: 'test-globals' },

      build: {

        publicPath: '__nuxt_test'

      },

      router: {

        base: '/foo/'

      },

      render: {

        id: 'test-render',

        dist: {

          id: 'test-render-dist'

        },

        static: {

          id: 'test-render-static',

          prefix: 'test-render-static-prefix'

        }

      },

      server: {},

      serverMiddleware: []

    },

    hook: jest.fn(),

    ready: jest.fn(),

    callHook: jest.fn(),

    resolver: {

      requireModule: jest.fn(),

      resolvePath: jest.fn().mockImplementation(p => p)

    }

  })

 

At this stage, we know that we are creating a nuxt application and injecting different mocks. So the next bit glues everything together:

 

beforeAll(() => {

  jest.spyOn(path, 'join').mockImplementation((...args) => `join(${args.join(', ')})`)

  jest.spyOn(path, 'resolve').mockImplementation((...args) => `resolve(${args.join(', ')})`)

  connect.mockReturnValue({ use: jest.fn() })

  serveStatic.mockImplementation(dir => ({ id: 'test-serve-static', dir }))

  nuxtMiddleware.mockImplementation(options => ({

    id: 'test-nuxt-middleware',

    ...options

  }))

  errorMiddleware.mockImplementation(options => ({

    id: 'test-error-middleware',

    ...options

  }))

  createTimingMiddleware.mockImplementation(options => ({

    id: 'test-timing-middleware',

    ...options

  }))

  launchMiddleware.mockImplementation(options => ({

    id: 'test-open-in-editor-middleware',

    ...options

  }))

  servePlaceholder.mockImplementation(options => ({

    key: 'test-serve-placeholder',

    ...options

  }))

})

 

Reading the test from the beginning gives an idea that to start with, there are 13 jest.mock invocations. Besides that, there are more setup on the beforeEach, around nine spies and stub setup. Probably, if I wanted to create a new test case from scratch or move tests across different files, I would need to keep the excessive setup as it is now - you know, the excessive setup.

 

The excessive setup is a common trap. I also felt the pain of having to build many dependencies before starting testing a piece of code. The following code is from my research project called testable - it's a reactjs piece of code that is oriented to functional style:

 

import { Component } from 'react';

import PropTypes from 'prop-types';

import { Redirect } from 'react-router';

import Emitter, { PROGRESS_UP, LEVEL_UP } from '../../../../packages/emitter/Emitter';

import { track } from '../../../../packages/emitter/Tracking';

import { auth } from '../../../../pages/login/Auth';

import Reason from '../../../../packages/engine/Reason';

import EditorManager from '../editor-manager/EditorManager';

import Guide from '../guide/Guide';

import Intro from '../intro/Intro';

import DebugButton from '../../buttons/debug/Debug';

import {SOURCE_CODE, TEST_CODE} from '../editor-manager/constants';

import {executeTestCase} from '../../../../packages/engine/Tester';

 

const Wrapped = (

  code,

  test,

  testCaseTests,

  sourceCodeTests,

  guideContent,

  whenDoneRedirectTo,

  waitCodeToBeExecutedOnStep,

  enableEditorOnStep,

  trackSection,

 

  testCaseStrategy,

  sourceCodeStrategy,

 

  disableEditor,

  introContent,

  enableIntroOnStep,

  editorOptions,

  attentionAnimationTo = []

 ) => {

  class Rocket extends Component {

   // code that has component logic

  }

}



The excessive number of parameters to test the function are so many that if anyone (or myself) starts to write a new test case, we would forget what to inject into the function to receive the desired result.

 

Dave Farley shows another example from Jenkins, an open-source ci/cd project. He depicts a particular test case that builds up a web browser to assert the URL being used is a development one:

 

/**

* Makes sure the use of "localhost" in the Hudson URL reports a warning.

*/

@Test

public void localhostWarning() throws Exception {

    HtmlPage p = j.createWebClient().goTo("configure");

    HtmlInput url = p.getFormByName("config").getInputByName("_.url");

    url.setValueAttribute("http://localhost:1234/");

 

    assertThat(

        p.getDocumentElement().getTextContent(),

        containsString("instead of localhost")

    );

}



He argues that the test case creates a web client for asserting the URL, which could have been a regular object call and assert on the returned value, avoiding the web client creation.

 

The excessive setup can show in different contexts. For a given situation, the question is: are we focused on testing the piece of code that we need or are we spending time setting up the scenario? If the answer is towards the second option, then you have a candidate of being excessive.

 

The liar

The liar is the 4th most popular tdd anti-patterns according to the survey data.

 

The liar is one of the most common anti-patterns that I can recall in my professional life practising TDD. I would list at least those two reasons for me to spot such issues among codebases:

 

  1. Async oriented test cases
  2. Time oriented test cases

 

The first one is well explained in the official jest documentation. Testing asynchronous code becomes tricky as it is based on a future value that you might receive or might not. The following code is a reproduced example from jest documentation depicting the problem related to async testing.

 

// Don't do this!

test('the data is peanut butter', () => {

  function callback(data) {

    expect(data).toBe('peanut butter');

  }

 

  fetchData(callback);

});

 

Getting back to the anti-pattern, this test would pass without complaining, you know, the liar. The correct approach is to wait for the async function to finish its execution and give jest control over the flow execution again; this is done by invoking a parameter that jest injects when running the tests. In the code example that follows, this parameter is called done.

 

test('the data is peanut butter', done => {

  function callback(data) {

    try {

      expect(data).toBe('peanut butter');

 

      done(); // <------ invokes jest flow again, saying: "look I am ready now!"

 

    } catch (error) {

      done(error);

    }

  }

 

  fetchData(callback);

});



The second one, Martin Fowler, elaborates on the reasons for that to be the case, and here I would like to share some opinions that go along with what he wrote.

 

Asynchronous is a source of non-determinism, and we should be careful with that (as we just explored in the previous example). Besides that, another candidate for such non-determinism are threads.

On the other hand, time-oriented tests sometimes can fail for no reason. For example, I have experienced test suites that failed because the test wasn't using a mock to handle dates. As a result, on the day that the code was written, the test was passed, but on the following day, it broke. You know, the liar again.

 

The giant

According to the survey data, the giant is the 5th most popular tdd anti-patterns.

The giant is also a sign of lack in the design of the codebase. Designing code is also a subject of discussion among TDD practitioners. Sandro Mancuso discussed the relationship between TDD and good design - Does TDD Really Lead to Good Design?

As opposed to the excessive setup, I would say that this anti-pattern can also happen while developing in a TDD fashion. For example, Javier Chacana explored the TPP(Transformation Priority Premise) concepts that show small transformations done in the source code to avoid mirroring the test code with the production code. The giant is often related to the God class, which goes against SOLID principles.

In TDD, the giant often relates to many assertions in a single test case, as Dave Farley depicts in his video. The same test file used from nuxtjs in the excessive setup shows signals of the giant. Inspecting the code closer, we see fifteen assertions for a single test case:

 

test('should setup middleware', async () => {

    const nuxt = createNuxt()

    const server = new Server(nuxt)

    server.useMiddleware = jest.fn()

    server.serverContext = { id: 'test-server-context' }

 

    await server.setupMiddleware()

 

    expect(server.nuxt.callHook).toBeCalledTimes(2) // 1

    expect(server.nuxt.callHook).nthCalledWith(1, 'render:setupMiddleware', server.app) // 2

    expect(server.nuxt.callHook).nthCalledWith(2, 'render:errorMiddleware', server.app) // 3

 

    expect(server.useMiddleware).toBeCalledTimes(4) // 4

    expect(serveStatic).toBeCalledTimes(2) // 5

    expect(serveStatic).nthCalledWith(1, 'resolve(/var/nuxt/src, var/nuxt/static)', server.options.render.static) // 6

    expect(server.useMiddleware).nthCalledWith(1, {

      dir: 'resolve(/var/nuxt/src, var/nuxt/static)',

      id: 'test-serve-static',

      prefix: 'test-render-static-prefix'

    }) // 7

    expect(serveStatic).nthCalledWith(2, 'resolve(/var/nuxt/build, dist, client)', server.options.render.dist) // 8

    expect(server.useMiddleware).nthCalledWith(2, {

      handler: {

        dir: 'resolve(/var/nuxt/build, dist, client)',

        id: 'test-serve-static'

      },

      path: '__nuxt_test'

    }) // 9

 

    const nuxtMiddlewareOpts = {

      options: server.options,

      nuxt: server.nuxt,

      renderRoute: expect.any(Function),

      resources: server.resources

    }

    expect(nuxtMiddleware).toBeCalledTimes(1) // 10

    expect(nuxtMiddleware).toBeCalledWith(nuxtMiddlewareOpts) // 11

    expect(server.useMiddleware).nthCalledWith(3, {

      id: 'test-nuxt-middleware',

      ...nuxtMiddlewareOpts

    }) // 12

 

    const errorMiddlewareOpts = {

      resources: server.resources,

      options: server.options

    }

    expect(errorMiddleware).toBeCalledTimes(1) // 13

    expect(errorMiddleware).toBeCalledWith(errorMiddlewareOpts) // 14

    expect(server.useMiddleware).nthCalledWith(4, {

      id: 'test-error-middleware',

      ...errorMiddlewareOpts

    }) // 15

  })



The point of attention here is to reflect if breaking down each block of code and assertion on its test case makes sense. It would require further inspection to double-check if it's possible to break the code as suggested. As Dave Farley depicts in his video, it makes an excellent example of the giant that this practice is not recommended.

 

The slow poke

According to the survey data, the slow poke is the 6th most popular tdd anti-patterns. Slow poke reminds me of pokemon, and like the creature, the slow poke takes down the efficiency of the test suite. Usually, it puts the test suite to execution and takes longer to finish and give the developer feedback.

One cause for that is time-related code, which is difficult to handle under a test case; It involves manipulating it in different ways.

For example, if we are dealing with payment systems, we would like to trigger some routine to launch a payment at the end of the month. For that, we would need a way to handle time and check for a specific date (the last day of the month) and time (somewhere around the morning? Evening?). In other words, we need a way to handle the time and deal with it without the need of waiting until the end of the month to run the test - or even worse, would you leave your test running for a month to get feedback?.

Code related to time leads to non-determinism, as already mentioned in the "The liar" section. But, lucky us, there is a way to overcome this situation with mocking.

 

Pushing towards integration tests or end-to-end tests can also transform the test suite into a pain to run, taking long hours or even days. This approach is also related to the ice cream cone instead of the pyramid of the test. In an ideal scenario, you would have as the base more unit tests, a few integration tests and even less from end-to-end tests.

In my experience, the slow poke comes in two different flavours:

 

  1. Lack of knowledge in TDD - more specifically in mocking strategies
  2. Testing with strategies that the framework already gives out of the box

 

Final considerations

 

Testing and TDD is a practice that developers have adopted and practised for a few years now. Therefore there is still room for improvement. Starting from the anti-patterns that were explored here. In short:

 

  • Time async behaviour needs extra care when writing tests.
  • Poor design of the code can lead to excessive setup and giant test cases, making it harder to test
  • Having a slow test suite can lead to frustration for developers. Keep the test suite as fast as possible to improve the feedback loop.

 

James Carr enumerated 22 anti-patterns, and there is no source of truth, as in comparison, we can find different numbers for different sources. Here, we went over four of them, which means that there are a lot more to go over; each anti-pattern in the list can be explored in different contexts and programming languages - the context is important for those anti-patterns.

 

Besides that, we also got some information about the practitioners in the field, which anti-patterns they know the most, which programming languages they use. Hopefully, we also leave some insights on how to move forward with this subject. 

 

Keep it sharp, and keep testing!