Skip to content

Commit ffa3026

Browse files
committed
Address @chadwhitacre comments
1 parent 14136eb commit ffa3026

File tree

1 file changed

+18
-14
lines changed

1 file changed

+18
-14
lines changed

tests/sentry/backup/test_models.py

+18-14
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@
2626
from tests.sentry.backup import ValidationError, tmp_export_to_file
2727

2828

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."""
29+
def targets_models(*expected_models: Type):
30+
"""A helper decorator that checks that every model that a test "targeted" was actually seen in
31+
the output, ensuring that we're actually testing the thing we think we are. Additionally, this
32+
decorator is easily legible to static analysis, which allows for static checks to ensure that
33+
all `__include_in_export__ = True` models are being tested."""
3134

3235
def decorator(func):
3336
def wrapped(*args, **kwargs):
@@ -60,14 +63,17 @@ def setUp(self):
6063

6164
def import_export_then_validate(self) -> JSONData:
6265
"""Test helper that validates that data imported from a temporary `.json` file correctly
63-
matches the actual outputted export data."""
66+
matches the actual outputted export data.
67+
68+
Return the actual JSON, so that we may use the `@targets_models` decorator to ensure that
69+
we have at least one instance of all the "tested for" models in the actual output."""
6470

6571
with tempfile.TemporaryDirectory() as tmpdir:
6672
tmp_expect = Path(tmpdir).joinpath(f"{self._testMethodName}.expect.json")
6773
tmp_actual = Path(tmpdir).joinpath(f"{self._testMethodName}.actual.json")
6874

69-
# Export the current state of the database into the "expected" temporary file, then parse it
70-
# into a JSON object for comparison.
75+
# Export the current state of the database into the "expected" temporary file, then
76+
# parse it into a JSON object for comparison.
7177
expect = tmp_export_to_file(tmp_expect)
7278

7379
# Write the contents of the "expected" JSON file into the now clean database.
@@ -84,8 +90,6 @@ def import_export_then_validate(self) -> JSONData:
8490
if res.findings:
8591
raise ValidationError(res)
8692

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.
8993
return actual
9094

9195
def create_monitor(self):
@@ -101,38 +105,38 @@ def create_monitor(self):
101105
config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
102106
)
103107

104-
@targets_models([AlertRule, QuerySubscription, SnubaQuery, SnubaQueryEventType])
108+
@targets_models(AlertRule, QuerySubscription, SnubaQuery, SnubaQueryEventType)
105109
def test_alert_rule(self):
106110
self.create_alert_rule()
107111
return self.import_export_then_validate()
108112

109-
@targets_models([AlertRuleActivity, AlertRuleExcludedProjects])
113+
@targets_models(AlertRuleActivity, AlertRuleExcludedProjects)
110114
def test_alert_rule_excluded_projects(self):
111115
user = self.create_user()
112116
org = self.create_organization(owner=user)
113117
excluded = self.create_project(organization=org)
114118
self.create_alert_rule(include_all_projects=True, excluded_projects=[excluded])
115119
return self.import_export_then_validate()
116120

117-
@targets_models([AlertRuleTrigger, AlertRuleTriggerAction, AlertRuleTriggerExclusion])
121+
@targets_models(AlertRuleTrigger, AlertRuleTriggerAction, AlertRuleTriggerExclusion)
118122
def test_alert_rule_trigger(self):
119123
excluded = self.create_project()
120124
rule = self.create_alert_rule(include_all_projects=True)
121125
trigger = self.create_alert_rule_trigger(alert_rule=rule, excluded_projects=[excluded])
122126
self.create_alert_rule_trigger_action(alert_rule_trigger=trigger)
123127
return self.import_export_then_validate()
124128

125-
@targets_models([Environment])
129+
@targets_models(Environment)
126130
def test_environment(self):
127131
self.create_environment()
128132
return self.import_export_then_validate()
129133

130-
@targets_models([Monitor])
134+
@targets_models(Monitor)
131135
def test_monitor(self):
132136
self.create_monitor()
133137
return self.import_export_then_validate()
134138

135-
@targets_models([MonitorEnvironment])
139+
@targets_models(MonitorEnvironment)
136140
def test_monitor_environment(self):
137141
monitor = self.create_monitor()
138142
env = Environment.objects.create(organization_id=monitor.organization_id, name="test_env")
@@ -142,7 +146,7 @@ def test_monitor_environment(self):
142146
)
143147
return self.import_export_then_validate()
144148

145-
@targets_models([Organization])
149+
@targets_models(Organization)
146150
def test_organization(self):
147151
user = self.create_user()
148152
self.create_organization(owner=user)

0 commit comments

Comments
 (0)