Skip to content

Add pytest-insta #1506

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

Merged
merged 1 commit into from
Jun 30, 2025
Merged

Add pytest-insta #1506

merged 1 commit into from
Jun 30, 2025

Conversation

Litarnus
Copy link
Contributor

@Litarnus Litarnus commented Jun 27, 2025

Required for getsentry/relay#4877

@asottile-sentry
Copy link
Member

is this for use in sentry? we've been trying to strongly discourage snapshot testing as they tend to lead to flakiness and people blindly accepting new snapshots rather than actually debugging and fixing problems.

we've removed two snapshotting systems from sentry already and would strongly recommend against adding a new one. there's already a snapshotting system in place as well but generally is discouraged from use (the pysnap system)

@Litarnus
Copy link
Contributor Author

is this for use in sentry?

No this is meant to be used in relay. We have tests that would strongly benefit from snapshot testing, especially PII tests.
I also added a fixture within relay to replace dynamic data to not make it flaky (or rather, make snapshots work in general)

there's already a snapshotting system in place as well but generally is discouraged from use (the pysnap system)

Do you mean the one that uses rust insta under the hood? I didn't see a different one in the list of dependencies.
I looked at it and I think it would be rather complex to make it work in relay, while this one is fairly easy.

@Litarnus Litarnus requested a review from a team June 30, 2025 12:57
@Litarnus Litarnus marked this pull request as ready for review June 30, 2025 12:57
@untitaker
Copy link
Member

untitaker commented Jun 30, 2025

pytest-insta did not exist at the time that pysnap was built into sentry. i think we should adopt pytest-insta and remove pysnap entirely. from what I remember pysnap is mainly used in grouping tests which are not flaky afaik. minidump tests in sentry are sometimes flaky, if we think removing snapshotting would reduce the amount of flakiness there let's just remove it there? maybe the guidance can be relaxed to: no snapshotting outside of unit tests?

@Litarnus Litarnus merged commit 0bb0c6d into main Jun 30, 2025
15 checks passed
@Litarnus Litarnus deleted the martinl/pytest-insta branch June 30, 2025 15:18
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.

4 participants