Skip to content

Commit 14136eb

Browse files
committed
test(backup): Ensure expected models are in output
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
1 parent 17479e4 commit 14136eb

File tree

1 file changed

+51
-8
lines changed

1 file changed

+51
-8
lines changed

tests/sentry/backup/test_models.py

+51-8
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,50 @@
22

33
import tempfile
44
from pathlib import Path
5+
from typing import Type
56

67
from click.testing import CliRunner
78
from django.core.management import call_command
89

10+
from sentry.incidents.models import (
11+
AlertRule,
12+
AlertRuleActivity,
13+
AlertRuleExcludedProjects,
14+
AlertRuleTrigger,
15+
AlertRuleTriggerAction,
16+
AlertRuleTriggerExclusion,
17+
)
918
from sentry.models.environment import Environment
19+
from sentry.models.organization import Organization
1020
from sentry.monitors.models import Monitor, MonitorEnvironment, MonitorType, ScheduleType
1121
from sentry.runner.commands.backup import import_, validate
1222
from sentry.silo import unguarded_write
23+
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
1324
from sentry.testutils import TransactionTestCase
25+
from sentry.utils.json import JSONData
1426
from tests.sentry.backup import ValidationError, tmp_export_to_file
1527

1628

29+
def targets_models(expected_models: list[Type]):
30+
"""A helper decorator that checks that every model that a test "targeted" was actually seen in the output, ensuring that we're actually testing the thing we think are. Additionally, this decorator is easily legible to static analysis, which allows for static checks to ensure that all `__include_in_export__ = True` models are being tested."""
31+
32+
def decorator(func):
33+
def wrapped(*args, **kwargs):
34+
ret = func(*args, **kwargs)
35+
if ret is None:
36+
return AssertionError(f"The test {func.__name__} did not return its actual JSON")
37+
actual_model_names = {entry["model"] for entry in ret}
38+
expected_model_names = {"sentry." + model.__name__.lower() for model in expected_models}
39+
notfound = sorted(expected_model_names - actual_model_names)
40+
if len(notfound) > 0:
41+
raise AssertionError(f"Some `@targets_models` entries were not used: {notfound}")
42+
return ret
43+
44+
return wrapped
45+
46+
return decorator
47+
48+
1749
class ModelBackupTests(TransactionTestCase):
1850
"""Test the JSON-ification of models marked `__include_in_export__ = True`. Each test here
1951
creates a fresh database, performs some writes to it, then exports that data into a temporary
@@ -26,7 +58,7 @@ def setUp(self):
2658
# Reset the Django database.
2759
call_command("flush", verbosity=0, interactive=False)
2860

29-
def import_export_then_validate(self):
61+
def import_export_then_validate(self) -> JSONData:
3062
"""Test helper that validates that data imported from a temporary `.json` file correctly
3163
matches the actual outputted export data."""
3264

@@ -52,6 +84,10 @@ def import_export_then_validate(self):
5284
if res.findings:
5385
raise ValidationError(res)
5486

87+
# Return the actual JSON, so that we may use the `@targets_models` decorator to ensure that
88+
# we have at least once instance of all the "tested for" models in the actual output.
89+
return actual
90+
5591
def create_monitor(self):
5692
"""Re-usable monitor object for test cases."""
5793

@@ -65,42 +101,49 @@ def create_monitor(self):
65101
config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
66102
)
67103

104+
@targets_models([AlertRule, QuerySubscription, SnubaQuery, SnubaQueryEventType])
68105
def test_alert_rule(self):
69106
self.create_alert_rule()
70-
self.import_export_then_validate()
107+
return self.import_export_then_validate()
71108

109+
@targets_models([AlertRuleActivity, AlertRuleExcludedProjects])
72110
def test_alert_rule_excluded_projects(self):
73111
user = self.create_user()
74112
org = self.create_organization(owner=user)
75113
excluded = self.create_project(organization=org)
76114
self.create_alert_rule(include_all_projects=True, excluded_projects=[excluded])
77-
self.import_export_then_validate()
115+
return self.import_export_then_validate()
78116

117+
@targets_models([AlertRuleTrigger, AlertRuleTriggerAction, AlertRuleTriggerExclusion])
79118
def test_alert_rule_trigger(self):
80119
excluded = self.create_project()
81120
rule = self.create_alert_rule(include_all_projects=True)
82121
trigger = self.create_alert_rule_trigger(alert_rule=rule, excluded_projects=[excluded])
83122
self.create_alert_rule_trigger_action(alert_rule_trigger=trigger)
84-
self.import_export_then_validate()
123+
return self.import_export_then_validate()
85124

125+
@targets_models([Environment])
86126
def test_environment(self):
87127
self.create_environment()
88-
self.import_export_then_validate()
128+
return self.import_export_then_validate()
89129

130+
@targets_models([Monitor])
90131
def test_monitor(self):
91132
self.create_monitor()
92-
self.import_export_then_validate()
133+
return self.import_export_then_validate()
93134

135+
@targets_models([MonitorEnvironment])
94136
def test_monitor_environment(self):
95137
monitor = self.create_monitor()
96138
env = Environment.objects.create(organization_id=monitor.organization_id, name="test_env")
97139
MonitorEnvironment.objects.create(
98140
monitor=monitor,
99141
environment=env,
100142
)
101-
self.import_export_then_validate()
143+
return self.import_export_then_validate()
102144

145+
@targets_models([Organization])
103146
def test_organization(self):
104147
user = self.create_user()
105148
self.create_organization(owner=user)
106-
self.import_export_then_validate()
149+
return self.import_export_then_validate()

0 commit comments

Comments
 (0)