Skip to content

"Could not update snapshots" message could be clearer #2634

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

Closed
ninevra opened this issue Dec 27, 2020 · 0 comments · Fixed by #2685
Closed

"Could not update snapshots" message could be clearer #2634

ninevra opened this issue Dec 27, 2020 · 0 comments · Fixed by #2685

Comments

@ninevra
Copy link
Contributor

ninevra commented Dec 27, 2020

What you're trying to do:

When AVA is passed --update-snapshots and a test runner encounters a skip()ed test (or, with #2623, a skip()ed assertion) the reporter prints "Could not update snapshots for the following test files: <list of filenames>".

This message doesn't say why snapshots couldn't be updated. In practice the reason is likely to be divinable (the user just recently added a .skip()), but it could be a lot clearer.

Further, the message can appear for test files that never use snapshots and test files where all snaphots match. It doesn't mean much of anything in these cases, and could mislead the user into thinking something went wrong.

How AVA could handle this

Indicating a reason: The Runner should know what problem prevented updating. It would have to pass this reason back with its {cannotSave: true} result from Runner#saveSnapshotState(), and then this would be propagated along though the 'snapshot-error' event and handled by reporters.

Suppressing the message when not relevant: checking whether snapshots existed should be simple enough. checking whether snapshots changed is trickier. The snapshot manager would have to be queried about whether any snapshots failed to match, which isn't currently something it knows when run in updating mode, since it ignores the .snap file entirely in favor of regenerating it.

This issue would benefit from reworking the snapshot manager to check for and load snapshots in every run, instead of only when it's called for and only when not updating.

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

Successfully merging a pull request may close this issue.

2 participants