Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async testing fails with async/await #9240

Closed
yanickrochon opened this issue Nov 28, 2019 · 10 comments 路 Fixed by #9817
Closed

Async testing fails with async/await #9240

yanickrochon opened this issue Nov 28, 2019 · 10 comments 路 Fixed by #9817

Comments

@yanickrochon
Copy link

馃悰 Bug Report

Asynchronous testing does not support async functions.

To Reproduce

await expect(async () => {
   throw new Error('Test');
}).rejects.toThrow('Test');

Yields:

    expect(received).rejects.toThrow()

    Matcher error: received value must be a promise

    Received has type:  function
    Received has value: [Function anonymous]

      22 |       await expect(async () => {
      23 |          throw new Error('Test');
    > 24 |       }).rejects.toThrow('Test');
         |                  ^
      25 |    });
      26 | 
      27 | 

Expected behavior

The test should treat the function as a promise. At the moment, the fix is to chain the function to a Promise, for example:

await expect( Promise.resolve().then(async () => {
   throw new Error('Test');
})).rejects.toThrow('Test');

The solution is quite easy, instead of expecting only an object, expect should check if the constructor is an AsyncFunction. For example :

if (value.constructor.name === 'Promise` || value.constructor.name === 'AsyncFunction') {
    ...
}

envinfo

  System:
    OS: Linux 5.3 Ubuntu 19.10 (Eoan Ermine)
    CPU: (16) x64 AMD Ryzen 7 PRO 2700 Eight-Core Processor
  Binaries:
    Node: 12.10.0 - ~/.nvm/versions/node/v12.10.0/bin/node
    Yarn: 1.19.1 - ~/.nvm/versions/node/v12.10.0/bin/yarn
    npm: 6.11.3 - ~/.nvm/versions/node/v12.10.0/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0 
@yacinehmito
Copy link
Contributor

Hello 馃憢

Your suggestion would indeed make the API more practical for the use case you present, but it would not work consistently. Relying on the name of the function being passed is error-prone. Indeed, it is not necessarily an arrow-function and it could be a named async function.

I also don't believe it is a bug. The documentation makes no claim to support functions; it knowingly only supports promises.

To work around this, you can simply call the function.
This is similar to your suggestion, but you don't really need the Promise.resolve. The code below will work just as well:

await expect((async () => {
   throw new Error('Test');
})()).rejects.toThrow('Test');

Not only does this work fine, but most of the time it is not even needed. Indeed, people tend to test one function call. Say I already have an async function called myfunction. This is how I would test it:

await expect(myfunction()).rejects.toThrow('Test'));

It doesn't look too bad.

@markmsmith
Copy link

I think the syntax of await expect(myfunction()).rejects.toThrow('Test')); is totally reasonable.

However, I also ran in to this situation, where it seemed inconsistent compared to the non-async case, where jest was smart enough to know it needed to invoke the supplied function due to the matchers that were chained on:

const fnToTest = (num) => {
  throw new Error('Test error ${num}`);
}
// calls the wrapper lambda for me due to the `toThrow` matcher
expect(() => fnToTest(1)).toThrow('Test error');

vs

const fnToTest = async (num) => {
  throw new Error('Test error ${num}`);
}
// doesn't call the wrapper lambda for me, though it seems like we could if we detect the parameter to `expect` was a function rather than a promise
expect(() => fnToTest(2)).rejects.toThrow('Test error');

So, while I agree that you can achieve the desired test pretty cleanly with the current way of doing things, the inconsistency with the non-async case seems like a gotcha for people learning the library that could be addressed by making jest a little smarter (and we wouldn't have to rely on function names with this solution).

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

I'll happily merge a PR which invokes the provided function if it's not a promise, and throws if the return value of that function is not a promise. Anyone up for it? 馃檪

@udaypydi
Copy link

@SimenB Can I take a look into this?

@SimenB
Copy link
Member

SimenB commented Feb 20, 2020

Yeah, that'd be wonderful. Go for it!

@tanettrimas
Copy link

@udaypydi Any update on your progress thus far? Otherwise, I might try to have a go as well :)

@udaypydi
Copy link

@tanettrimas I am working on it. Will raise a PR by tomorrow end of the day.
Thanks!

@notoriousmango
Copy link
Contributor

I like to take a look into this

@Patrick-Remy
Copy link

Patrick-Remy commented Apr 20, 2020

Still exists for resolves.

Current behavior

await expect(async () => new Promise(resolve => setTimeout(resolve, 1000))).resolves.toBeUndefined()

// Matcher error: received value must be a promise

Workaround

await expect((async () => new Promise(resolve => setTimeout(resolve, 1000)))()).resolves.toBeUndefined()

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants