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

Allow updating snapshots for individual tests or individual assertions #2635

Closed
ninevra opened this issue Dec 27, 2020 · 1 comment · Fixed by #2685
Closed

Allow updating snapshots for individual tests or individual assertions #2635

ninevra opened this issue Dec 27, 2020 · 1 comment · Fixed by #2685
Labels
bug current functionality does not work as desired help wanted scope:snapshot-management

Comments

@ninevra
Copy link
Contributor

ninevra commented Dec 27, 2020

What you're trying to do

Example use case:

  • a developer writes tests using t.snapshot() assertions
  • a regression or external change causes such a test to begin failing
  • the developer marks the test .failing(), expecting the issue to remain for some time
  • later, the developer makes changes to other tests in the same file, and runs --update-snapshots to record the changes
  • this incidentally updates the snapshots for the .failing() test, causing it to begin passing, even though the underlying issue remains
  • developer is now stuck: they have no way to update snapshots for just part of a file (not even manually, because AVA's snapshot format is an implementation detail)

Why you can't use AVA for this

AVA appears to provide no way to select individual tests or assertions for update. Selecting files can be accomplished by passing AVA a glob, but using --match or line number selection with --update-snapshots throws.

Fundamentally, this is because AVA implements --update-snapshots by entirely ignoring and overwriting the snapshot files. If AVA allowed users to select specific tests, then every other test in their files would have its snapshots deleted.

And maybe how you think AVA could handle this

From a ui perspective, AVA could handle this by providing an interactive --review-snapshots option, like the similar features in insta (implemented as a cli tool, cargo insta review) or jest (seemingly only available during watch mode).

AVA could also allow the use of --match and other existing forms of selection with --update-snapshots, but this would only allow per-test granularity, not per-snapshot.

From an implementation perspective, AVA would probably need to read snapshot files from disk even when run with --update-snapshots, so that the unselected tests/assertions could retain their preexisting snapshots.

It might be possible to implement this using AVA's currently-unused ability to append updates to .snap files:

ava/lib/snapshot-manager.js

Lines 184 to 186 in 3fe4c40

// Entry start and end pointers are relative to the header length. This means
// it's possible to append new entries to an existing snapshot file, without
// having to rewrite pointers for existing entries.

ava/lib/snapshot-manager.js

Lines 298 to 299 in 3fe4c40

// Allow for new entries to be appended to an existing header, which could
// lead to the same hash being present multiple times.

but I think that would require (backwards-compatible!) changes to the snapshot file format.

@novemberborn
Copy link
Member

If we knew the test was failing because of the snapshot, then we could retain that failing snapshot while updating everything else. We'd have to be able to regenerate the Markdown file based on the binary snapshot serialization, which should be possible.

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 scope:snapshot-management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants