Fix throw matcher error support

[zouxuoz]

Summary

Resolve #4946

Test plan

yarn jest --testPathPattern="throw_matcher"

[SimenB]

Thanks for the PR!

I think we only want this behavior if we're in a rejects block. Synchronous matcher solved it with a fromPromise?: boolean when constructing the matcher, is that possible here as well?

[codecov-io]

Codecov Report

Merging #4962 into master will increase coverage by 0.13%.
The diff coverage is 62.5%.

@@            Coverage Diff             @@
##           master    #4962      +/-   ##
==========================================
+ Coverage   60.31%   60.44%   +0.13%     
==========================================
  Files         198      198              
  Lines        6632     6664      +32     
  Branches        3        3              
==========================================
+ Hits         4000     4028      +28     
- Misses       2632     2636       +4

Impacted Files	Coverage Δ
...circus/src/legacy_code_todo_rewrite/jest_expect.js	0% <0%> (ø)	⬆️
packages/jest-jasmine2/src/jest_expect.js	0% <0%> (ø)	⬆️
packages/jest-snapshot/src/index.js	63.04% <83.33%> (+19.86%)	⬆️
packages/jest-message-util/src/index.js	86.13% <0%> (-0.6%)	⬇️
packages/jest-jasmine2/src/index.js	5.47% <0%> (-0.41%)	⬇️
packages/jest-runtime/src/script_transformer.js	86.5% <0%> (-0.33%)	⬇️
packages/jest-runner/src/run_test.js	1.96% <0%> (-0.27%)	⬇️
packages/jest-util/src/install_common_globals.js	100% <0%> (ø)	⬆️
packages/jest-runtime/src/index.js	74.83% <0%> (+1.2%)	⬆️
packages/jest-leak-detector/src/index.js	72.72% <0%> (+1.89%)	⬆️
... and 2 more

---

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 16e7c46...b6b5d84. Read the comment docs.

[zouxuoz]

@SimenB Oops :) I will do it today

[cpojer]

@zouxuoz do you have some time to finish this? :)

[zouxuoz]

@SimenB @cpojer Sorry for delay. Fixed, but not sure if this is the best solution 🤔

[cpojer]

Unfortunately expect cannot depend on jest-snapshot.

[SimenB]

I'm not sure if it's possible to have this feature the way expect.extend is written, then

[cpojer]

Is there any way we can expose something in expect that will allow us to put most of the logic into jest-snapshot without taking on this dependency?

[zouxuoz]

@cpojer @SimenB maybe we can do it something like this?

[SimenB]

Ah, this looks really promising! Could you add a test somewhere fitting that await expect(Promise.reject()).rejectes.toMatchSnapshot() works like it should? Maybe it's best as an integration test.

EDIT: hah, pun not intended

[zouxuoz]

@SimenB good idea, but how I can check the content of the written snapshot in integration tests? I can't find tests with similar behavior.

[thymikee]

Right after expectation you should be able to just read from FS, but before deleting the snapshot in (usually done in afterEach hook).

[zouxuoz]

@SimenB @cpojer done

[SimenB]

This is awesome, thanks!

Can you update the changelog?

[zouxuoz]

@SimenB updated 😉

[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.