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 clients to disable auto-record feature (#530) #531

Closed
wants to merge 1 commit into from

Conversation

hebdavepaul0
Copy link

PR for issue #530.

Adds a global boolean to control whether or not the framework will auto-record missing snapshots.

Helpful for cases where CI/CD might attempt multiple retries, and auto-recording snapshots could cause false successes.

@jklingen
Copy link

I tested the suggested change locally and for me it works as expected.

If there is anything I can help with to get this merged (e.g. further testing), please let me know.

@nunogoncalves
Copy link

We need this functionality as well! I upvote this!

I was just testing this and it works well. I'm just thinking whether or not it makes sense to also include this as an argument to the assertSnapshot function, just as record is.

@stephencelis @mbrandonw do you think that makes sense?

Also, maybe this deserves a mention on the Readme, and we would probably need a bump? Unless maybe Brandon and Stephen take care of these once more PRs get merged?

@markst
Copy link

markst commented Oct 20, 2022

This is useful to us to be able to disable recording missing snapshots on a per architecture basis:

#if arch(x86_64)
public var recordMissingSnapshots = false
#else
public var recordMissingSnapshots = true
#endif

@broomburgo
Copy link

Thanks for this! It would be really useful for us, for the same reason mentioned in the OP.

@nunogoncalves
Copy link

@stephencelis @mbrandonw will this still be considered? by the way, @hebdavepaul0 this has conflicts. 🙏🏼

@mbrandonw
Copy link
Member

Closed via #867.

@mbrandonw mbrandonw closed this Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants