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

obsolete snaps not deleted #130

Closed
sbdchd opened this issue Jul 20, 2020 · 7 comments
Closed

obsolete snaps not deleted #130

sbdchd opened this issue Jul 20, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@sbdchd
Copy link

sbdchd commented Jul 20, 2020

Currently when moving files around the old snapshots with the old filenames aren't deleted.

reproduce

  1. Create a snapshot in a file foo.rs
  2. Rename that file to bar.rs
  3. run the tests again and accept the snapshots
  4. now there are two snapshots, the unused snap and the one used for bar.rs
@wgoodall01
Copy link

I ran into this issue earlier today---I was considering either:

  • A cargo insta prune [--check] subcommand to remove the unused snapshots. If --check is passed, exit with failure if any unused snapshots exist, so we can make sure there aren't any unused snapshots in a CI flow
  • Or, some way of marking .snap files as unused as the tests run, and incorporating "this snapshot file isn't used, delete it?" into cargo insta review.
    • To do this, every snapshot could touch the timestamp on its .snap file, and we'd figure out which files were untouched when reviewing changes
  • Or some combination of the above

Thoughts?

@sbdchd
Copy link
Author

sbdchd commented Jul 21, 2020

Another example is Jest which deletes the obsolete tests when it's passed an update flag, pretty much cargo insta accept.

Seems Jest determines obsolete snaps by first reading the all the snapshots in the directory and then checking off whether it was actually used when evaluating the tests:

@max-sixty
Copy link
Sponsor Contributor

To the extent you're familiar with Jest, do you know how it evaluates whether the full test suite is being run, rather than a subset?

@sbdchd
Copy link
Author

sbdchd commented Jul 23, 2020

sorry I'm not really familiar of jest's inner workings

But I get what you're saying: how does jest avoid deleting snapshots for tests that weren't run

@wgoodall01
Copy link

Jest controls the entire test harness---it knows which tests are running and which aren't, because Jest is the thing running the tests.

Is there a way to replicate this in libtest, without messing with the internals of the harness?

@mitsuhiko
Copy link
Owner

I toyed around with this earlier and I couldn't find a good way to deal with running only a subset of tests unfortunately.

@mitsuhiko
Copy link
Owner

Going to close this for now. While still not perfect, at least you can hack it together now: #136

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

No branches or pull requests

4 participants