Detect test scenarios using promises and not using done callback

[mik01aj]

Imho we should warn about such bugs:

it('should be invalid', function () {
    doSomeAsyncStuff().then(function () {
        expect(true).toBe(false);
    });
});

This test, when run, will pass, but some other test later will fail. This is the correct, fixed version:

it('should be invalid', function (done) {
    doSomeAsyncStuff().then(function () {
        expect(true).toBe(false);
    }).catch(fail).then(done);
});

This was actually a pretty common bug for me.

[mik01aj]

The same is also true for usages of (non-mocked!) setTimeout.

[mik01aj]

Looks like https://www.npmjs.com/package/eslint-plugin-should-promised does something similar, you might borrow some code there.

[tlvince]

eslint-plugin-mocha has a rule for this, which should be trivial to port.

However, as mentioned in https://github.com/lo1tuma/eslint-plugin-mocha/issues/24, I'd like to see some form of extracted core so we don't end up reimplementing the same thing.

[mik01aj]

@tlvince that rule is similar (and sure a great idea as well), but my example is different because there is no done argument in the function given to it. If I had the done argument but didn't use it, I would get a warning from no-unused-vars.

[jrencz]

there might be a variation of this rule to disable expects in .then()/.catch()/.finally() entirely, but it will only work as long as one passes a function into one of those, not a lexical identifier:

.then(() => {
  // this can be detected
  expect().toBe()
})
.then(doSomething);

const doSomething = () => {
  // this either can't be detected or it would give a lot of false positives
  // because one can create helpers like this and they don't have to be
  // called inside `.then()`
  expect().toBe()
}

For Angular there's bvaughn/jasmine-promise-matchers providing .toBeResolved()/.toBeResolveWith()/.toBeRejected()/.toBeRejectedWith() matchers.

[ideadapt]

@mik01aj I created a gist to illustrate the problem: https://gist.github.com/ideadapt/1d14e11841de3ede13a89d0d5e52c835 same code in a jsfiddle: http://jsfiddle.net/mn6dgtmy/1/

For me it would be great to have a rule that detects missing / inappropriate done calls. Since I've also stumbled across that issue from time to time.

I just think its not too easy to create such a rule ...

A good starting point might be, to detect if an expect call within a callback, is within a spec that has no done argument. To evaluate whether the done-call is made in a useful place, might be a next, more difficult step.

[ideadapt]

The linkes implementation is not as thorough as the one from eslint-plugin-mocka.

As @mik01aj said, an unused argument would give an issue anyway. But, if you disable the "unused-var" rule, my implementation will be satisfied, since it does not verify if done is actually called.