Skip to content

Instantly share code, notes, and snippets.

@jmerrifield
Created June 24, 2015 18:52
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save jmerrifield/6de1948480b3311be5b4 to your computer and use it in GitHub Desktop.
Save jmerrifield/6de1948480b3311be5b4 to your computer and use it in GitHub Desktop.
Be careful DRYing up test code

A test for a module performing trivial transformation on data from an external service:

it('returns the first element in the response', function () {
  var serviceResponse = [{a: 1}]
  serviceStub.returns(serviceResponse)
  
  expect(subject()).to.equal(serviceResponse[0])
})

This seems reasonable at first glance, and feels good because we've DRYed it up nicely, the test data isn't duplicated, but that comes at a high price in this case!

The subject can modify the value of serviceResponse. JS objects are mutable! In application code when dealing with a service response, it's common to mutate the response under the expectation that nothing else will refer to it:

request.get('/users/12345', function (err, res, body) {
  // Transform the user in-place, rather than return a new object, because why not?
  body.user.capitalizedName = body.user.name.toUpperCase()
  callback(null, body.user)
})

Unfortunately for our simple test code above, such a possibility means we really need to re-think the test.

it('returns the first element in the response', function () {
  var serviceResponse = [{a: 1}]
  serviceStub.returns(serviceResponse)
  
  expect(subject()).to.equal(serviceResponse[0])
})

// if this was the implementation of the subject, the test would pass:
var response = makeServiceCall() // gets the fake response [{a: 1}]
response[0].dbcredentials = [config.db.url, config.db.password]
delete response[0].a

return response[0]

The test asserts that the output is the same as the input at the time that the assertion is made. There is no guarantee that the object we used as input still looks anything like it did before we invoked the subject. We must break the object reference between the input and the output if we want to have confidence in this test:

it('returns the first element in the response', function () {
  serviceStub.returns([{a: 1}])
  
  expect(subject()).to.equal({a: 1})
})

This has the added benefit of being clearer - it's immediately obvious what the output should be, given a particular piece of input. This becomes even more important when you start to test more complex transformations. See the following different ways of testing a more complex transformation:

it('removes unneccessary data and adds a flag', function () {
  var response = {name: 'jon', favorite_color: 'blue', created_at: '2014-02-21'}
  
  // Too much DRY
  expect(subject()).to.deep.equal(_.extend(_.pick(response, 'name'), {success: true})
  
  // Straightforward literal expectation
  expect(subject()).to.deep.equal({name: 'jon', success: true})
})

In summary, try to keep your DRY instincts in check when writing test code! Don't be afraid to repeat test data, and don't make the person reading your tests jump through mental hoops just to understand what assertion you're actually making!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment