[WIP] Make toThrow matcher pass only if Error-like object is returned from promises

[peterdanis]

Summary

Fix 1)
Fixes rejects.not path, where previously regular (non-promise) matcher was used.

Fix 2)
Current behavior of toThrow matcher allows to pass on any resolved/rejected promise. For example:

// Will successfully pass the test
test('Resolved promise', async () => {
  await expect(Promise.resolve('anything')).resolves.toThrow();
});

I propose to pass toThrow tests only if promise resolves/rejects to Error object. Also see #4884 (comment)

Test plan

Tests have been added and run successfully.

[codecov-io]

Codecov Report

Merging #5670 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5670   +/-   ##
=======================================
  Coverage   63.63%   63.63%           
=======================================
  Files         216      216           
  Lines        7911     7911           
  Branches        4        3    -1     
=======================================
  Hits         5034     5034           
  Misses       2876     2876           
  Partials        1        1

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbe05d3...91b91a4. Read the comment docs.

[SimenB]

Is the instanceof check safe? Ref #2549.

I guess it is seeing as tests pass.

/cc @mjesun

[cpojer]

No it isn't because there will be one instance of the Error class in both the parent and child contexts. We can only use duck typing.

[peterdanis]

Thanks for catching that. Is typeof actual.name === 'string' && typeof actual.message === 'string' enough instead of actual instanceof Error condition?

[peterdanis]

Or maybe use actual.toString && typeof actual.toString === 'function' && actual.toString() === Error.prototype.toString.call(actual)?

[peterdanis]

Its ok to leave the 2731c61 commit in this PR? Or should I move to it a separate PR?
Commit removed, it was not solving the root cause of failing CircleCi builds. See #5687

[peterdanis]

In angular/angular.js#15872 PR I found a test, where Error from different context is failing simple err instanceof Error check but implementation of isError (based on NPM package isError) is passing. Is an isError function worth implementing somewhere in utils etc.?

function isError(value) {
  switch (Object.prototype.toString.call(value)) {
    case '[object Error]': return true;
    case '[object Exception]': return true;
    case '[object DOMException]': return true;
    default: return value instanceof Error;
  }
}

[SimenB]

I guess an isError would be nice, we do have some weird checks here and there to detect errors

[peterdanis]

Moved isError function to packages/expect/src/utils.js with its own tests.

[github-actions]

This pull request 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.