Add thorough test suite for backup.py #156

azaslavsky · 2023-07-05T17:38:51Z

Tasks

Give feedback

We move the `validate()` method from the test suite into the main code itself. The end goal of this work is to be able to perform an opinionated diff on two JSON exports for validation purposes on real production data as part of the migration flow, so `validate()` is not merely a test function. The tests still use `freezegun` for now, but they will also have to migrated to a more sophisticated comparator-based system when working in production, where time of course cannot be frozen. Issue: getsentry/team-ospo#156

Adds a number of tests for exportable AlertRule* Sentry models, which necessitates testing for SnubaQuery* and QuerySubscription models as well, as these are included in AlertRule instances by default. Issue: getsentry/team-ospo#156

We add a new decorator, `@targets_models`, for the `.../backup/test_models.py` test suite. The goal is two-fold: for each individual test, the decorator provides a concise way to express which models must be included in the output, lest we end up with a test that passes the equality check, but only because it excluded our target model(s) altogether. The second goal is to make the set of models being exercised in the `ModelBackupTests` class easily visible to static analysis tools like flake8, so that we may later create a check ensuring that all `__include_in_export__ = True` marked models in this repository are included in this test suite. Issue: getsentry/team-ospo#156 Issue: getsentry/team-ospo#160

This creates a second test file for tests of specific models. The eventual end-goal here is to test all models that have `__include_in_export = True` set. This is the first change supporting that work, focused mostly on adding a single proof-of-concept test (for organizations), along with the scaffolding that will make adding more such tests easier. Issue: getsentry/team-ospo#156

Tests for monitoring. Issue: getsentry/team-ospo#156

Import using the proper style to make flake8 happy, and change a couple of test names to be more consistent. I've also re-alphabetized the tests in test_models.py after they got a bit out of order due to competing merges. Issue: getsentry/team-ospo#156

The basic idea here is to include a test that finds all non-abstract descendants of our `BaseModel`, filter down to just the exportable ones (ie, those that set `__include_in_export__ = True`), and then ensure that each of them is included in at least one of the tests seen in `test_models.py`. This is done by introducing a new `mark` wrapper class which ingests all of the "target" models used by the `@targets` test decorator at init time, allowing us to create an exhaustive list of all types passed to `@targets`. Issue: getsentry/team-ospo#156

Previously, all validation mutated the inputs in-place. This was performant (no need to make deep copies of two potentially large JSON blobs), but potentially bug prone if not held carefully, since the validation itself mutated the inputs via JSONScrubbingComparators. This means that the post-comparison input objects MUST NOT be re-used, since they are meaningfully different from the inputs that were declared valid. Instead, we now do deepcopies under the hood, leaving the originally passed-in inputs untouched. Issue: getsentry/team-ospo#156

We've already introduced model coverage checks in a previous PR, but these only validate that every `__include_in_export = True`-marked model is exercised by at least one test in `tests/sentry/backup/test_models.py`. This change improves on that slightly, by only considering a model to be "tested" if the matching instance in the output has all of its fields set, and none of those fields are set to their default value (if they have one). This ensures that not only are we covering every exportable model, we are covering every field of every such model. Issue: getsentry/team-ospo#156

Previously, all validation mutated the inputs in-place. This was performant (no need to make deep copies of two potentially large JSON blobs), but potentially bug prone if not held carefully, since the validation itself mutated the inputs via JSONScrubbingComparators. This means that the post-comparison input objects MUST NOT be re-used, since they are meaningfully different from the inputs that were declared valid. Instead, we now do deepcopies under the hood, leaving the originally passed-in inputs untouched. Issue: getsentry/team-ospo#156

The basic idea here is to include a test that finds all non-abstract descendants of our `BaseModel`, filter down to just the exportable ones (ie, those that set `__include_in_export__ = True`), and then ensure that each of them is included in at least one of the tests seen in `test_models.py`. This is done by introducing a new `mark` wrapper class which ingests all of the "target" models used by the `@targets` test decorator at init time, allowing us to create an exhaustive list of all types passed to `@targets`. Issue: getsentry/team-ospo#156

Previously, all validation mutated the inputs in-place. This was performant (no need to make deep copies of two potentially large JSON blobs), but potentially bug prone if not held carefully, since the validation itself mutated the inputs via JSONScrubbingComparators. This means that the post-comparison input objects MUST NOT be re-used, since they are meaningfully different from the inputs that were declared valid. Instead, we now do deepcopies under the hood, leaving the originally passed-in inputs untouched. Issue: getsentry/team-ospo#156

The basic idea here is to include a test that finds all non-abstract descendants of our `BaseModel`, filter down to just the exportable ones (ie, those that set `__include_in_export__ = True`), and then ensure that each of them is included in at least one of the tests seen in `test_models.py`. This is done by introducing a new `mark` wrapper class which ingests all of the "target" models used by the `@targets` test decorator at init time, allowing us to create an exhaustive list of all types passed to `@targets`. Issue: getsentry/team-ospo#156

We've already introduced model coverage checks in a previous PR, but these only validate that every `__include_in_export = True`-marked model is exercised by at least one test in `tests/sentry/backup/test_models.py`. This change improves on that slightly, by only considering a model to be "tested" if the matching instance in the output has all of its fields set, and none of those fields are set to their default value (if they have one). This ensures that not only are we covering every exportable model, we are covering every field of every such model. Issue: getsentry/team-ospo#156

We've already introduce model coverage checks in a previous PR, but these only validated that every `__include_in_export = True`-marked model is exercised by at least one test in `tests/sentry/backup/test_models.py`. This change improves on that slightly, by only considering a model to be "tested" if the matching instance in the output has all of its fields set, and none of those fields are set to their default value (if they have one). This ensures that not only are we covering every exportable model, we are covering every field of every model. Issue: getsentry/team-ospo#156

Previously, all validation mutated the inputs in-place. This was performant (no need to make deep copies of two potentially large JSON blobs), but potentially bug prone if not held carefully, since the validation itself mutated the inputs via JSONScrubbingComparators. This means that the post-comparison input objects MUST NOT be re-used, since they are meaningfully different from the inputs that were declared valid. Instead, we now do deepcopies under the hood, leaving the originally passed-in inputs untouched. Issue: getsentry/team-ospo#156

The basic idea here is to include a test that finds all non-abstract descendants of our `BaseModel`, filter down to just the exportable ones (ie, those that set `__include_in_export__ = True`), and then ensure that each of them is included in at least one of the tests seen in `test_models.py`. This is done by introducing a new `mark` wrapper class which ingests all of the "target" models used by the `@targets` test decorator at init time, allowing us to create an exhaustive list of all types passed to `@targets`. Issue: getsentry/team-ospo#156

We've already introduce model coverage checks in a previous PR, but these only validated that every `__include_in_export = True`-marked model is exercised by at least one test in `tests/sentry/backup/test_models.py`. This change improves on that slightly, by only considering a model to be "tested" if the matching instance in the output has all of its fields set, and none of those fields are set to their default value (if they have one). This ensures that not only are we covering every exportable model, we are covering every field of every model. Issue: getsentry/team-ospo#156

azaslavsky mentioned this issue Jul 5, 2023

Improve self-hosted ⇒ SaaS conversion alongside EU rollout #153

Closed

azaslavsky mentioned this issue Jul 11, 2023

feat(backup): Support alert rules getsentry/sentry#52669

Merged

This was referenced Jul 11, 2023

test(backup): Ensure expected models are in output getsentry/sentry#52670

Closed

feat(backup): Add validate() function to backup.py getsentry/sentry#52484

Merged

azaslavsky mentioned this issue Jul 11, 2023

test(backup): Ensure expected models are in output getsentry/sentry#52690

Merged

azaslavsky added a commit to getsentry/sentry that referenced this issue Jul 13, 2023

test(backup): Add monitor tests (#52485)

3a356c3

Tests for monitoring. Issue: getsentry/team-ospo#156

michellewzhang pushed a commit to getsentry/sentry that referenced this issue Jul 13, 2023

test(backup): Add monitor tests (#52485)

230e710

Tests for monitoring. Issue: getsentry/team-ospo#156

mifu67 pushed a commit to getsentry/sentry that referenced this issue Jul 13, 2023

test(backup): Add monitor tests (#52485)

9531ea6

Tests for monitoring. Issue: getsentry/team-ospo#156

This was referenced Jul 20, 2023

ref(backup): Do deepcopy before validation getsentry/sentry#53297

Merged

test(backup): Add more thorough model coverage checks getsentry/sentry#53298

Merged

azaslavsky mentioned this issue Jul 21, 2023

ref(backup): Do deepcopy before validation getsentry/sentry#53346

Merged

azaslavsky closed this as completed Aug 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add thorough test suite for backup.py #156

Add thorough test suite for backup.py #156

azaslavsky commented Jul 5, 2023 •

edited

Loading

Tasks

Add thorough test suite for backup.py #156

Add thorough test suite for backup.py #156

Comments

azaslavsky commented Jul 5, 2023 • edited Loading

Tasks

azaslavsky commented Jul 5, 2023 •

edited

Loading