Skip to content

Conversation

@lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Nov 12, 2025

This is a small refactor to the snapshotter we use in tests, including the following changes:

  • Rename a few variables for clarity.
  • Stop capturing the header when reading a snapshot file, since we never use it.
  • Remove timestamp and creator from the header. The timestamp often causes merge conflicts (and the same information can be gleaned from the commit date), and the creator is hard-coded as 'sentry', which... yeah, okay, but is that news?
  • Add test name to source entry in header. Yes, by default the test name is included in the snapshot file's path, but that's configurable, so might not always be true. Putting it in the header guarantees the information is easily available.

Note that this PR doesn't actually change the contents of any snapshots. Because snapshot files are only written when they change, and the header isn't actually anything we match on, we have the option to let the dates be removed organically, or to remove them in batches in separate PRs, but either way, this change won't make any current snapshot tests fail in the meantime.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 12, 2025
@lobsterkatie lobsterkatie marked this pull request as ready for review November 12, 2025 19:14
@lobsterkatie lobsterkatie merged commit 26c9a2a into master Nov 12, 2025
67 checks passed
@lobsterkatie lobsterkatie deleted the kmclb-refactor-snapshotter-and-simplify-header branch November 12, 2025 19:16
lobsterkatie added a commit that referenced this pull request Nov 12, 2025
This is a follow-up to #103243, which simplified the header for new snapshots, applying the change to existing grouping snapshots. Doing this manually will both prevent future potential merge conflicts and reduce noise in future PRs as snapshots are changed.
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
This is a small refactor to the snapshotter we use in tests, including the following changes:

- Rename a few variables for clarity.
- Stop capturing the header when reading a snapshot file, since we never use it.
- Remove timestamp and creator from the header. The timestamp often causes merge conflicts (and the same information can be gleaned from the commit date), and the creator is hard-coded as 'sentry', which... yeah, okay, but is that news?
- Add test name to `source` entry in header. Yes, by default the test name is included in the snapshot file's path, but that's configurable, so might not always be true. Putting it in the header guarantees the information is easily available.

Note that this PR doesn't actually change the contents of any snapshots. Because snapshot files are only written when they change, and the header isn't actually anything we match on, we have the option to let the dates be removed organically, or to remove them in batches in separate PRs, but either way, this change won't make any current snapshot tests fail in the meantime.
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
This is a follow-up to #103243, which simplified the header for new snapshots, applying the change to existing grouping snapshots. Doing this manually will both prevent future potential merge conflicts and reduce noise in future PRs as snapshots are changed.
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants