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

add filename for obsolete snapshot file #8665

Conversation

@ndelangen
Copy link
Contributor

ndelangen commented Jul 9, 2019

Summary

When a snapshot exists that's completely redundant because there's no test file that generates it, jest will report it found a obsolete snapshot:

Screenshot 2019-07-09 at 11 39 59

Especially on a CI this can be not particularity useful, as there is no information on what's actually gone wrong, and the user cannot run -u on CI.

Test plan

I have added e2e test called snapshot-unknown. This demonstrated the output or the cli reporter when there are 2 unknown snapshots. It should look like this:

Screenshot 2019-07-09 at 16 21 51

This addresses:
#5005 (comment)

ndelangen added 2 commits Jul 9, 2019
…new property filesRemovedList
@ndelangen

This comment was marked as resolved.

Copy link
Contributor Author

ndelangen commented Jul 9, 2019

I'm unsure what to do about the CI errors now, please help.

@SimenB
SimenB approved these changes Jul 9, 2019
Copy link
Collaborator

SimenB left a comment

This is great, thanks! Will need to figure out why CI fails, but the code LGTM

@SimenB

This comment was marked as resolved.

Copy link
Collaborator

SimenB commented Jul 9, 2019

Oh, I can reproduce. We report the two snapshots added in the integration test, not matter what test is run... The reporter just fails to mention that

image

This might be a bug in the obsolete detection logic, as it's covered by testPathIgnorePatterns

SimenB added 4 commits Jul 26, 2019
…ot-file
This reverts commit d2ff007.
@SimenB SimenB requested review from jeysal and thymikee Jul 26, 2019
@SimenB
SimenB approved these changes Jul 26, 2019
Copy link
Collaborator

SimenB left a comment

CI is happy now

@jeysal
jeysal approved these changes Jul 26, 2019
Copy link
Collaborator

jeysal left a comment

LGTM! Test result property naming getting awkward but I guess we'll have to live with that as anything else is too breaking

packages/jest-core/src/TestScheduler.ts Outdated Show resolved Hide resolved
@jeysal jeysal merged commit f440ded into facebook:master Jul 26, 2019
7 of 11 checks passed
7 of 11 checks passed
ci/circleci: test-jest-circus Your tests failed on CircleCI
Details
ci/circleci: test-node-6 CircleCI is running your tests
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
facebook.jest in progress
Details
ci/circleci: lint-and-typecheck Your tests passed on CircleCI!
Details
ci/circleci: test-browser Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
ci/circleci: test-node-8 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details
@ndelangen ndelangen deleted the ndelangen:ndelangen/add-filename-for-obsolete-snapshot-file branch Jul 28, 2019
@ndelangen

This comment has been minimized.

Copy link
Contributor Author

ndelangen commented Jul 28, 2019

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.