Skip to content

Result of .snapshot() assertions in --update-snapshots mode is inconsistent #2636

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 · 3 comments
Closed
Labels
bug current functionality does not work as desired help wanted needs triage scope:snapshot-management

Comments

@ninevra
Copy link
Contributor

ninevra commented Dec 27, 2020

What you're trying to do

Run AVA in --update-snapshots mode.

What happened

Whether t.snapshot() assertions pass or fail depends on the state of other tests in the same file, and can differ between files.

Specifically, t.snapshot() assertions always pass in --update-snapshots mode, unless any test in their file is test.skip()ed, in which case they pass or fail as they would without --update-snapshots.

What you expected to happen

I'd expect that, whatever the behavior of t.snapshot() assertions is here, it would be consistent, or at least not change as a result of changes to unrelated tests.

This isn't a bug, precisely; the documentation doesn't specify whether snapshot assertions will or won't pass in these situations. It's just odd.

MRE.

AVA configuration: none.

AVA version: 3.14.0.

@novemberborn
Copy link
Member

We don't want to update snapshots if the resulting file may be incomplete due to tests or assertions being skipped. It's a little too easy to accidentally drop snapshots that way.

We may have implemented that as an assertion error, whereas instead we should have a way to "fail" the test file itself.

@novemberborn
Copy link
Member

@ninevra was this addressed in your refactor or does this remain outstanding?

@ninevra
Copy link
Contributor Author

ninevra commented Jun 7, 2021

At the time I didn't think snapshots v3 addressed this, but looking at it again, I guess it did. The refactor allows using skip()s freely while updating snapshots, which means snapshot assertions should now always pass in --update-snapshots mode.

I've just quickly checked this both against the MRE and against the code and everything looks fine.

Good catch!

@ninevra ninevra closed this as completed Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired help wanted needs triage scope:snapshot-management
Projects
None yet
Development

No branches or pull requests

2 participants