Testing callback functions

I’ve been doing JavaScript development daily for several years now and been teaching it for about ~2~8+ years. And still, I’m stumped on some of the basic concepts in the language. Today it was asynchronous code with callbacks. Again.

Callbacks, why does it always have to be callbacks.

But I thought that if I spent some time here and try to understand why it would lead to me understand of callbacks and asynchronous code execution even better. I was not wrong.

In this particular case, we were surprised about why a mocha unit test did not only fail but crashed the test run.

Here’s the little code that I want to test:

function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(() => callback('toCheck was false', undefined), 100);
  }
  setTimeout(() => callback(undefined, 'toCheck was true'), 100);
}

// Example call
errorIfFalse(true, (err, data) => {
  if(err) {
    console.log("FAIL");
    console.log(err);
    return;
  }
  console.log("GREAT SUCCESS");
  console.log(data);
})

Small explanation…

This code checks if toCheck is false or not:

• If it is false it will call the callback-function, after waiting for 100 ms, passing the string 'toCheck was false' as the err, and undefined as data
• if toCheck is true, it will call the callback-function, after waiting for 100 ms, passing undefined as err and 'toCheck was true' as data

This mimics how many original Node APIs looks, for example fs.ReadFile() </small>

It’s a simplification of another code snippet from a programming test, but this works. There’s a couple of errors in this code that led us to find the problem, but humor me while we talk through it.

The only thing to notice now is that you can call setTimeout like that, passing the argument to the function to call after the timeout, last. Look for yourself. This is part of the problem but we will get there.

Being a great (?) developer I naturally wanted to write some tests for my advanced code. Just to get a feel for how client code would use it. Here’s a passing first test:

/* global describe, it */
function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(() => callback('toCheck was false', undefined), 100);
  }
  setTimeout(() => callback(undefined, 'toCheck was true'), 100);
}

const assert = require('assert')

describe('callbacks that calls back too many times', () => {

  it('does work', (done) => {
    errorIfFalse(true, (err, data) => {
      assert.strictEqual(err, undefined)
      assert.notStrictEqual(data, undefined)
      done()
    })
  })
})

(You can paste this into a tests.js and then go npx mocha . to run it)

Ok - that works fine. One test passing - I’m just checking that the error is null and the data is not.

Let’s write a test for the opposite, error case:

// ... as before

describe('callbacks that calls back too many times', () => {
    // ... as before

  it('does fail', (done) => {
    errorIfFalse(false, (err, data) => {
      assert.notStrictEqual(err, undefined)
      assert.strictEqual(data, undefined)
      done()
    })
  })
})

Running this and this will… Fail?! Not only that - we get 3 errors statuses reported:

  callbacks that calls back too many times
    ✓ does work (103ms)
    ✓ does fail (100ms)
    1) does fail


  2 passing (212ms)
  1 failing

Wahaaaah? How can this be? 3 tests. I have only written 2. And one of those tests that I haven’t written is failing? Did I just enter another dimension?

Let’s fix the production code first and see if that teaches us something.

The correct code

The code is wrong since, in the case of toCheck being false-y, we will call the callback twice. Can you see that?

function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(() => callback('toCheck was false', undefined), 100);
  }
  setTimeout(() => callback(undefined, 'toCheck was true'), 100);
}

There are a couple of ways to solve the problem - first using a single well place return to break out of the function:

function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(callback, 100, 'toCheck was false', undefined)
    return
  }
  setTimeout(callback, 100, undefined, 'toCheck was true')
}

Rerunning the test will now show 2 passing and 0 failing. Good. 1 + 1 is back to being 2. Thank you, gods, of Math and Logic for cooperating in the brotherly harmony once again. I have faith in the world again.

The reason this failed is easy to see once you step through the code. Without that return-statement, if the toCheck is false-y we will first call setTimeout once (error parameter set and data undefined) Then we will call setTimeout again. This time with the error undefined and the data set.

By introducing the return we break out of the function after the first setTimeout call.

We could also have written the code like this, using dreadful else-statements:

function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(() => callback('toCheck was false', undefined), 100);
  }
  else {
    setTimeout(() => callback(undefined, 'toCheck was true'), 100);
  }
}

Which also protects us in the same way - we are now only calling setTimeout once.

Both of these works and doesn’t cause the double calls to the test.

So, now we know what was wrong in the test… but why did it break the test run?

Why did the funky code break the test run?

Let’s go back to the original code and run the tests again:

function errorIfFalse(toCheck, callback) {
  if (!toCheck) {
    setTimeout(() => callback('toCheck was false', undefined), 100);
  }
  setTimeout(() => callback(undefined, 'toCheck was true'), 100);
}

Notice that we get 2 passing and 1 failing. Yes - because we have written that bad code, without the return.

But … now move the it('does work') {} test below the other test and rerun. Like this:

describe('callbacks that calls back too many times', () => {

  it('does fail', (done) => {
    errorIfFalse(false, (err, data) => {
      assert.notStrictEqual(err, undefined)
      assert.strictEqual(data, undefined)
      done()
    })
  })

  it('does work', (done) => {
    errorIfFalse(true, (err, data) => {
      assert.strictEqual(err, undefined)
      assert.notStrictEqual(data, undefined)
      done()
    })
  })
})

WHAT!? 1 passing and 1 failing. It’s Schrödinger’s test all over.

Let’s add some logging to that case again to try to clarify:

it('does fail', (done) => {
  let i = 0
  errorIfFalse(false, (err, data) => {
    i += 1
    console.log(`Called for the ${i}th time`)
    console.log(`err  = ${err}`)
    console.log(`data = ${data}`)
    assert.notStrictEqual(err, undefined)
    assert.strictEqual(data, undefined)
    done()
  })
})

Now when we run the tests we get the following print out:

Called for the 1th time
err  = toCheck was false
data = undefined
    ✓ does fail (107ms)
Called for the 2th time
err  = undefined
data = toCheck was true
    1) does fail

Which makes sense. It gets called twice, due to our missing return or else or whatever. See how it gets called twice. And the err and data are reversed. And that’s why the asserts fail.

Just for fun - let’s take out the asserting and see if that get’s both cases to pass.

it('does fail', (done) => {
    let i = 0
    errorIfFalse(false, (err, data) => {
      i += 1
      console.log(`Called for the ${i}th time`)
      console.log(`err  = ${err}`)
      console.log(`data = ${data}`)
      // assert.notStrictEqual(err, undefined)
      // assert.strictEqual(data, undefined)
      done()
    })
  })

Wait? What?!

  2 passing (215ms)
  1 failing

  1) callbacks that calls back too many times
       does fail:
       Error: done() called multiple times

That fails too, but with another error.

Ha! Of course. The setTimeout will trigger a call to the callback function, that is defined as

(err, data) => {
      i += 1
      console.log(`Called for the ${i}th time`)
      console.log(`err  = ${err}`)
      console.log(`data = ${data}`)
      // assert.notStrictEqual(err, undefined)
      // assert.strictEqual(data, undefined)
      done()
    }

Which hence calls done() twice for this test. This fails the test run - as mocha is setup.

Conclusion

As always, and promised, I learned a lot about the flow of asynchronous code by walking through this. I hope you did too.

In this particular case I learned that calling a callback method doesn’t break the flow of the code. Our code will still continue to run after the callback is called. There’s nothing special with a callback function, it’s just another function (that we happen to pass as a parameter)

So calling return is still important, to break the flow and not call your callback twice.