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

Update snapshots without re-running tests #9591

Closed
aalexgabi opened this issue Feb 18, 2020 · 12 comments
Closed

Update snapshots without re-running tests #9591

aalexgabi opened this issue Feb 18, 2020 · 12 comments

Comments

@aalexgabi
Copy link

🐛 Bug Report

Currently you cannot update snapshots without re-running tests. This is problematic because:

  • this takes time (a lot of time in case you have a lot of test cases or the test cases are slow)
  • this is not necessary (jest already know what shapshots failed and what was the value for each snapshot so it can write them immediately instead of re-running tests)
  • this is not safe (re-running the tests does not ensure that the snapshots have not been altered in the meantime: code change, external api change, external db change etc.)

To Reproduce

Steps to reproduce the behavior:

  • use snapshots
  • try to update a snapshots for a failed test without re-running the test

Expected behavior

Update the failed snapshot on disk without re-running the test.

@jeysal
Copy link
Contributor

jeysal commented Feb 18, 2020

Thanks for opening the issue. While it clearly takes time, I disagree that it is unnecessary or unsafe.

In fact, code alteration in the meantime is exactly why your proposal could be considered unsafe - it updates based on an old state, not the state when pressing u / running jest -u.
It could be debated which behavior is better here, but it's definitely not a bug.

@aalexgabi
Copy link
Author

aalexgabi commented Feb 18, 2020

@jeysal It should use the state that is shown in the terminal before pressing u and not the unseen state afterwards.

@jeysal
Copy link
Contributor

jeysal commented Feb 18, 2020

I understand your request, that is what you said in the OP, but it is a feature request and not a bug and it has to be considered as such, not as something we should change without a doubt.
@SimenB @thymikee aside from the refactors needed for storing the serialized Received strings somewhere and being able to write them into the correct files later on, do you think this would be desirable?

@aalexgabi
Copy link
Author

@jeysal For me the current behavior is unsafe. I am using jest to test a service that generates videos from requests. I generate a hash from video frames using ffmpeg after each request (https://ffmpeg.org/ffmpeg-formats.html#hash-1) for each video and store the hash in the snapshot. Generating videos takes a long time (even at low resolution and fps). There are many test cases. The hash does not actually tell me anything about the contents of the video so I need to review the video contents manually by opening the file in the output folder before submitting the new hash to the snapshot using interactive mode.

In case the request queue was not cleaned correctly in the MQ server OR in case there is an orphaned process running an older version of the code in parallel I can actually add the hash of an entirely different video to the snapshot. Of course I could review the video twice but this is would be unnecessary if jest allows saving the snapshot displayed in the terminal without re-running the test.

This can be a configuration option.

@SimenB
Copy link
Member

SimenB commented Feb 18, 2020

I wonder if it makes sense to always always update with data from the previous run (like requested here), but still run the tests - that way we know that the data is stable. Right now you can snapshot Math.random() and a -u run will happily update the snapshot and call your test passing, even though a second run would fail. Updating snapshots and then rerunning the tests should not be an observable change (I think?).
A way of opting out of the verification run would then make sense to me, if you do not want that security.

@aalexgabi
Copy link
Author

@SimenB I think that it's a good idea especially if there is a way of opting out of the verification run.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@aalexgabi
Copy link
Author

Why should an issue be closed just because does not get comments? I never understood this on GitHub. How is that going to solve the issue or prove that the issue should be closed?

@github-actions github-actions bot removed the Stale label Feb 25, 2022
@SimenB
Copy link
Member

SimenB commented Feb 26, 2022

See #12496

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 26, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants