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

show file path for obsolete snapshots #19

Closed
wants to merge 2 commits into from

Conversation

ron23
Copy link
Contributor

@ron23 ron23 commented Feb 13, 2020

If a test fail due to obsolete snapshot, it's impossible to tell which file has the snapshots.
The filePath flag only address an actual test failure (via asserting testResult.failureMessage).
This PR adds support to show filePath for obsolete snapshots as well.
This tries to solve the same problem reported here #15 but instead of showing the report, simply show the file path for where the error occurred.

The result

$ yarn test:ci 
yarn run v1.21.1
$ JEST_SILENT_REPORTER_SHOW_WARNINGS=true JEST_SILENT_REPORTER_SHOW_PATHS=true <some path> --ci --silent --reporters=jest-silent-reporter --reporters=jest-junit

<the file path with the error>
 › 1 obsolete snapshot found.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

const hasFailures = testResult.failureMessage || hasUncheckedSnapshots;

if (this.showPaths && hasFailures) {
this.stdio.log('\n' + test.path);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should include some text I think, not just the path. log(`Obsolete snapshot at ${test.path}`) or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That text will be logged via helpers.getSnapshotStatus L60

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added result example in the description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, perfect!

@ron23 ron23 requested a review from rickhanlonii February 14, 2020 18:39
@ron23
Copy link
Contributor Author

ron23 commented Mar 11, 2020

@SimenB Would you be able to approve this or it has to be merged by @rickhanlonii ?

@SimenB
Copy link
Collaborator

SimenB commented Mar 11, 2020

No sorry, I have neither commit nor publish access here

@rickhanlonii rickhanlonii deleted the branch rickhanlonii:master November 9, 2020 14:31
@SimenB
Copy link
Collaborator

SimenB commented Nov 9, 2020

@rickhanlonii assuming this was closed due to master branch deletion - should change the target branch here

@rickhanlonii
Copy link
Owner

Hey @ron23, sorry this slipped - can you re-create targeted at the current main branch?

@ron23
Copy link
Contributor Author

ron23 commented Nov 20, 2020

Hey @rickhanlonii no worries I just created it. Thanks!

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

Successfully merging this pull request may close these issues.

3 participants