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

Improved the jest reporter with snapshot info per test. #3660

Merged
merged 1 commit into from
Aug 27, 2017

Conversation

excitement-engineer
Copy link
Contributor

Improved the jest terminal reporter to show snapshot info per test. The snapshot summary shown at the end of the test run shows some generic info like "2 snapshots written in 1 test suite", "1 obsolete snapshot file found". However, it wasn't always immediately clear in what test suite these snapshot changes had occurred. This PR attempts to resolve this by showing all the snapshot changes per test file so that it is immediately clear what happened after a test run. I am curious to hear what you think!

This addresses issue #3581.

I was also thinking that it would be nice if the name of the snapshot which has been updated is shown in the terminal rather than simply stating "2 snapshots written" or "2 obsolete snapshots found". Adding the name to the output does not seem very trivial; the type TestResult used in the jest-cli reporter doesn't export the names of the snapshots, it only exports the properties shown below. Therefore, adding the updated snapshot names would entail changing the structure in which the reporter gets its data. What do you guys think, is it worth adding the snapshot names to the output?

// the snapshot structure for type TestResult
snapshot: {|
    added: number,
    fileDeleted: boolean,
    matched: number,
    unchecked: number,
    unmatched: number,
    updated: number,
  |},

jest-snapshot-demo 1

Let me know what you think!

@codecov-io
Copy link

codecov-io commented May 26, 2017

Codecov Report

Merging #3660 into master will increase coverage by 0.17%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3660      +/-   ##
==========================================
+ Coverage   55.85%   56.02%   +0.17%     
==========================================
  Files         189      190       +1     
  Lines        6383     6406      +23     
  Branches        6        6              
==========================================
+ Hits         3565     3589      +24     
+ Misses       2815     2814       -1     
  Partials        3        3
Impacted Files Coverage Δ
...ages/jest-cli/src/reporters/get_snapshot_status.js 100% <100%> (ø)
...ackages/jest-cli/src/reporters/default_reporter.js 91.42% <100%> (+0.38%) ⬆️
packages/jest-cli/src/reporters/utils.js 63.54% <0%> (+1.04%) ⬆️

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 db4465e...6e33741. Read the comment docs.

@orta
Copy link
Member

orta commented May 26, 2017

The update to the TestResult is nice, then editors can run updates for specific tests too 👍

@excitement-engineer
Copy link
Contributor Author

Any updates on the status of this PR?

@cpojer
Copy link
Member

cpojer commented Aug 24, 2017

I'd like to merge this for 21, sorry for not getting to it earlier.

@excitement-engineer do you mind rebasing this change? Also, can we move it below console logs and failures, to the bottom of the result per test? :)

@excitement-engineer
Copy link
Contributor Author

Hey @cpojer, no problem! I refactored the code a bit to make it cleaner and I wrote some tests for the functionality I added. I also moved it below the console logs and failures:) Let me know what you think!

@excitement-engineer
Copy link
Contributor Author

I rebased the changes!

@excitement-engineer excitement-engineer mentioned this pull request Aug 26, 2017
6 tasks
@cpojer cpojer merged commit cb8e760 into jestjs:master Aug 27, 2017
@cpojer
Copy link
Member

cpojer commented Aug 27, 2017

Lovely. Thanks for the PR! :)

@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 13, 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.

5 participants