tests(incidents): Add serializer parity test for incident activity (#113010)

When we identify issues with our API backport, I want to ensure we have
delta tests to cover that situation so all know differences there are
explicit.

Also added a 'unreliable' option to assert_serializer_parity, as some
fields (like activity.id) will sometimes be different and sometimes not
depending on what other tests have run. 'unreliable' allows us to
differentiate those from known differences, and likely has some other
legitimate uses.

Author: kcons

src/sentry/testutils/helpers/serializer_parity.py

@@ -10,6 +10,7 @@ def assert_serializer_parity(
     old: Mapping[str, Any],
     new: Mapping[str, Any],
     known_differences: set[str] | None = None,
+    unreliable: set[str] | None = None,
 ) -> None:
     """Assert that two serializer responses are equal, modulo known differences.
 
@@ -23,6 +24,14 @@ def assert_serializer_parity(
     Raises if any listed known difference is not actually different — unnecessary
     entries should be removed.
 
+    ``unreliable`` is a set of dot-separated field paths that may or may not
+    differ depending on test ordering (e.g. auto-incremented IDs from different
+    tables).  These fields are silently skipped with no "must actually differ"
+    check.  The field must still exist in both responses.
+
+    Use sparingly — ``unreliable`` masks both real regressions and reliable
+    consistency, so prefer ``known_differences`` when the field reliably differs.
+
     Examples::
 
         assert_serializer_parity(old=old, new=new)
@@ -32,10 +41,17 @@ def assert_serializer_parity(
             new=new,
             known_differences={"resolveThreshold", "triggers.resolveThreshold"},
         )
+
+        assert_serializer_parity(
+            old=old,
+            new=new,
+            unreliable={"activities.id"},
+        )
     """
     known_diffs = frozenset(known_differences or ())
+    unreliable_fields = frozenset(unreliable or ())
     checker = _ParityChecker()
-    checker.compare(old, new, known_diffs)
+    checker.compare(old, new, known_diffs, unreliable=unreliable_fields)
 
     assert not checker.mismatches, "Serializer differences:\n" + "\n".join(checker.mismatches)
 
@@ -58,23 +74,38 @@ class _ParityChecker:
     # known_diffs entries confirmed to be actual differences.
     confirmed: set[str] = field(default_factory=set)
 
-    def _nested_diffs(self, known_diffs: frozenset[str], key: str) -> frozenset[str]:
+    def _nested_fields(self, field_set: frozenset[str], key: str) -> frozenset[str]:
+        """Extract child paths for *key* from a dot-separated field set.
+
+        E.g. ``_nested_fields({"activities.id", "title"}, "activities")``
+        returns ``{"id"}``.
+        """
         prefix = key + "."
-        return frozenset(e[len(prefix) :] for e in known_diffs if e.startswith(prefix))
+        return frozenset(e[len(prefix) :] for e in field_set if e.startswith(prefix))
 
     def compare(
         self,
         old: Mapping[str, Any],
         new: Mapping[str, Any],
         known_diffs: frozenset[str],
         path: str = "",
-        kd_path: str = "",
+        diffs_path: str = "",
+        *,
+        unreliable: frozenset[str] = frozenset(),
     ) -> None:
         for key in set(list(old.keys()) + list(new.keys())):
             if key in known_diffs:
-                full_kd_key = _qualify(kd_path, key)
+                full_diffs_key = _qualify(diffs_path, key)
                 if key not in new or key not in old or old[key] != new[key]:
-                    self.confirmed.add(full_kd_key)
+                    self.confirmed.add(full_diffs_key)
+                continue
+
+            if key in unreliable:
+                full_path = _qualify(path, key)
+                if key not in new:
+                    self.mismatches.append(f"Missing from new: {full_path}")
+                elif key not in old:
+                    self.mismatches.append(f"Extra in new: {full_path}")
                 continue
 
             full_path = _qualify(path, key)
@@ -88,10 +119,11 @@ def compare(
 
             old_val = old[key]
             new_val = new[key]
-            nested = self._nested_diffs(known_diffs, key)
+            nested_diffs = self._nested_fields(known_diffs, key)
+            nested_unreliable = self._nested_fields(unreliable, key)
 
-            if nested:
-                child_kd_path = _qualify(kd_path, key)
+            if nested_diffs or nested_unreliable:
+                child_diffs_path = _qualify(diffs_path, key)
                 if isinstance(old_val, list) and isinstance(new_val, list):
                     if len(old_val) != len(new_val):
                         self.mismatches.append(
@@ -100,13 +132,27 @@ def compare(
                     for i, (old_item, new_item) in enumerate(zip(old_val, new_val)):
                         item_path = f"{full_path}[{i}]"
                         if isinstance(old_item, Mapping) and isinstance(new_item, Mapping):
-                            self.compare(old_item, new_item, nested, item_path, child_kd_path)
+                            self.compare(
+                                old_item,
+                                new_item,
+                                nested_diffs,
+                                item_path,
+                                child_diffs_path,
+                                unreliable=nested_unreliable,
+                            )
                         elif old_item != new_item:
                             self.mismatches.append(
                                 f"{item_path}: old={old_item!r}, new={new_item!r}"
                             )
                 elif isinstance(old_val, Mapping) and isinstance(new_val, Mapping):
-                    self.compare(old_val, new_val, nested, full_path, child_kd_path)
+                    self.compare(
+                        old_val,
+                        new_val,
+                        nested_diffs,
+                        full_path,
+                        child_diffs_path,
+                        unreliable=nested_unreliable,
+                    )
                 elif old_val != new_val:
                     self.mismatches.append(f"{full_path}: old={old_val!r}, new={new_val!r}")
             elif old_val != new_val:

tests/sentry/incidents/endpoints/test_organization_incident_index.py

@@ -8,10 +8,11 @@
 from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id
 from sentry.incidents.grouptype import MetricIssue
 from sentry.incidents.logic import update_incident_status
-from sentry.incidents.models.incident import IncidentStatus
+from sentry.incidents.models.incident import IncidentActivity, IncidentActivityType, IncidentStatus
 from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
 from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
 from sentry.models.groupopenperiod import GroupOpenPeriod
+from sentry.models.groupopenperiodactivity import GroupOpenPeriodActivity, OpenPeriodActivityType
 from sentry.silo.base import SiloMode
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.models import SnubaQuery, SnubaQueryEventType
@@ -284,6 +285,84 @@ def test_workflow_engine_serializer_matches_old_serializer(self) -> None:
                 },
             )
 
+    @freeze_time("2024-12-11 03:21:34")
+    def test_activity_serialization_parity(self) -> None:
+        self.create_team(organization=self.organization, members=[self.user])
+        self.login_as(self.user)
+
+        alert_rule = self.create_alert_rule()
+        trigger = self.create_alert_rule_trigger(alert_rule=alert_rule)
+        _, _, _, detector, _, _, _, _ = migrate_alert_rule(alert_rule)
+        migrate_metric_data_conditions(trigger)
+        migrate_resolve_threshold_data_condition(alert_rule)
+
+        incident = self.create_incident(alert_rule=alert_rule, status=IncidentStatus.CRITICAL.value)
+
+        with assume_test_silo_mode(SiloMode.CELL):
+            group = self.create_group(type=MetricIssue.type_id, project=self.project)
+            group.priority = PriorityLevel.HIGH.value
+            group.save()
+            DetectorGroup.objects.create(detector=detector, group=group)
+
+            group_open_period = GroupOpenPeriod.objects.get(group=group, project=self.project)
+            group_open_period.update(date_started=incident.date_started)
+
+            IncidentGroupOpenPeriod.objects.create(
+                group_open_period=group_open_period,
+                incident_id=incident.id,
+                incident_identifier=incident.identifier,
+            )
+
+            # Legacy activity: STATUS_CHANGE to CRITICAL
+            IncidentActivity.objects.create(
+                incident=incident,
+                type=IncidentActivityType.STATUS_CHANGE.value,
+                value=str(IncidentStatus.CRITICAL.value),
+                previous_value=None,
+            )
+
+            # Matching WE activity: OPENED with HIGH priority (maps to CRITICAL)
+            GroupOpenPeriodActivity.objects.create(
+                group_open_period=group_open_period,
+                type=OpenPeriodActivityType.OPENED,
+                value=PriorityLevel.HIGH,
+                date_added=incident.date_started,
+            )
+
+        with self.feature(["organizations:incidents", "organizations:performance-view"]):
+            old_resp = self.get_success_response(self.organization.slug, expand="activities")
+        old_data = old_resp.data
+        assert len(old_data) > 0
+
+        with self.feature(
+            [
+                "organizations:incidents",
+                "organizations:performance-view",
+                "organizations:workflow-engine-rule-serializers",
+            ]
+        ):
+            new_resp = self.get_success_response(self.organization.slug, expand="activities")
+        new_data = new_resp.data
+        assert len(new_data) == len(old_data)
+
+        for old_incident_data, new_incident_data in zip(old_data, new_data):
+            assert_serializer_parity(
+                old=old_incident_data,
+                new=new_incident_data,
+                known_differences={
+                    "alertRule.resolveThreshold",
+                    "alertRule.triggers.resolveThreshold",
+                    "alertRule.triggers.label",
+                    "title",
+                    # Legacy activities include fields the WE path intentionally omits
+                    "activities.incidentIdentifier",
+                    "activities.user",
+                    "activities.comment",
+                },
+                # Different tables with independent auto-increment sequences
+                unreliable={"activities.id"},
+            )
+
 
 class WorkflowEngineIncidentListTest(APITestCase):
     endpoint = "sentry-api-0-organization-incident-index"

tests/sentry/testutils/helpers/test_serializer_parity.py

@@ -0,0 +1,43 @@
+import pytest
+
+from sentry.testutils.helpers.serializer_parity import assert_serializer_parity
+
+
+class TestUnreliable:
+    def test_ignores_difference(self) -> None:
+        old = {"id": "1", "name": "a"}
+        new = {"id": "2", "name": "a"}
+        assert_serializer_parity(old=old, new=new, unreliable={"id"})
+
+    def test_ignores_match(self) -> None:
+        old = {"id": "1", "name": "a"}
+        new = {"id": "1", "name": "a"}
+        assert_serializer_parity(old=old, new=new, unreliable={"id"})
+
+    def test_nested_in_list(self) -> None:
+        old = {"items": [{"id": "1", "v": "x"}, {"id": "2", "v": "y"}]}
+        new = {"items": [{"id": "99", "v": "x"}, {"id": "100", "v": "y"}]}
+        assert_serializer_parity(old=old, new=new, unreliable={"items.id"})
+
+    def test_nested_in_dict(self) -> None:
+        old = {"outer": {"id": "1", "v": "x"}}
+        new = {"outer": {"id": "2", "v": "x"}}
+        assert_serializer_parity(old=old, new=new, unreliable={"outer.id"})
+
+    def test_still_fails_on_other_fields(self) -> None:
+        old = {"id": "1", "name": "a"}
+        new = {"id": "2", "name": "b"}
+        with pytest.raises(AssertionError, match="name"):
+            assert_serializer_parity(old=old, new=new, unreliable={"id"})
+
+    def test_fails_if_missing_from_new(self) -> None:
+        old = {"id": "1", "name": "a"}
+        new = {"name": "a"}
+        with pytest.raises(AssertionError, match="Missing from new"):
+            assert_serializer_parity(old=old, new=new, unreliable={"id"})
+
+    def test_fails_if_extra_in_new(self) -> None:
+        old = {"name": "a"}
+        new = {"id": "1", "name": "a"}
+        with pytest.raises(AssertionError, match="Extra in new"):
+            assert_serializer_parity(old=old, new=new, unreliable={"id"})