mocha: Error: Resolution method is overspecified.

This:

before(done => {
    return Promise
        .all([])
        .then(() => Model.insert(...)) // Bookshelf model returning a Bluebird Promise
        .then(() => done())
        .catch(done)
})

will result in an error Error: Resolution method is overspecified. Specify a callback *or* return a Promise; not both.

Docs say:

In Mocha v3.0.0 and newer, returning a Promise and calling done() will result in an exception, as this is generally a mistake:

The model call is resolving with a Promise.<Object> of the newly inserted entry, however if I omit .then(() => done()) then the test timeouts.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 91 (36 by maintainers)

Commits related to this issue

Most upvoted comments

async functions (babel) with done also break.

For anyone else who was driven crazy by this…

This works:

it("should work", async () => {
  await something;
});

This does not:

it("should work", async (done) => {
  await something;
});

You must remove done as a parameter.

Here’s an example of both async (essentially a promise) and done that breaks:

it('fires change event when calling blah.doSomethingThatFiresChange', async function (done) {
  const blah = await getBlah()
  blah.on('change', () => done())
  blah.doSomethingThatFiresChange()
})

Is there a workaround for this when using babel?

I wrap twice:

it("should work", done => {
  (async () => {
    await something;
    done();
  })();
});

@artyomtrityak How about embracing promises fully? Then you can shorten your test to:

it('should fill userName and password', async function () {
    const userNameField = global.driver.wait(until.elementLocated({css: '#username'}));
    userNameField.sendKeys('admin');

    const passNameField = global.driver.findElement({css: '#password'});
    passNameField.sendKeys('*******');

    const userNameVal = await userNameField.getAttribute('value');
    const passwordVal = await passNameField.getAttribute('value');

    assert.equal(userNameVal, 'admin');
    assert.equal(passwordVal, 'changeme');
  });

In case some body is still struggling…

Is there a workaround for this when using babel?

Now with the built in handler for promise as mention in #2509 , we don’t have to use the wrapping twice hack like this:

it("should work", done => {
  (async () => {
    await something;
    done();
  })();
});

In lieu of that, we can simply go with this:

it("should work", async () => {
  await something;
});

Check this post for more detail

@elado That’s an interesting use case, although from mocha’s point of view it’s the same situation – a function that both takes a done callback and returns a promise. I’d rewrite it to be fully promise-based:

it('fires change event when calling blah.doSomethingThatFiresChange', async function () {
  const blah = await getBlah()
  return new Promise(resolve => {
    blah.on('change', resolve)
    blah.doSomethingThatFiresChange()
  })
})

… but I guess what you’re getting at is that in this example, it would be better if mocha waited for both the promise to be resolved and the callback to be called?

Unfortunately that particular combo is most often a bug in the test, which is why this error message was added in the first place.

@benfavre Thank you for the encouraging words that will most definitely motivate volunteers to take on the responsibility of implementing whatever solution you haven’t specified in their free time rather than playing with their kids

I don’t think this is a good idea. If one requests a callback I think they have made their intentions clear. The error here is just an annoyance. Please remove it.

6 months later, this is issue is still braking all modern js tests … shame

It’s not ambiguous, the callback was requested. How is that ambiguous?

As the error states, you’re not supposed to provide a function with an arity of >0 (meaning that it’s accepting a done callback), and return a Promise.

The easiest way to fix it would be to omit the return, but since you’re using promises I suggest getting rid of the done callback instead, as it’ll result in a much simpler construct:

before(() => Promise.all([]).then(() => Model.insert(...)));

@Munter No worries glad I could Help identify a specific issue I faced as a new user of mochajs. @SaschaNaz suggested solution of wrapping twice did not help.

=> Using promises exclusively did work as it should.

Next time I guess i should just “+1” like a morron so I don’t get insulted for free. There was nothing insulting in my message, furthurmore my statement remains true.

Most people will just choose another framework … as of right now, it’s just plain broken with async/await, with no clear indications anywhere on the main site, and no clear error message in the cli.

Happy new year and have fun playing with kids.

Upgraded to avoid vulnerabilities. Now I have to rewrite 99 tests. FML

The detractors have said their piece. Yet, the maintainers of Mocha disagree with the argument(s) for reverting this change. Eran Hammer said (paraphrased), “As a maintainer, one of the hardest things you can do is dissappoint those who want to move the work in their direction.”

I’m welcome to workarounds–more documentation (e.g. more examples of this error and how to fix them), better error messaging–but am not interested in drama, rudeness, or complaining. Contributing any of these workarounds to Mocha would help turn a negative into a positive.

If you don’t like this change, and simply can’t be constructive about it, it’s OSS–you may fork the project and revert the changes there.

As meintoned before babel async/await does not work well with new mocha@3. Example:

it('should fill userName and password', async function (done) {
    const userNameField = global.driver.wait(until.elementLocated({css: '#username'}));
    userNameField.sendKeys('admin');

    const passNameField = global.driver.findElement({css: '#password'});
    passNameField.sendKeys('*******');

    const userNameVal = await userNameField.getAttribute('value');
    const passwordVal = await passNameField.getAttribute('value');

    try {
      assert.equal(userNameVal, 'admin');
      assert.equal(passwordVal, 'changeme');
    } catch (error) {
      done(error);
      return;
    }

    done();
  });

This code works well with mocha@2 but does not with mocha@3 because inawait it returns promise

If you feel personally attacked, then I think you need to reconsider how emotionally invested you are in this conversation. None of the comments have been personal. I’m sure you’re all great, especially considering you’ve donated your time to help maintain this project which is greatly appreciated and very commendable.

Most of you (the currently active maintainers) started working on Mocha around mid-2014. Mocha was already established by the point you guys started contributing. It’s just my opinion, but I don’t think I’m alone in thinking one shouldn’t be making breaking changes to an established library unless it’s justified. Though I can imagine the original justification for this change, it doesn’t stand up well when one points out the following. Asking for a callback communicates a clear intention. Promises are not as clear because they are not requested, they are returned, which can happen indirectly and accidentally (returned from a 3rd party library for example). Because of these differences the two ways of yielding are not equal, and thus trying to use both is not really ambiguous. Callbacks must be written into the test arguments. You can’t do that with promises, and so by asking for a callback, you’ve communicated your intentions explicitly. Your community raised these concerns, and instead of acknowledging the misstep you guys are doubling down. Seems you’re even considering forcing tests to be async to ensure this error acts consistently. See => https://github.com/mochajs/mocha/pull/2413. Seems like a big change for an error message protecting against an unlikely mistake.

You guys have done a great job maintaining this library since @tj’s departure, can you please think a bit more about this change. My concern is this could compromise the library.

I don’t think he means you broke semver, I think he means you broke Mocha. This change doesn’t make things easier for developers, it’s enforcing an opinion.

beforeEach((cb) => connection.db.collection('users').remove({}, cb));
            ^^

^^ That is not ambiguous. It’s pretty clear what is expected by the author. The author has gone out of their way to request a callback.

So just to make clear what the problem is, as some are saying you shouldn’t need to use done and async, here is an example where you would want to use both.

it('should error on fn', async function(done) {
  try {
    await fn();
  } catch (e) {
    // assert some things here
    done()
  }
});

Not using done will let the test pass if no error is thrown. Some have suggested using things like expect(await fn).to.throw('blah'), but sometimes you need to check more properties than are going to fit into a one-liner.

Hey all, Ran into this issue with the following code;

describe('Page', ->
  describe('/GET home', ->
    it('it SHOULD return the homepage', (done) ->
      chai.request(app).get('/').end((err, res) ->
        res.should.have.status(200)
        res.text.should.be.a('string')
        done()
      )
    )
  )
)

Got this resolved by respecting the promise chain and omitting the callback done().

describe('Page', ->
  describe('/GET home', ->
    it('it SHOULD return the homepage', ->
      chai.request(app).get('/').then((res) ->
        res.should.have.status(200)
        res.text.should.be.a('string')
      )
    )
  )
)

I hope this helps others, who run into a similar error 😄

P.S. ❤️ Mocha btw 👍

EDIT Remove catch() based on @Munter’s comment.

For anyone else who was driven crazy by this…

This works:

it("should work", async () => {
  await something;
});

This does not:

it("should work", async (done) => {
  await something;
});

You must remove done as a parameter.

@tamoyal You saved my life ❤️

Hi guys, this one works well with async\await (just omit ‘done()’)

describe('New order from landing', function() {
    describe('check new client function', function () {
        it("must check db and return that random client is new", async function () {
            var result = await handler.checkClient(reqNewClient.body)
                expect(result).to.have.property('newclient').equal(true)
            })
    })
})

result:

New order from landing check new client function √ must check db and return that random client is new 1 passing (9 ms)

Here’s an example working for me

  describe('STRIPE CHARGES: Get Charges', () => {
    it('should list customer charges', async () => {
      const charges = await Stripe.getChargesList(customerObj);
      charges.data[0].should.have.property('id');
      charges.data[0].amount.should.have.property('id');
    });
  });

Upon thinking about it a little more I really like the behavior of waiting for both the promise and the callback. Was any progress made with getting that implemented?

@plroebuck two things:

  1. There’s a difference between documenting something and being one of the few libraries in npm that introspects on a function’s parameters. I could document that I take a background check of all my friends when I first meet them, but it’s still strange and people would still complain about it unless I had a reason and explicitly told them the reason.

  2. There’s a flaw in the documentation:

In Mocha v3.0.0 and newer, returning a Promise and calling done() …

It’s not about calling done, it’s about specifying done as a parameter whether or not you use it.

@ScottFreeCode What’s the fix here?

…if I omit .then(() => done()) then the test timeouts.

This seems like a bug, in any case.

I was nearly SOL on this. I need to execute async code AND its expected that code won’t finish executing(ever) so I HAVE to use done. The annoying hack-a-round was to wrap my async test code in a self invoking async function but leave the it func as a sync func.

Solution:

it("It shouldn't be like this", function (done) {
    ( async function(){
        var life = require('InfiniteLife');
        var asyncRes = await life.someCoolstuff();
        assert(asyncRes);
        setTimeout( function(){
            done();
        },letsCallItQuitsNow);
    })();
});

Well, we can do this in phases. The first would be to ensure that if a Promise is returned and a done callback is given, then Mocha will break in a friendly manner.

It’s conceivable that one could do something like this (with Bluebird):

// to make this work, you need to omit `return`
it('should do something async for sure', function(done) {
  return somethingAsync()
    .then(makeAssertion)
    .asCallback(done);
});

But that’s just something you could do; I have yet to determine a use case for this.

I was getting this error but I was accidentally returning a promise (supertest)

it('Should do something', (done) => {
       return supertest(server)
           .get(`/api/endpoint`)
           .set('somekey', 'somevalue')
           .expect(200)
           .then(() => { done(); })
           .catch(done);
});

Removing the ‘return’ clause solved the problem

@karlbateman Thanks. You don’t need that last .catch though. A rejected promise is counted as an error by Mocha

@Munter With async functions in the picture I think the returned promise scores lower on the explicitness scale because it’s created and returned automatically, whether or not you use await:

it('should work with a async function that could have been sync', async function () {
  assert.ok(true);
});

it('should work with a legitimately async function', async function () {
  assert.equal(await getBlah(), 'blah');
});

it('should work with a fully spelled-out Promise-based test', function () {
  return getBlah().then(function (blah) {
    assert.equal(blah, 'blah');
  });
});

And then there’s the controversial one:

it('should foo', async function (done) {
  assert.equal('foo', 'foo');
  done();
});

It’s also easy for that teeny-weeny async to sneak in by accident, so we should think of (at least) the first example as a new kind of gotcha, as @elado points out.

Totally agree with @RobertWHurst.

Requesting done should override the returned promise behavior. It is not likely to request done when it’s not needed, and scenarios of emitted events in an async function are a perfect example.

From my comment above:

it('fires change event when calling blah.doSomethingThatFiresChange', async function (done) {
  const blah = await getBlah()
  blah.on('change', () => done())
  blah.doSomethingThatFiresChange()
})

As more people move to ES6/7+async/await, this will become a common gotcha when using Mocha.

Please reconsider this change.

I am agree with that @RobertWHurst there is no ambiguous here.

Besides that I think this issue could be something “opinion based” and developers will have different point of views. I consider that is very extremist to do a breaking change and force people use that way.

To give you a fast solution to this issue, wrap your entire test in a promise, and use resolve as you would done.

Turn this:

it.only("Works with the database", async (done) => {
    let browser = await puppeteer.launch();
    let query = db.collection('lists').where('ownerId', '==', 'UJIXXwynaCj8oeuWfYa8');
    let ct = 0;
    query.onSnapshot(async querySnapshot => {
        if (ct === 0) {
            await addAPurpose(browser, session, sessionSig); // A function with a bunch of Puppeteer code.
            ct++
        } else {
            expect(querySnapshot.size).to.equal(1);
            expect(querySnapshot.docs[0].get().title).to.equal("Understand Mocha");
            done();
        }
        console.log(`Received query snapshot of size ${querySnapshot.size}`);
    }, err => {
        console.log(`Encountered error: ${err}`);
    });
});

into this:

it.only("Works with the database", async () => {
    const onSnap = new Promise(async (res, rej) => {
        let browser = await puppeteer.launch();
        let query = db.collection('lists').where('ownerId', '==', 'UJIo1gGMueoubgfWfYa8');
        let ct = 0;
        let unsubscribe = query.onSnapshot(async querySnapshot => {
            if (ct === 0) {
                await addAPurpose(browser, session, sessionSig);
                ct++
            } else {
                expect(querySnapshot.size).to.equal(1);
                expect(querySnapshot.docs[0].data().title).to.equal("Evolution");
                // done(); 
                unsubscribe();
                res();
            }
            console.log(`Received query snapshot of size ${querySnapshot.size}`);
        }, err => {
            console.log(`Encountered error: ${err}`);
            rej()
        });
    });
    return onSnap;
});

And it works like you want.

The need to remove done surprised me, because the test ran until done was called. To make it more obvious that done shouldn’t be used with async, async functions should fail immediately if done is passed. Mocha should start the test by:

  1. Seeing if the function is async, and
  2. Detecting the done argument.

If they’re both present, it should throw instead of letting my tests run up until done is called and then blue-balling me. Then the error should suggest that you wrap your code in another promise, and use resolve as you would done.

I know you can use function.prototype.name === "AsyncFunction". Then it’s

if (function.prototype.name === "AsyncFunction && arg1.name === "done") {
  throw new Error("Can't use done with an async function")
}

to implement this.

The low level and error-prone solution:

it('should throw an error', async function() {
  try {
    result = await something('value that causes something() to throw an Error');
    throw new Error('Promise unexpectedly fulfilled');
  } catch (e) {
    // Optionally assert something about e...
  }
});

I’d use an assertion library any day of the week:

const expect = require('unexpected');
it('should throw an error', async function () {
    await expect(something(), 'to be rejected with', 'the error message');
});

Or chai + chai-as-promised:

const chai = require('chai');
chai.use(require('chai-as-promised'));
const expect = chai.expect;

it('should throw an error', async function () {
    await expect(something()).to.be.rejectedWith('argh');
});

I’ve been using mocha 4 with chai.assert.

At first I tried to use the done() callback like so.

it('should throw an error', async function(done) {
  try {
    result = await something('value that causes something() to throw an Error');
    done('failed');
  } catch (e) {
    done();
  }
});

It failed with the

Error: Resolution method is overspecified. Specify a callback *or* return a Promise; not both."

That error is what led me here, to this page. From wading through this rather long thread (and I admit I didn’t fully understand all the disputes), am I to understand that I should not be using done() at all with async calls because they are Promised based?

If so, how do I make the following test pass when the call to await something() throws an Error and this is what I would expect to happen?

it('should throw an error', async function() {
  try {
    result = await something('value that causes something() to throw an Error');
  } catch (e) {
    // how to indicate that this should happen, if I can't call done() to tell mocha?
  }
});

Could someone help me understand this specific case please? Should I be using the assertion library or is there something additional I need to put in the code above?

Many thanks.

Not sure if I’m missing something… but I solved this problem by not using any return statements with my Promises and just relying on done().

@RobertWHurst You argue that defining a done callback is explicit intention. Is a return statement not an explicit intention? Both are defined in your code by you. How can we decide that one part of your code is intentional and another is not? If you imagine a world before () => foo any return statement would always have been explicit. The only reason you are all up in arms now is because you have started using implicit return statements, for what I can only think are aesthetical reasons.

Given that a lot of Mocha usage is by beginners who usually copy/paste examples, which very likely contain a done callback, how would you handle the case where this new user explicitly returns a promise, but get a timeout? This is the result if the change you are mad about gets reverted.

The current behavior is much more clear about what is wrong than just an unexpected timeout

@ScottFreeCode Hmm, yeah, it seems to be because the “overspecified” error is issued in the function provided as the done callback: https://github.com/mochajs/mocha/blob/4944e31ff60105815f4b314996a9861e73f6bfd2/lib/runnable.js#L357-L373

… but we can of course determine that we have to fail with that error as soon as the function has returned.

That test looks fine; I’d guess something else is wrong. But this thread is not where that debugging should happen.

(Used the wrong account the first time, sorry for the noise everyone).

Yeah - it’s not pretty, but you can wrap your code in an async closure:

it("should error on fn", function (done) {
  (async () => {
    try {
      await fn();
    } catch (e) {
      // assert some things here
      done();
    }
  })();
});

An example of async functions with done breaking.

it('If the credentials exists in the system it should return the token generated against it.', async (done) => {
        let aObj = await admin.createAdmin();
        chai.request(server)
        .post("/authenticate")
        .set("Content-Type", "application/x-www-form-urlencoded")
        .send({username: aObj.login,password:aObj.password})
        .end((err, res) => {
            res.should.have.status(200);
            res.body.should.be.a("string");
            done();
        });
    });

Success Case

it('If the credentials exists in the system it should return the token generated against it.', async () => {
        let adminObj = await admin.createAdmin();
        chai.request(server)
        .post("/auth/login")
        .set("Content-Type", "application/x-www-form-urlencoded")
        .send({username: adminObj.login,password:adminObj.password})
        .end((err, res) => {
            res.should.have.status(200);
            res.body.should.be.a("string");
            // done();
        });
    });

@tamoyal, yeah – as elado posted back in Aug 3, 2016.

Using async/await returns an implicit Promise. Use of the done callback is for CPS-based async, as shown here. Mocha supports both… but not at the same time. This is documented and has been standard behavior since the Mocha-3.0 release.

In my case, I wrapped the async block in a try/catch

it('It should activate a user.', async() => {
        return new Promise((resolve, reject) => {
            try {
                // Get the last activation token from the DB
                let sql = 'select * from activation_tokens order by created_at desc limit 1';
                let res = await DB.query(sql);
                let {token} = res[0];

                server
                    .post('/auth/activate')
                    .send({token})
                    .set('Content-Type', 'application/x-www-form-urlencoded')
                    .expect('Content-Type', /json/)
                    .expect(200)
                    .end((err, res) => {
                        res.body.data.should.contain.keys('message');
                        res.body.data.message.should.equal("Activated");
                        resolve();
                    });
            } catch (e) {
                console.error(e);
            }
        });
    });

@SaschaNaz thank you, this works in v3.2.0 😃

Is there a workaround for this when using babel?

Actually the reason you get an exception here is the maximum avoidance of enforcing opinion. Mocha does not have an opinion on wether the returned promise or the callback are authoritative. You can’t have both at the same time since that leads to ambiguous results. Ambiguous results in a test framework should be considered an error. Hence the error message to help you be aware of this and make the choice and change matching your opinion.

It might be beneficial to dial down the amount of drama. “you broke Mocha” is not helping anyone. A semver major version increase is explicitly defined as breaking changes that might require you to adjust your code. You can stay on 2.x to give you time to make the changes to fix your tests. This is an evolution, not a breakage

@ScottFreeCode I’m in agreement. So it’s

  1. Detect problem; instantiate the Error but do not call done() with it
  2. Wait until Promise fulfillment
  3. Reject with the Error

Bonus question: What to do with the result of the Promise fulfillment?