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

INSTA_UPDATE=no still writes new snapshots #368

Closed
max-sixty opened this issue Apr 10, 2023 · 3 comments
Closed

INSTA_UPDATE=no still writes new snapshots #368

max-sixty opened this issue Apr 10, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@max-sixty
Copy link
Sponsor Collaborator

What happened?

Not a big deal personally, but was surprised by the behavior so I thought worth logging1.

INSTA_UPDATE=no still writes new files. The docs describe it as no: does not update snapshot files at all (just runs tests).

I would have thought it's similar to --check.

Reproduction steps

Create a new snapshot test; test_upper2, and run

INSTA_UPDATE=no cargo insta test -- test_upper2

This will create a new .snap.new file.

Insta Version

cargo-insta 1.29.0

rustc Version

1.65

What did you expect?

Resetting and adding --check has the expected behavior — no new files are written:

INSTA_UPDATE=no cargo insta test --check -- test_upper2

Footnotes

  1. Personally I almost always use --accept and use git as the diff...

@max-sixty max-sixty added the bug Something isn't working label Apr 10, 2023
@mitsuhiko
Copy link
Owner

I think that's because INSTA_UPDATE is overwritten by cargo insta test. This double use of envvars really needs to be cleaned up.

@max-sixty
Copy link
Sponsor Collaborator Author

@mitsuhiko I came around to looking at this. clap has a bunch of ways of handling overriding env vars, such as default_value_if. That would let us get this & associated issues in order without lots of manual logic.

I know you want to keep dependencies light, so may be keen to stay on structopt rather than moving to clap.

How would you balance this? Notably this is in cargo-insta only, so maybe has less dependency pressure?

@max-sixty
Copy link
Sponsor Collaborator Author

The takeaway here is not to use INSTA_UPDATE if using cargo insta; will close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants