Browse Source

fix(tests): Another attempt to reduce flakiness of event frequency tests (#32580)

Fixing the date at a specific time of day and also making sure that freeze_time is used overall for
the test and not at multiple points.

Also improved how we create sessions
Dan Fuller 3 years ago
parent
commit
757ab5c0b8
1 changed files with 122 additions and 126 deletions
  1. 122 126
      tests/snuba/rules/conditions/test_event_frequency.py

+ 122 - 126
tests/snuba/rules/conditions/test_event_frequency.py

@@ -99,76 +99,75 @@ class StandardIntervalMixin:
         self._run_test(data=data, minutes=10080, passes=False)
 
     def test_comparison(self):
-        with freeze_time(before_now(days=1).replace(hour=12, minute=30, second=0, microsecond=0)):
-            # Test data is 4 events in the current period and 2 events in the comparison period, so
-            # a 100% increase.
-            event = self.store_event(
-                data={
-                    "fingerprint": ["something_random"],
-                    "timestamp": iso_format(before_now(minutes=1)),
-                    "user": {"id": uuid4().hex},
-                },
-                project_id=self.project.id,
-            )
-            self.increment(
-                event,
-                3,
-                timestamp=now() - timedelta(minutes=1),
-            )
-            self.increment(
-                event,
-                2,
-                timestamp=now() - timedelta(days=1, minutes=20),
-            )
-            data = {
-                "interval": "1h",
-                "value": 99,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertPasses(rule, event)
+        # Test data is 4 events in the current period and 2 events in the comparison period, so
+        # a 100% increase.
+        event = self.store_event(
+            data={
+                "fingerprint": ["something_random"],
+                "timestamp": iso_format(before_now(minutes=1)),
+                "user": {"id": uuid4().hex},
+            },
+            project_id=self.project.id,
+        )
+        self.increment(
+            event,
+            3,
+            timestamp=now() - timedelta(minutes=1),
+        )
+        self.increment(
+            event,
+            2,
+            timestamp=now() - timedelta(days=1, minutes=20),
+        )
+        data = {
+            "interval": "1h",
+            "value": 99,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertPasses(rule, event)
 
-            data = {
-                "interval": "1h",
-                "value": 101,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertDoesNotPass(rule, event)
+        data = {
+            "interval": "1h",
+            "value": 101,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertDoesNotPass(rule, event)
 
     def test_comparison_empty_comparison_period(self):
-        with freeze_time(before_now(days=1).replace(hour=12, minute=30, second=0, microsecond=0)):
-            # Test data is 1 event in the current period and 0 events in the comparison period. This
-            # should always result in 0 and never fire.
-            event = self.store_event(
-                data={
-                    "fingerprint": ["something_random"],
-                    "timestamp": iso_format(before_now(minutes=1)),
-                    "user": {"id": uuid4().hex},
-                },
-                project_id=self.project.id,
-            )
-            data = {
-                "interval": "1h",
-                "value": 0,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertDoesNotPass(rule, event)
+        # Test data is 1 event in the current period and 0 events in the comparison period. This
+        # should always result in 0 and never fire.
+        event = self.store_event(
+            data={
+                "fingerprint": ["something_random"],
+                "timestamp": iso_format(before_now(minutes=1)),
+                "user": {"id": uuid4().hex},
+            },
+            project_id=self.project.id,
+        )
+        data = {
+            "interval": "1h",
+            "value": 0,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertDoesNotPass(rule, event)
 
-            data = {
-                "interval": "1h",
-                "value": 100,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertDoesNotPass(rule, event)
+        data = {
+            "interval": "1h",
+            "value": 100,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertDoesNotPass(rule, event)
 
 
+@freeze_time((now() - timedelta(days=2)).replace(hour=12, minute=40, second=0, microsecond=0))
 class EventFrequencyConditionTestCase(
     FrequencyConditionMixin, StandardIntervalMixin, RuleTestCase, SnubaTestCase
 ):
@@ -189,7 +188,7 @@ class EventFrequencyConditionTestCase(
             )
 
 
-@freeze_time()
+@freeze_time((now() - timedelta(days=2)).replace(hour=12, minute=40, second=0, microsecond=0))
 class EventUniqueUserFrequencyConditionTestCase(
     FrequencyConditionMixin,
     StandardIntervalMixin,
@@ -215,16 +214,15 @@ class EventUniqueUserFrequencyConditionTestCase(
             )
 
 
-@freeze_time()
+@freeze_time((now() - timedelta(days=2)).replace(hour=12, minute=40, second=0, microsecond=0))
 class EventFrequencyPercentConditionTestCase(
     RuleTestCase,
     SnubaTestCase,
 ):
     rule_cls = EventFrequencyPercentCondition
 
-    def _make_sessions(self, num, minutes):
-        received = time.time() - minutes * 60
-        session_started = received // 60 * 60
+    def _make_sessions(self, num):
+        received = time.time()
 
         def make_session(i):
             return dict(
@@ -240,7 +238,7 @@ class EventFrequencyPercentConditionTestCase(
                 duration=None,
                 errors=0,
                 # The line below is crucial to spread sessions throughout the time period.
-                started=session_started - (i * (minutes / 30)),
+                started=received - i,
                 received=received,
             )
 
@@ -268,13 +266,12 @@ class EventFrequencyPercentConditionTestCase(
             )
         rule = self.get_rule(data=data, rule=Rule(environment_id=None))
         environment_rule = self.get_rule(data=data, rule=Rule(environment_id=self.environment.id))
-        with freeze_time(before_now(minutes=minutes)):
-            if passes:
-                self.assertPasses(rule, self.test_event)
-                self.assertPasses(environment_rule, self.test_event)
-            else:
-                self.assertDoesNotPass(rule, self.test_event)
-                self.assertDoesNotPass(environment_rule, self.test_event)
+        if passes:
+            self.assertPasses(rule, self.test_event)
+            self.assertPasses(environment_rule, self.test_event)
+        else:
+            self.assertDoesNotPass(rule, self.test_event)
+            self.assertDoesNotPass(environment_rule, self.test_event)
 
     def increment(self, event, count, environment=None, timestamp=None):
         data = {
@@ -294,7 +291,7 @@ class EventFrequencyPercentConditionTestCase(
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_five_minutes_with_events(self):
-        self._make_sessions(60, 5)
+        self._make_sessions(60)
         data = {"interval": "5m", "value": 39}
         self._run_test(data=data, minutes=5, passes=True, add_events=True)
         data = {"interval": "5m", "value": 41}
@@ -302,7 +299,7 @@ class EventFrequencyPercentConditionTestCase(
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_ten_minutes_with_events(self):
-        self._make_sessions(60, 10)
+        self._make_sessions(60)
         data = {"interval": "10m", "value": 49}
         self._run_test(data=data, minutes=10, passes=True, add_events=True)
         data = {"interval": "10m", "value": 51}
@@ -310,7 +307,7 @@ class EventFrequencyPercentConditionTestCase(
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_thirty_minutes_with_events(self):
-        self._make_sessions(60, 30)
+        self._make_sessions(60)
         data = {"interval": "30m", "value": 49}
         self._run_test(data=data, minutes=30, passes=True, add_events=True)
         data = {"interval": "30m", "value": 51}
@@ -318,7 +315,7 @@ class EventFrequencyPercentConditionTestCase(
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_one_hour_with_events(self):
-        self._make_sessions(60, 60)
+        self._make_sessions(60)
         data = {"interval": "1h", "value": 49}
         self._run_test(data=data, minutes=60, add_events=True, passes=True)
         data = {"interval": "1h", "value": 51}
@@ -326,69 +323,68 @@ class EventFrequencyPercentConditionTestCase(
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_five_minutes_no_events(self):
-        self._make_sessions(60, 5)
+        self._make_sessions(60)
         data = {"interval": "5m", "value": 39}
         self._run_test(data=data, minutes=5, passes=True, add_events=True)
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_ten_minutes_no_events(self):
-        self._make_sessions(60, 10)
+        self._make_sessions(60)
         data = {"interval": "10m", "value": 49}
         self._run_test(data=data, minutes=10, passes=True, add_events=True)
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_thirty_minutes_no_events(self):
-        self._make_sessions(60, 30)
+        self._make_sessions(60)
         data = {"interval": "30m", "value": 49}
         self._run_test(data=data, minutes=30, passes=True, add_events=True)
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_one_hour_no_events(self):
-        self._make_sessions(60, 60)
+        self._make_sessions(60)
         data = {"interval": "1h", "value": 49}
         self._run_test(data=data, minutes=60, passes=False)
 
     @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_comparison(self):
-        with freeze_time(before_now(minutes=0)):
-            self._make_sessions(10, 1)
-            # Create sessions for previous period
-            self._make_sessions(10, 60 * 24 + 20)
-
-            # Test data is 2 events in the current period and 1 events in the comparison period.
-            # Number of sessions is 20 in each period, so current period is 20% of sessions, prev
-            # is 10%. Overall a 100% increase comparitively.
-            event = self.store_event(
-                data={
-                    "fingerprint": ["something_random"],
-                    "timestamp": iso_format(before_now(minutes=1)),
-                },
-                project_id=self.project.id,
-            )
-            self.increment(
-                event,
-                1,
-                timestamp=now() - timedelta(minutes=1),
-            )
-            self.increment(
-                event,
-                1,
-                timestamp=now() - timedelta(days=1, minutes=20),
-            )
-            data = {
-                "interval": "1h",
-                "value": 99,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertPasses(rule, event)
+        self._make_sessions(10)
+        # Create sessions for previous period
+        self._make_sessions(10)
 
-            data = {
-                "interval": "1h",
-                "value": 101,
-                "comparisonType": "percent",
-                "comparisonInterval": "1d",
-            }
-            rule = self.get_rule(data=data, rule=Rule(environment_id=None))
-            self.assertDoesNotPass(rule, event)
+        # Test data is 2 events in the current period and 1 events in the comparison period.
+        # Number of sessions is 20 in each period, so current period is 20% of sessions, prev
+        # is 10%. Overall a 100% increase comparitively.
+        event = self.store_event(
+            data={
+                "fingerprint": ["something_random"],
+                "timestamp": iso_format(before_now(minutes=1)),
+            },
+            project_id=self.project.id,
+        )
+        self.increment(
+            event,
+            1,
+            timestamp=now() - timedelta(minutes=1),
+        )
+        self.increment(
+            event,
+            1,
+            timestamp=now() - timedelta(days=1, minutes=20),
+        )
+        data = {
+            "interval": "1h",
+            "value": 99,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertPasses(rule, event)
+
+        data = {
+            "interval": "1h",
+            "value": 101,
+            "comparisonType": "percent",
+            "comparisonInterval": "1d",
+        }
+        rule = self.get_rule(data=data, rule=Rule(environment_id=None))
+        self.assertDoesNotPass(rule, event)