Browse Source

feat(tests): Add `save_new_event` helper (#64248)

This PR adds a new test helper, `save_new_event`, which creates an event in the given project with the given data. In case the tests needs async parts of the save process to have run (for instance, if we're testing group metadata being updated), it runs the save using `TaskRunner`, which forces all of the async parts to be synchronous. 

(I finally got tired of writing the same three lines over and over, and continually feeling like I should add a comment every time we used `with self.tasks()` (which creates an instance of `TaskRunner`) about why that was necessary.)

In order to show that the helper works, I switched the event manager grouping tests to use it, without changing any of the assertions*. They all still pass!

*This is not technically true. I added two assertions which seemed obviously to be missing (about events belonging to the same group, or not), and I fixed one which was only passing because we were creating two events with identical timestamps.

In switching all of the tests over, I also removed any unnecessary data being included in the created events, to make it easier to see what the test was about. I also was able to get rid of the `make_event` helper, which wasn't really necessary in the first place, and which only was there because it hitchhiked along when the tests were moved into their own file.
Katie Byers 1 year ago
parent
commit
d17543357a

+ 20 - 0
src/sentry/testutils/helpers/eventprocessing.py

@@ -1,4 +1,10 @@
+from typing import Any
+
+from sentry.event_manager import EventManager
+from sentry.eventstore.models import Event
 from sentry.eventstore.processing import event_processing_store
 from sentry.eventstore.processing import event_processing_store
+from sentry.models.project import Project
+from sentry.testutils.helpers.task_runner import TaskRunner
 
 
 
 
 def write_event_to_cache(event):
 def write_event_to_cache(event):
@@ -6,3 +12,17 @@ def write_event_to_cache(event):
     cache_data["event_id"] = event.event_id
     cache_data["event_id"] = event.event_id
     cache_data["project"] = event.project_id
     cache_data["project"] = event.project_id
     return event_processing_store.store(cache_data)
     return event_processing_store.store(cache_data)
+
+
+def save_new_event(event_data: dict[str, Any], project: Project) -> Event:
+    """
+    Save a new event with the given data, returning the Event object.
+
+    Note: Runs async tasks, like updating group metadata, synchronously, so the results can be
+    tested.
+    """
+    # This makes async tasks synchronous, by setting `CELERY_ALWAYS_EAGER = True`.
+    with TaskRunner():
+        event = EventManager(event_data).save(project.id)
+
+    return event

+ 40 - 77
tests/sentry/event_manager/test_event_manager_grouping.py

@@ -1,17 +1,15 @@
 from __future__ import annotations
 from __future__ import annotations
 
 
-import logging
-import uuid
 from time import time
 from time import time
 from unittest import mock
 from unittest import mock
 from unittest.mock import MagicMock
 from unittest.mock import MagicMock
 
 
 from sentry import tsdb
 from sentry import tsdb
-from sentry.event_manager import EventManager
 from sentry.grouping.result import CalculatedHashes
 from sentry.grouping.result import CalculatedHashes
 from sentry.models.group import Group
 from sentry.models.group import Group
 from sentry.testutils.cases import TestCase
 from sentry.testutils.cases import TestCase
-from sentry.testutils.helpers.datetime import before_now, freeze_time, iso_format
+from sentry.testutils.helpers.datetime import freeze_time
+from sentry.testutils.helpers.eventprocessing import save_new_event
 from sentry.testutils.silo import region_silo_test
 from sentry.testutils.silo import region_silo_test
 from sentry.testutils.skips import requires_snuba
 from sentry.testutils.skips import requires_snuba
 from sentry.tsdb.base import TSDBModel
 from sentry.tsdb.base import TSDBModel
@@ -19,17 +17,6 @@ from sentry.tsdb.base import TSDBModel
 pytestmark = [requires_snuba]
 pytestmark = [requires_snuba]
 
 
 
 
-def make_event(**kwargs):
-    result = {
-        "event_id": uuid.uuid1().hex,
-        "level": logging.ERROR,
-        "logger": "default",
-        "tags": [],
-    }
-    result.update(kwargs)
-    return result
-
-
 def get_relevant_metrics_calls(mock_fn: MagicMock, key: str) -> list[mock._Call]:
 def get_relevant_metrics_calls(mock_fn: MagicMock, key: str) -> list[mock._Call]:
     return [call for call in mock_fn.call_args_list if call.args[0] == key]
     return [call for call in mock_fn.call_args_list if call.args[0] == key]
 
 
@@ -37,78 +24,67 @@ def get_relevant_metrics_calls(mock_fn: MagicMock, key: str) -> list[mock._Call]
 @region_silo_test
 @region_silo_test
 class EventManagerGroupingTest(TestCase):
 class EventManagerGroupingTest(TestCase):
     def test_puts_events_with_matching_fingerprints_in_same_group(self):
     def test_puts_events_with_matching_fingerprints_in_same_group(self):
-        ts = time() - 200
-        manager = EventManager(
-            make_event(message="foo", event_id="a" * 32, fingerprint=["a" * 32], timestamp=ts)
+        event = save_new_event(
+            {"message": "Dogs are great!", "fingerprint": ["maisey"]}, self.project
         )
         )
-        with self.tasks():
-            event = manager.save(self.project.id)
-
-        manager = EventManager(
-            make_event(message="foo bar", event_id="b" * 32, fingerprint=["a" * 32], timestamp=ts)
+        # Normally this should go into a different group, since the messages don't match, but the
+        # fingerprint takes precedence.
+        event2 = save_new_event(
+            {"message": "Adopt don't shop", "fingerprint": ["maisey"]}, self.project
         )
         )
-        with self.tasks():
-            event2 = manager.save(self.project.id)
+
+        assert event.group_id == event2.group_id
 
 
         group = Group.objects.get(id=event.group_id)
         group = Group.objects.get(id=event.group_id)
 
 
         assert group.times_seen == 2
         assert group.times_seen == 2
-        assert group.last_seen == event.datetime
+        assert group.last_seen == event2.datetime
         assert group.message == event2.message
         assert group.message == event2.message
 
 
     def test_puts_events_with_different_fingerprints_in_different_groups(self):
     def test_puts_events_with_different_fingerprints_in_different_groups(self):
-        manager = EventManager(
-            make_event(message="foo", event_id="a" * 32, fingerprint=["{{ default }}", "a" * 32])
+        event = save_new_event(
+            {"message": "Dogs are great!", "fingerprint": ["maisey"]}, self.project
         )
         )
-        with self.tasks():
-            manager.normalize()
-            event = manager.save(self.project.id)
-
-        manager = EventManager(
-            make_event(message="foo bar", event_id="b" * 32, fingerprint=["a" * 32])
+        # Normally this should go into the same group, since the message matches, but the
+        # fingerprint takes precedence.
+        event2 = save_new_event(
+            {"message": "Dogs are great!", "fingerprint": ["charlie"]}, self.project
         )
         )
-        with self.tasks():
-            manager.normalize()
-            event2 = manager.save(self.project.id)
 
 
         assert event.group_id != event2.group_id
         assert event.group_id != event2.group_id
 
 
     def test_adds_default_fingerprint_if_none_in_event(self):
     def test_adds_default_fingerprint_if_none_in_event(self):
-        manager = EventManager(make_event())
-        manager.normalize()
-        event = manager.save(self.project.id)
+        event = save_new_event({"message": "Dogs are great!"}, self.project)
 
 
         assert event.data.get("fingerprint") == ["{{ default }}"]
         assert event.data.get("fingerprint") == ["{{ default }}"]
 
 
     @freeze_time()
     @freeze_time()
     def test_ignores_fingerprint_on_transaction_event(self):
     def test_ignores_fingerprint_on_transaction_event(self):
-        manager1 = EventManager(make_event(event_id="a" * 32, fingerprint="fingerprint1"))
-        event1 = manager1.save(self.project.id)
-
-        manager2 = EventManager(
-            make_event(
-                event_id="b" * 32,
-                fingerprint="fingerprint1",
-                transaction="wait",
-                contexts={
+        event1 = save_new_event(
+            {"message": "Dogs are great!", "fingerprint": ["charlie"]}, self.project
+        )
+        event2 = save_new_event(
+            {
+                "transaction": "dogpark",
+                "fingerprint": ["charlie"],
+                "type": "transaction",
+                "contexts": {
                     "trace": {
                     "trace": {
-                        "parent_span_id": "bce14471e0e9654d",
-                        "op": "foobar",
-                        "trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
-                        "span_id": "bf5be759039ede9a",
+                        "parent_span_id": "1121201212312012",
+                        "op": "sniffing",
+                        "trace_id": "11212012123120120415201309082013",
+                        "span_id": "1231201211212012",
                     }
                     }
                 },
                 },
-                spans=[],
-                timestamp=iso_format(before_now(minutes=1)),
-                start_timestamp=iso_format(before_now(minutes=1)),
-                type="transaction",
-                platform="python",
-            )
+                "start_timestamp": time(),
+                "timestamp": time(),
+            },
+            self.project,
         )
         )
-        event2 = manager2.save(self.project.id)
 
 
         assert event1.group is not None
         assert event1.group is not None
         assert event2.group is None
         assert event2.group is None
+        assert event1.group_id != event2.group_id
         assert (
         assert (
             tsdb.backend.get_sums(
             tsdb.backend.get_sums(
                 TSDBModel.project,
                 TSDBModel.project,
@@ -133,14 +109,7 @@ class EventManagerGroupingTest(TestCase):
 
 
     def test_none_exception(self):
     def test_none_exception(self):
         """Test that when the exception is None, the group is still formed."""
         """Test that when the exception is None, the group is still formed."""
-        manager = EventManager(
-            make_event(
-                exception=None,
-            )
-        )
-        with self.tasks():
-            manager.normalize()
-            event = manager.save(self.project.id)
+        event = save_new_event({"exception": None}, self.project)
 
 
         assert event.group
         assert event.group
 
 
@@ -153,9 +122,7 @@ class EventManagerGroupingMetricsTest(TestCase):
         project.update_option("sentry:grouping_config", "legacy:2019-03-12")
         project.update_option("sentry:grouping_config", "legacy:2019-03-12")
         project.update_option("sentry:secondary_grouping_config", None)
         project.update_option("sentry:secondary_grouping_config", None)
 
 
-        manager = EventManager(make_event(message="dogs are great"))
-        manager.normalize()
-        manager.save(project.id)
+        save_new_event({"message": "Dogs are great!"}, self.project)
 
 
         hashes_calculated_calls = get_relevant_metrics_calls(
         hashes_calculated_calls = get_relevant_metrics_calls(
             mock_metrics_incr, "grouping.hashes_calculated"
             mock_metrics_incr, "grouping.hashes_calculated"
@@ -167,9 +134,7 @@ class EventManagerGroupingMetricsTest(TestCase):
         project.update_option("sentry:secondary_grouping_config", "legacy:2019-03-12")
         project.update_option("sentry:secondary_grouping_config", "legacy:2019-03-12")
         project.update_option("sentry:secondary_grouping_expiry", time() + 3600)
         project.update_option("sentry:secondary_grouping_expiry", time() + 3600)
 
 
-        manager = EventManager(make_event(message="dogs are great"))
-        manager.normalize()
-        manager.save(project.id)
+        save_new_event({"message": "Dogs are great!"}, self.project)
 
 
         hashes_calculated_calls = get_relevant_metrics_calls(
         hashes_calculated_calls = get_relevant_metrics_calls(
             mock_metrics_incr, "grouping.hashes_calculated"
             mock_metrics_incr, "grouping.hashes_calculated"
@@ -206,9 +171,7 @@ class EventManagerGroupingMetricsTest(TestCase):
                         hashes=secondary_hashes, hierarchical_hashes=[], tree_labels=[]
                         hashes=secondary_hashes, hierarchical_hashes=[], tree_labels=[]
                     ),
                     ),
                 ):
                 ):
-                    manager = EventManager(make_event(message="dogs are great"))
-                    manager.normalize()
-                    manager.save(project.id)
+                    save_new_event({"message": "Dogs are great!"}, self.project)
 
 
                     hash_comparison_calls = get_relevant_metrics_calls(
                     hash_comparison_calls = get_relevant_metrics_calls(
                         mock_metrics_incr, "grouping.hash_comparison"
                         mock_metrics_incr, "grouping.hash_comparison"