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

test(backup): Create backup version snapshot tests #58173

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

azaslavsky
Copy link
Contributor

The goal for the export.json feature is to support two minor versions (since we're using CalVer, roughly equivalent to two months) of releases when importing. That is to say, if the latest release is 23.10.0, and the previous two releases were 23.9.0 and 23.8.0, then a self-hosted release running code from [email protected] or self-hosted@latest (aka nightly) should be able to successfully import JSON backup data generated by [email protected] and [email protected], but not necessarily [email protected].

To support this, we add a new file test_releases.py that provides a snapshot test of an "exhaustive" (aka, has at least one of every exportable model) export. Every time we cut a new release, we will add a new commit directly after the release commit that renames the test_at_head snapshot to test_at_<RELEASED_VERSION>, and creates a test case for that version (see test_at_23_9_1 in this PR for one such example). Test cases older than the 2 version support window will simultaneously be removed.

In the future, we plan to automate this "release test rotation" process via GitHub actions, but for now the ospo team will take care to do this manually for every release.

Some other notes of interest:

  • Exports cannot be compared for equality using simple string comparisons, since they have special comparators for cases like passwords, dates that may or may not have been updated, etc (see comparators.py for greater detail). To accommodate this, we extend the insta_snapshot fixture with an extra argument, inequality_comparator, that allows a custom comparison lambda to be passed in for such cases.
  • The test_at_23_9_1 snapshot was generated by taking an early version of this PR, cherry-picking it onto the 23.9.1 release tag, generating a new snapshot there, then copying the result back into the main commit.

Closes getsentry/team-ospo#197

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 16, 2023
def test_at_23_9_1(self):
with tempfile.TemporaryDirectory() as tmp_dir:
_, snapshot_refval = read_snapshot_file(self.get_snapshot_path("23.9.1"))
snapshot_data = yaml.safe_load(snapshot_refval)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'm assuming most of this code will be reused in the future, is it worth abstracting that to another function? I guess we will only support 2 minor releases so it's not a big deal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have two test methods (as described in my comment on the main thread), so it will be a bit cleaner if we have a standalone function. This is especially true when we add automation to replace these test cases via codegen.

Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the steps to creating this new snapshot for 23.9.1 would be:

  1. git checkout 23.9.1
  2. run pytest with SENTRY_SNAPSHOTS_WRITEBACK=new
  3. Add test_at_23_9_1 in test_releases.py

@azaslavsky
Copy link
Contributor Author

azaslavsky commented Oct 16, 2023

When I do the 23.10.0 release later today, I'll:

  1. git checkout 23.10.0
  2. Rename snapshots/test_at_head.pysnap to snapshots/test_at_23_10_0.pysnap.
  3. Run SENTRY_SNAPSHOTS_WRITEBACK=1 pytest tests/sentry/backup/test_releases.py::ReleaseTests::test_at_head t generate a new snapshots/test_at_head.pysnap.
  4. Copy the test method test_at_23_9_1 and change it to test_at_23_10_0. We'll now have two version test methods: test_at_23_9_1 and test_at_23_10_0.

This is a bit of a special case release though, because we only have 1 version test at the moment, so we only need to add a version test case, not replace the oldest one. In the future (23.11.0 going forward), the process will be modified to:

  1. git checkout 23.11.0
  2. Rename snapshots/test_at_head.pysnap to snapshots/test_at_23_11_0.pysnap.
  3. Run SENTRY_SNAPSHOTS_WRITEBACK=1 pytest tests/sentry/backup/test_releases.py::ReleaseTests::test_at_head t generate a new snapshots/test_at_head.pysnap.
  4. Rename the test method test_at_23_9_1 (now 3 minor versions ago) and change it to test_at_23_11_0 (latest minor version released). We'll now have two version test methods: test_at_23_10_0 and test_at_23_11_0.

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #58173 (55b7c5d) into master (6028542) will increase coverage by 0.67%.
Report is 1 commits behind head on master.
The diff coverage is 76.19%.

❗ Current head 55b7c5d differs from pull request most recent head 1d31ff7. Consider uploading reports for the commit 1d31ff7 to get more accurate results

@@            Coverage Diff             @@
##           master   #58173      +/-   ##
==========================================
+ Coverage   78.38%   79.05%   +0.67%     
==========================================
  Files        5135     5135              
  Lines      223460   223471      +11     
  Branches    37623    37625       +2     
==========================================
+ Hits       175150   176674    +1524     
+ Misses      42570    41139    -1431     
+ Partials     5740     5658      -82     
Files Coverage Δ
src/sentry/testutils/pytest/fixtures.py 82.85% <76.19%> (+2.36%) ⬆️

... and 119 files with indirect coverage changes

The goal for the export.json feature is to support two minor versions
(since we're using CalVer, roughly equivalent to two months) of releases
when importing. That is to say, if the latest release is 23.10.0, and
the previous two releases were 23.9.0 and 23.8.0, then a self-hosted
release running code from [email protected] or self-hosted@latest (aka
nightly) should be able to successfully import JSON backup data
generated by [email protected] and [email protected], but not
necessarily [email protected].

To support this, we add a new file `test_releases.py`, that provides a
snapshot test of an "exhaustive" (aka, has at least one of every
exportable model) export. Every time we cut a new release, we will
add a new commit directly after the release commit that renames the
`test_at_head` snapshot to `test_at_<RELEASED_VERSION>`, and creates a
test case for that version (see `test_at_23_9_1` in this PR for one such
example). Test cases older than the 2 version support window will
simultaneously be removed.

In the future, we plan to automate this "release test rotation" process
via GitHub actions, but for now the ospo team will take care to do this
manually for every release.

Some other notes of interest:
- Exports cannot be compared for equality using simple string
  comparisons, since they have special comparators for cases like
  passwords, dates that may or may not have been updated, etc (see
  comparators.py for greater detail). To accommodate this, we extend the
  `insta_snapshot` fixture with an extra argument,
  `inequality_comparator`, that allows a custom comparison lambda to be
  passed in for such cases.
- The `test_at_23_9_1` snapshot was generated by taking an early version
  of this PR, cherry-picking it onto the `23.9.1` release tag,
  generating a new snapshot there, then copying the result back into the
  main commit.

Closes getsentry/team-ospo#197
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/release_tests branch from 55b7c5d to 1d31ff7 Compare October 16, 2023 21:42
@azaslavsky azaslavsky marked this pull request as ready for review October 16, 2023 21:51
@azaslavsky azaslavsky enabled auto-merge (squash) October 16, 2023 21:51
@azaslavsky azaslavsky merged commit ceae88a into master Oct 16, 2023
50 checks passed
@azaslavsky azaslavsky deleted the azaslavsky/backup/release_tests branch October 16, 2023 22:16
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2023
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.

Release test for import from last two versions before current
2 participants