Jest: Implement a more robust way to test Promise-returning code

[jeanlauliac]

Do you want to request a feature or report a bug?

Feature.

What is the current behavior?

For testing Promise-returning code at the moment, one can just return a Promise, ex:

it("do smth", () => doSmth().then(intermediateResult => {
  return doSomeMoreThings(intermediateResult).then(result => {
    expect(result).toEqual("beep");
  };
});

However, this is error prone. Consider what happens in this example above if we remove the return: the outer promise will resolve successfully, but the expect clause will never be run before the end of the test!

To mitigate that issue, one could use expect.assertions(1), but this is a hassle to maintain, and is actually still error prone as one could forget to increment when adding new code. For example, this is a bit contrived, but subtly broken:

it("do smth", () => doSmth().then(intermediateResult => {
  expect.assertions(1);
  doSomeMoreThings(intermediateResult).then(result => {
    expect(result).toEqual("beep");
  };
  expect(intermediateResult).toEqual("boop");
});

What is the expected behavior?

To make it more robust, jest could be made so as to expect a particular token from a Promise, given by the test case as a proof that the test was successful. For example:

it("do smth", () => doSmth().then(intermediateResult => {
  return doSomeMoreThings(intermediateResult).then(result => {
    expect(result).toEqual("beep");
    return jest.successfulPromiseToken;
  };
});

In this example, if the return statement was forgotten, then the Promise-returning test case would resolve with undefined instead, prompting jest to detect the test as failed.

What do you think?

[cpojer]

I like this idea!

[probablyup]

Isn't this the same concept as calling done()?

[incompl]

I agree that it's a problem that the test passes if you remove the return. Not opinionated about what the solution should be.

[SimenB]

I don't think Jest should do anything here. Like mentioned above, the example in the OP is just a fancy way of doing done that will instantly reject fail instead of time out on rejections.

I think a better solution is to promote https://www.npmjs.com/package/babel-jest-assertions. It ties into the alternative in the OP, but you don't have to maintain the number of assertions manually.

(might be interesting to include it in babel-jest behind an option...)

[jeanlauliac]

Cool let's close this then.

[incompl]

I urge you to reconsider. This is not a stylistic choice. It's a serious problem to have a test report as "passed" when it did not pass. Consider this example:

describe('Example test', () => {

  it('demonstrates an issue', () => {
    expect.assertions(1);
    var promise = new Promise((resolve) => {
      resolve();
    });

    promise.then(val => {
      expect(1).toBe(2);
    });
  });

});

This test passes, and absolutely should not.

[SimenB]

The suggestion in this issue wouldn't help in that case.

You have to tell Jest that your test is async (either by returning a promise, or taking a done callback), and tell it when it's done.

I'd be interested in hearing ideas here. I suppose we could add some magic for expect.assertions that make the test pending until either something rejects, or the number of expected assertions are ran. Feels sorta hackish though.

I'd prefer this to be a lint rule, but I'm not sure how that could be implemented...

Are there any other test runners which automatically figure out that your test is asynchronous without the developer indicating it somehow?

(also, the test above will make jest complain about uncaught rejections, in addition to printing the assertion failure. I agree it would be ideal if the test failed, but we don't hide the fact that something is wrong)

[incompl]

I'm on board with your idea to make the test pending until the expected number of assertions are seen. I'm not sure why that would be considered magical or hackish: from my perspective it's the expected and intuitive behavior.

[incompl]

Since you've identified that this is a separate issue, shall I open a new one for this to be discussed separately?

[SimenB]

Separate issue would be nice for "wait for pending assertions". It should probably work sorta like a done callback.

I meant it's "magic" in that you signal Jest that your test is synchronous (by not specifying that it's async) while it's actually not, and we should figure that out and _treat_ it as async. The way JS works usually forces you to be very specific about this.

[incompl]

Opened #4982, thanks for the help!