Failing test - Mocha's done() called multiple times

Question

I've tried looking at topics with a similar error but could not fit those solutions into the context of my issue.

When I try to run the the following test (function included that is tested):

function myFunc(next, obj) {
  const pairs = {};
  obj.listing.forEach((element) => {
    if (element.x in pairs && pairs[element.x] !== element.y) {
      const err = new Error('This was not ok');
      next(err);
    } else {
      pairs[element.x] = element.y;
    }
  });
  next();
}

it('should fail as 9 has been changed to 5 in the second object of the listing', function (done) {
  const callback = (err) => {
    if (err && err instanceof Error && err.message === 'This was not ok') {
      // test passed, called with an Error arg
      done();
    } else {
      // force fail the test, the `err` is not what we expect it to be
      done(new Error('Assertion failed'));
    }
  }
  myFunc(callback, {
    "listing": [
      { "x": 5, "y": 9 },
      { "x": 5, "y": 11 }
    ]
  });
});

I get this error: What is the cause of this and how can I fix it?

score 0 · Answer 1 · answered Oct 31 '19 at 07:39

0

You need to add a return in the if block of your myFunc so that the callback function next is called only once and indeed the done() callback in the main test case:

function myFunc(next, obj) {
  const pairs = {};
  obj.listing.forEach((element) => {
    if (element.x in pairs && pairs[element.x] !== element.y) {
      const err = new Error('This was not ok');
      return next(err);
    } else {
      pairs[element.x] = element.y;
    }
  });
  next();
}

answered Oct 31 '19 at 07:39

Ankit Agarwal

30,378
5
37
62

I just tried this (also tried adding a return to both calls to next()) but it still throws the same error. – SomeDutchGuy Oct 31 '19 at 07:55
Your answer did push me in the right direction. I just posted the solution to the problem. – SomeDutchGuy Oct 31 '19 at 09:27

score 0 · Accepted Answer · answered Oct 31 '19 at 09:26

@Ankif Agarwal's solution was not the correct one but it did point me in the right direction.

The forEach() method is not short circuited and therefor makes a call to next() more than once (Short circuit Array.forEach like calling break).

I was able to solve this in one of two way's.

By extracting the call to next() from the forEach() logic:

function myFunc(next, obj) {
  const pairs = {};
  let err = null;
  obj.listing.forEach((element) => {
    if (element.x in pairs && pairs[element.x] !== element.y) {
      err = new Error('This was not ok');
    } else {
      pairs[element.x] = element.y;
    }
  });

  if (err !== null) {
    next(err);
  } else {
    next();
  }
}

However this still makes the forEach() run through all element. If possible it seems better to short circuit it and break out of it soon as a violation occurs that sets the error, like so:

function myFunc(next, obj) {
  const pairs = {};
  const BreakException = {};
  let err = null;
  try {
    obj.listing.forEach((element) => {
      if (element.x in pairs && pairs[element.x] !== element.y) {
        err = new Error('This was not ok');
        throw BreakException;
      } else {
        pairs[element.x] = element.y;
      }
    });
    next();
  } catch (e) {
    if (e !== BreakException) throw e;
    next(err);
  }
}

Hopefully someone can use this in the future.

Failing test - Mocha's done() called multiple times

2 Answers2