(wip) test: No more separate record_lost_transaction

Author: szokeasaurusrex

tests/test_client.py

@@ -1,4 +1,3 @@
-import contextlib
 import os
 import json
 import subprocess
@@ -1223,48 +1222,6 @@ class TestSpanClientReports:
     Tests for client reports related to spans.
     """
 
-    class LostEventCapturingTransport(sentry_sdk.Transport):
-        """
-        A transport that captures lost events.
-        """
-
-        def __init__(self):
-            self.record_lost_event_calls = []
-            self.record_lost_transaction_calls = []
-
-        def capture_envelope(self, _):
-            pass
-
-        def record_lost_event(
-            self,
-            reason,
-            data_category=None,
-            item=None,
-            *,
-            quantity=1,
-        ):
-            self.record_lost_event_calls.append((reason, data_category, item, quantity))
-
-        def record_lost_transaction(
-            self,
-            reason,  # type: str
-            span_count,  # type: int
-        ):  # type: (...) -> None
-            self.record_lost_transaction_calls.append((reason, span_count))
-
-    @staticmethod
-    @contextlib.contextmanager
-    def patch_transport():
-        """Patches the transport with a new LostEventCapturingTransport, which we yield."""
-        old_transport = sentry_sdk.get_client().transport
-        new_transport = TestSpanClientReports.LostEventCapturingTransport()
-        sentry_sdk.get_client().transport = new_transport
-
-        try:
-            yield new_transport
-        finally:
-            sentry_sdk.get_client().transport = old_transport
-
     @staticmethod
     def span_dropper(spans_to_drop):
         """
@@ -1293,23 +1250,19 @@ def __init__(self, span_count):
         """Configures a test case with the number of spans dropped and whether the transaction was dropped."""
         self.span_count = span_count
         self.expected_record_lost_event_calls = Counter()
-        self.expected_record_lost_transaction_calls = Counter()
         self.before_send = lambda event, _: event
         self.event_processor = lambda event, _: event
-        self.already_dropped_spans = 0
 
-    def _update_resulting_calls(
-        self, reason, drops_transaction=False, drops_spans=None
-    ):
+    def _update_resulting_calls(self, reason, drops_transactions=0, drops_spans=0):
         """
         Updates the expected calls with the given resulting calls.
         """
-        if drops_transaction:
-            dropped_spans = self.span_count - self.already_dropped_spans
-            self.expected_record_lost_transaction_calls[(reason, dropped_spans)] += 1
+        if drops_transactions > 0:
+            self.expected_record_lost_event_calls[
+                (reason, "transaction", None, drops_transactions)
+            ] += 1
 
-        elif drops_spans is not None:
-            self.already_dropped_spans += drops_spans
+        if drops_spans > 0:
             self.expected_record_lost_event_calls[
                 (reason, "span", None, drops_spans)
             ] += 1
@@ -1318,14 +1271,13 @@ def with_before_send(
         self,
         before_send,
         *,
-        drops_transaction=False,
-        drops_spans=None,
+        drops_transactions=0,
+        drops_spans=0,
     ):
-        """drops_transaction and drops_spans are mutually exclusive."""
         self.before_send = before_send
         self._update_resulting_calls(
             "before_send",
-            drops_transaction,
+            drops_transactions,
             drops_spans,
         )
 
@@ -1335,73 +1287,87 @@ def with_event_processor(
         self,
         event_processor,
         *,
-        drops_transaction=False,
-        drops_spans=None,
+        drops_transactions=0,
+        drops_spans=0,
     ):
         self.event_processor = event_processor
         self._update_resulting_calls(
             "event_processor",
-            drops_transaction,
+            drops_transactions,
             drops_spans,
         )
 
         return self
 
-    def run(self):
+    def run(self, sentry_init, capture_record_lost_event_calls):
         """Runs the test case with the configured parameters."""
-        sentry_sdk.init(before_send_transaction=self.before_send)
+        sentry_init(before_send_transaction=self.before_send)
+        record_lost_event_calls = capture_record_lost_event_calls()
 
         with sentry_sdk.isolation_scope() as scope:
             scope.add_event_processor(self.event_processor)
-            with self.patch_transport() as transport:
-                event = self.mock_transaction_event(self.span_count)
-                sentry_sdk.get_client().capture_event(event, scope=scope)
+            event = self.mock_transaction_event(self.span_count)
+            sentry_sdk.get_client().capture_event(event, scope=scope)
 
         # We use counters to ensure that the calls are made the expected number of times, disregarding order.
-        assert (
-            Counter(transport.record_lost_event_calls)
-            == self.expected_record_lost_event_calls
-        )
-        assert (
-            Counter(transport.record_lost_transaction_calls)
-            == self.expected_record_lost_transaction_calls
-        )
+        assert Counter(record_lost_event_calls) == self.expected_record_lost_event_calls
 
 
 @pytest.mark.parametrize(
     "test_config",
     (
-        TestSpanClientReports(10),  # No spans dropped
-        TestSpanClientReports(0).with_before_send(
-            lambda e, _: None, drops_transaction=True
+        TestSpanClientReports(span_count=10),  # No spans dropped
+        TestSpanClientReports(span_count=0).with_before_send(
+            lambda e, _: None,
+            drops_transactions=1,
+            drops_spans=1,
         ),
-        TestSpanClientReports(10).with_before_send(
-            lambda e, _: None, drops_transaction=True
+        TestSpanClientReports(span_count=10).with_before_send(
+            lambda e, _: None,
+            drops_transactions=1,
+            drops_spans=11,
         ),
-        TestSpanClientReports(10).with_before_send(
-            TestSpanClientReports.span_dropper(3), drops_spans=3
+        TestSpanClientReports(span_count=10).with_before_send(
+            TestSpanClientReports.span_dropper(3),
+            drops_spans=3,
         ),
-        TestSpanClientReports(10).with_before_send(
-            TestSpanClientReports.span_dropper(10), drops_spans=10
+        TestSpanClientReports(span_count=10).with_before_send(
+            TestSpanClientReports.span_dropper(10),
+            drops_spans=10,
         ),
-        TestSpanClientReports(10).with_event_processor(
-            lambda e, _: None, drops_transaction=True
+        TestSpanClientReports(span_count=10).with_event_processor(
+            lambda e, _: None,
+            drops_transactions=1,
+            drops_spans=11,
         ),
-        TestSpanClientReports(10).with_event_processor(
-            TestSpanClientReports.span_dropper(3), drops_spans=3
+        TestSpanClientReports(span_count=10).with_event_processor(
+            TestSpanClientReports.span_dropper(3),
+            drops_spans=3,
         ),
-        TestSpanClientReports(10).with_event_processor(
-            TestSpanClientReports.span_dropper(10), drops_spans=10
+        TestSpanClientReports(span_count=10).with_event_processor(
+            TestSpanClientReports.span_dropper(10),
+            drops_spans=10,
+        ),
+        TestSpanClientReports(span_count=10)
+        .with_event_processor(
+            TestSpanClientReports.span_dropper(3),
+            drops_spans=3,
+        )
+        .with_before_send(
+            TestSpanClientReports.span_dropper(5),
+            drops_spans=5,
         ),
         TestSpanClientReports(10)
-        .with_event_processor(TestSpanClientReports.span_dropper(3), drops_spans=3)
-        .with_before_send(TestSpanClientReports.span_dropper(5), drops_spans=5),
-        TestSpanClientReports(10)
-        .with_event_processor(TestSpanClientReports.span_dropper(3), drops_spans=3)
+        .with_event_processor(
+            TestSpanClientReports.span_dropper(3),
+            drops_spans=3,
+        )
         .with_before_send(
-            lambda e, _: None, drops_transaction=True
-        ),  # Test proper number of spans with each reason
+            lambda e, _: None,
+            drops_transactions=1,
+            drops_spans=8,  # 3 of the 11 (incl. transaction) spans already dropped
+        ),
     ),
 )
-def test_dropped_transaction(test_config):
-    test_config.run()
+def test_dropped_transaction(sentry_init, capture_record_lost_event_calls, test_config):
+    test_config.run(sentry_init, capture_record_lost_event_calls)

tests/test_transport.py

@@ -669,14 +669,6 @@ class TestCustomHubClass(sentry_sdk.Hub):
         assert transport.hub_cls is TestCustomHubClass
 
 
-def test_record_lost_event_transaction_warning():
-    transport = http_transport(send_client_reports=True)
-
-    # Ensure a warning related to lost transaction events is emitted
-    with pytest.warns(UserWarning, match=r".+record_lost_transaction.+"):
-        transport.record_lost_event(reason="foo", data_category="transaction")
-
-
 @pytest.mark.parametrize("quantity", (1, 2, 10))
 def test_record_lost_event_quantity(capturing_server, make_client, quantity):
     client = make_client()
@@ -698,11 +690,17 @@ def test_record_lost_event_quantity(capturing_server, make_client, quantity):
     ]
 
 
-def assert_transaction_report(capturing_server, span_quantity):
-    """
-    Convenience function to assert that the capturing server captured a client report with 1 transaction and
-    `span_quantity` spans.
-    """
+@pytest.mark.parametrize("span_count", (0, 1, 2, 10))
+def test_record_lost_event_transaction_item(capturing_server, make_client, span_count):
+    client = make_client()
+    transport = client.transport
+
+    envelope = mock_transaction_envelope(span_count)
+    (transaction_item,) = envelope.items
+
+    transport.record_lost_event(reason="test", item=transaction_item)
+    client.flush()
+
     (captured,) = capturing_server.captured  # Should only be one envelope
     envelope = captured.envelope
     (item,) = envelope.items  # Envelope should only have one item
@@ -723,36 +721,5 @@ def assert_transaction_report(capturing_server, span_quantity):
     assert {
         "category": "span",
         "reason": "test",
-        "quantity": span_quantity,
+        "quantity": span_count + 1,
     } in discarded_events
-
-
-@pytest.mark.parametrize("span_count", (0, 1, 2, 10))
-def test_record_lost_transaction(capturing_server, make_client, span_count):
-    client = make_client()
-    transport = client.transport
-
-    transport.record_lost_transaction(reason="test", span_count=span_count)
-    client.flush()
-
-    assert_transaction_report(
-        capturing_server,
-        span_quantity=span_count + 1,  # +1 for the transaction itself
-    )
-
-
-@pytest.mark.parametrize("span_count", (0, 1, 2, 10))
-def test_record_lost_event_transaction_item(capturing_server, make_client, span_count):
-    client = make_client()
-    transport = client.transport
-
-    envelope = mock_transaction_envelope(span_count)
-    (transaction_item,) = envelope.items
-
-    transport.record_lost_event(reason="test", item=transaction_item)
-    client.flush()
-
-    assert_transaction_report(
-        capturing_server,
-        span_quantity=span_count + 1,  # +1 for the transaction itself
-    )