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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document and test custom, async, inline snapshot matcher #10922

Merged

Conversation

eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Dec 6, 2020

Summary

Documents and tests how to implement custom, async, inline snapshot matchers. Specifically, that this.error = new Error() must be set before any await.

Without overriding the stack jest fails when it encounters new snapshots with "Multiple inline snapshots for the same call are not supported"

Test plan

  • CI green

@eps1lon eps1lon marked this pull request as draft December 6, 2020 18:00
@eps1lon
Copy link
Contributor Author

eps1lon commented Dec 6, 2020

Test failures look like they're unrelated since the same errors happen on master. Marking as ready for review.

@SimenB
Copy link
Member

SimenB commented Dec 7, 2020

Test failures look like they're unrelated since the same errors happen on master. Marking as ready for review.

CI should be fixed via #10924 - could you rebase?

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff, thanks! Can you update the relevant versioned docs? I assume that works the same across Jest versions supporting inline snapshots?

@eps1lon eps1lon force-pushed the docs/custom-async-inline-snapshot-matcher branch from 2890d96 to ce36eac Compare December 7, 2020 14:17
@eps1lon
Copy link
Contributor Author

eps1lon commented Dec 7, 2020

I assume that works the same across Jest versions supporting inline snapshots?

I tried the same implementation in 25 and 23. Only 25 was working. 23 threw with Jest: Couldn't locate all inline snapshots.. Full repro: https://github.com/eps1lon/jest-async-inlinesnapshot

I won't have time to investigate the issue for 23.

@SimenB
Copy link
Member

SimenB commented Dec 7, 2020

I won't have time to investigate the issue for 23.

That's perfectly fine, thanks for taking the time to check at all!


expect.extend({
async toMatchObservationInlineSnapshot(fn, ...rest) {
// The error (and its stacktrace) must be created before any `await`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if Jest itself could instantiate an error and store it before calling this function? So that particular gotcha is gone. Not needed for now of course, and documenting for Jest 25 and 26 is great regardless

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did some shallow debugging but couldn't really figure out why it was working without this.error = new Error() for existing snapshots but not for new snapshots. I might revisit this though having a test in the repo is a sufficient guarantee for me now 😉

@SimenB SimenB merged commit a424896 into jestjs:master Dec 7, 2020
@eps1lon eps1lon deleted the docs/custom-async-inline-snapshot-matcher branch December 7, 2020 14:34
jeysal added a commit to mattphillips/jest that referenced this pull request Dec 13, 2020
* master: (30 commits)
  chore: verify TS project references are correct (jestjs#10941)
  chore(deps): bump actions/setup-node from v2.1.2 to v2.1.3 (jestjs#10940)
  docs: Rectify typo in tutorialReactNative (jestjs#10930)
  chore: patch react-native jest preprocessor to avoid warning
  Ensure `toContain` only accepts strings when `received` is a string (jestjs#10929)
  chore: update lockfile after publish
  v27.0.0-next.2
  Document and test custom, async, inline snapshot matcher (jestjs#10922)
  feat(transform): pass config options through to transformer (jestjs#10926)
  chore: bump eslint-config-prettier
  chore: run prettier using eslint
  chore: update lockfile after publish
  v27.0.0-next.1
  fix: move binary file declaration from runtime to repl (jestjs#10925)
  chore(test-result): remove deprecated `sourcemap` property (jestjs#10355)
  chore: remove mapCoverage remainings; remove deprecated CLI options test (jestjs#9968)
  refactor(jest-runtime,jest-transform): add readonly for some class fields (jestjs#10918)
  chore: ensure single environment package as well
  chore: fix failing tests (jestjs#10924)
  chore: fix lint warning
  ...
@github-actions
Copy link

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.

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

Successfully merging this pull request may close these issues.

3 participants