Skip to content

feat(relay): snapshot testing for python integration tests #4877

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Litarnus
Copy link
Contributor

@Litarnus Litarnus commented Jun 30, 2025

Adds snapshot testing in python integration tests for relay.

Dynamic values can be redacted to prevent flakyness (or enable snapshot testing in general).

For example, event_id get's automatically replaced by <event_id> in the snapshot.

The list is configurable through direct parameters, e.g. relay_snapshot("json", additional_keys={"foo"}).

Keys need to match the full path, so timestamp will only match the root level timestamp key but not the one in spans (see snapshot in PR).

@Litarnus Litarnus changed the title Martinl/snapshot testing feat(relay): snapshot testing for python integration tests Jun 30, 2025
@Litarnus Litarnus marked this pull request as ready for review July 2, 2025 14:45
@Litarnus Litarnus requested a review from a team as a code owner July 2, 2025 14:45
@Dav1dde
Copy link
Member

Dav1dde commented Jul 2, 2025

Just some high level questions:

  1. Can we have the snapshots inline, I prefer that much over having them in separate files
  2. Instead of ignoring certain keys, can we assert that they match some bound (e.g. the assertions we have with time_within (or other generic functions that can produce some output)

I think ideally we have both of these capabilities before we start replacing uses. This may still be useful for some very large assertions though.

@Litarnus
Copy link
Contributor Author

Litarnus commented Jul 3, 2025

Can we have the snapshots inline, I prefer that much over having them in separate files

It should be relatively easy to add, regular python asserts give a good diff of what the differences between two dicts are.

Instead of ignoring certain keys, can we assert that they match some bound (e.g. the assertions we have with time_within (or other generic functions that can produce some output)

For inline snapshots it shouldn't be too hard to add it since you could just pass the comparison functions in the local snapshot but for file snapshots it would be more tricky since it would need to have a way to configure them upfront based on key name or something like that.

@Litarnus
Copy link
Contributor Author

Litarnus commented Jul 4, 2025

After giving it more thought I'm not sure what the benefit would be if this could also do inline snapshots. assert is pretty much an inline snapshot.

While I agree that inline snapshots are nicer and easier to review, I think this could be useful for PII related snapshot tests where we can not only test if fields are properly scrubbed but also make sure that _meta is correctly populated without having to write _meta ourselves or write helper functions that generate the meta.
For those tests it would also not be relevant if the timestamp redacted or within a timeframe since that would not be the scope of said tests.

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.

3 participants