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

Generating snapshot update files is still desirable in CI environment #411

Closed
cyqsimon opened this issue Oct 7, 2023 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@cyqsimon
Copy link

cyqsimon commented Oct 7, 2023

What happened?

Hi, I'm the current maintainer of bandwhich, and I was trying to debug a weird test failure on Windows CI. This is a very random/unstable test error that happens on Mac and Windows environments, but which I am unable to reproduce on a local Windows VM.

Linked is one such occurrence of this failure. As you can see, the new result looks identical to the snapshot in logs despite being actually different. Therefore in this commit I tried to upload the artefacts so I can diff them locally, but to my surprise cargo-insta isn't generating snapshot update files in CI.

So I did my due diligence, searched the issues here, and quickly found #344 and #345. It seems like CI detection is indeed working as intended, but the behaviour of not generating updated snapshots is undocumented and surprising (in a bad way).

Since I cannot really think of a downside to generating these files in CI, I think it would be better behaviour if insta does both in a CI environment (print the diff and generate the .snap.new files). Just in case (such as the one here) the user wants the updated snapshot files for whatever reason.

Reproduction steps

  1. Use cargo-insta in CI
  2. Induce a test failure
  3. Observe that .snap.new files are not generated

Insta Version

1.33

rustc Version

No response

What did you expect?

cargo-insta generates .snap.new files in CI too, just like in non-CI environments.

@cyqsimon cyqsimon added the bug Something isn't working label Oct 7, 2023
@mitsuhiko
Copy link
Owner

I'm not entirely opposed to making this change but in most cases you cannot actually do anything with these files. If you do want them generated, you can already accomplish this by exporting INSTA_UPDATE=new in which case it will always write .new files, even in CI.

@cyqsimon
Copy link
Author

cyqsimon commented Oct 8, 2023

Thanks for the quick response. Hey I'm happy as long as there is a way to specify this behaviour. If you feel like the current default is better I have no objections.

I noticed though that insta uses quite a few environment variables; some standard, some custom. But I cannot seem to find a place where they are collectively documented. Does such documentation currently exist? If not please consider this a suggestion. Thanks.

cyqsimon added a commit to imsnif/bandwhich that referenced this issue Oct 8, 2023
@mitsuhiko
Copy link
Owner

I believe the full list of settings is down here: https://insta.rs/docs/settings/

@mitsuhiko mitsuhiko added enhancement New feature or request and removed bug Something isn't working labels Oct 8, 2023
@mitsuhiko
Copy link
Owner

I added a section on how to use this on github actions to the docs: https://insta.rs/docs/patterns/#uploading-snapshots-from-github-actions

@cyqsimon
Copy link
Author

cyqsimon commented Oct 8, 2023

Awesome. But you actually need if: ${{ failure() && steps.run_tests.outcome == 'failure' }} for the upload step to run.

See GitHub documentation.

@mitsuhiko
Copy link
Owner

Thanks. Changed.

@cyqsimon cyqsimon closed this as completed Oct 8, 2023
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

2 participants