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

fix: strip added indent in error inline snapshots #10217

Merged
merged 4 commits into from Jul 3, 2020

Conversation

@pmmmwh
Copy link
Contributor

pmmmwh commented Jun 28, 2020

Summary

Currently, jest-snapshot does not strip added white space characters when comparing inline snapshots for errors. Test will pass on the first run (when the snapshot is written), but will fail in any subsequent runs. This mainly only affects errors with multi-line messages.

Test plan

Run this code (I'm happy to move this into a test case if required):

it('should pass', () => {
  // This will fail when the snapshot is written
  expect(() => {
    throw new Error(['Line 1', 'Line 2'].join('\n'));
  }).toThrowErrorMatchingInlineSnapshot();
});

Before the change, the test fails.
It passes afterwards.

Copy link
Collaborator

SimenB left a comment

Thanks! Could you add a test as well? The test added in #8492 might serve as inspiration

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Jul 2, 2020

@SimenB I've added tests - please check if you think it's appropriate!

@pmmmwh pmmmwh force-pushed the pmmmwh:fix/error-inline-snapshots branch from 53365fe to fe0e701 Jul 2, 2020
@SimenB
SimenB approved these changes Jul 2, 2020
Copy link
Collaborator

SimenB left a comment

Just run prettier and it should be good to go 👍

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Jul 3, 2020

I think I've fixed everything.
The failed test is unrelated to this PR and I'm not sure why it's failing either ...

@SimenB SimenB merged commit 1d8285c into facebook:master Jul 3, 2020
21 of 22 checks passed
21 of 22 checks passed
cleanup-runs
Details
Running TypeScript compiler & ESLint
Details
Node v10.x on ubuntu-latest
Details
Node v10.x on macOS-latest
Details
Node v10.x on windows-latest
Details
Node v12.x on ubuntu-latest
Details
Node v12.x on macOS-latest
Details
Node v12.x on windows-latest
Details
Node v13.x on ubuntu-latest
Details
Node v13.x on macOS-latest Node v13.x on macOS-latest
Details
Node v13.x on windows-latest
Details
Node v14.x on ubuntu-latest
Details
Node v14.x on macOS-latest
Details
Node v14.x on windows-latest
Details
ci/circleci: test-jest-circus Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-12 Your tests passed on CircleCI!
Details
ci/circleci: test-node-13 Your tests passed on CircleCI!
Details
ci/circleci: test-node-14 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
facebook.jest #20200702.10 succeeded
Details
@SimenB
Copy link
Collaborator

SimenB commented Jul 3, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.