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

use raw serializer for e2e output snapshots #7651

Merged
merged 3 commits into from
Jan 19, 2019

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Jan 19, 2019

Summary

Noticed we had some inconsistencies in newlines on CI. Added a trim, but that looked pretty bad due to the " all over.

So I've added jest-snapshot-serializer-raw, which really cleans up the snapshots, IMO (should probably be default behavior in Jest... (#5660)).

An advantage is that now the snapshots actually match what the user sees, as we get rid of a lot of escaping

Test plan

Updated a lot of snapshots, hopefully green CI

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7651   +/-   ##
=======================================
  Coverage   68.23%   68.23%           
=======================================
  Files         251      251           
  Lines        9639     9639           
  Branches        5        6    +1     
=======================================
  Hits         6577     6577           
  Misses       3060     3060           
  Partials        2        2

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 44054af...e076ea2. Read the comment docs.

@jeysal
Copy link
Contributor

jeysal commented Jan 19, 2019

Wow, this looks like a huge DX improvement to me.
Would it be an option to make this automatic by having our e2e utils (runJest, extractSummary, ...) set a symbol property on the output strings and adding a serializer that detects those?

@SimenB
Copy link
Member Author

SimenB commented Jan 19, 2019

I had that at the start, but we do quite a lot of .split and .replace here and there in the tests.

Adding a property doesn't work, as pretty-format itself will handle typeof 'string'

@jeysal
Copy link
Contributor

jeysal commented Jan 19, 2019

I had that at the start, but we do quite a lot of .split and .replace here and there in the tests.

Implement String methods ourselves? :D Just kidding, if there's no simple way to automate it then it's probably not worth it as the extra effort required in each test seems manageable.
Just a bit worried that the e2e tests will inevitably become inconsistent with some using wrap and some forgetting it.

@SimenB
Copy link
Member Author

SimenB commented Jan 19, 2019

Just a bit worried that the e2e tests will inevitably become inconsistent with some using wrap and some forgetting it.

For sure, I agree! Personally, I'd like some option in Jest that does not escape strings in snapshots (beyond what must be escaped in a template string for it to still be valid JS)

@SimenB SimenB merged commit 647b2ad into jestjs:master Jan 19, 2019
@SimenB SimenB deleted the raw-serializer branch January 19, 2019 15:51
thymikee added a commit to thymikee/jest that referenced this pull request Jan 20, 2019
…m-runtime

* upstream/master:
  add missing truncate comment to recent blog posts (jestjs#7655)
  use raw serializer for e2e output snapshots (jestjs#7651)
  chore: use a Set for reserved words list in `jest-mock`
  Fix automock for numeric function names (jestjs#7653)
  Update docs re: `moduleFileExtensions` to add ordering note (left-to-right) (jestjs#7616)
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@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 12, 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.

None yet

4 participants