fix carryforward metric, mock feature/metrics in most tests (#384)

conftest.py

@@ -298,3 +298,29 @@ def mock_submit_fn(metric, start, end):
         "submit_subflow",
         mock_submit,
     )
+
+
+@pytest.fixture(autouse=True)
+def mock_metric_context(mocker, request):
+    if request.node.get_closest_marker("real_metric_context"):
+        return
+
+    from helpers.telemetry import MetricContext
+
+    def populate(self):
+        self.populated = True
+
+    return mocker.patch.object(MetricContext, "populate", populate)
+
+
+@pytest.fixture(autouse=True)
+def mock_feature(mocker, request):
+    if request.node.get_closest_marker("real_feature"):
+        return
+
+    from shared.rollouts import Feature
+
+    def check_value(self, *, owner_id=None, repo_id=None, default=False):
+        return default
+
+    return mocker.patch.object(Feature, "check_value", check_value)

helpers/telemetry.py

@@ -142,7 +142,7 @@ def log_simple_metric(self, name: str, value: float):
 
     @fire_and_forget
     async def attempt_log_simple_metric(self, name: str, value: float):
-        self.log_simple_metric(name, value)
+        await self.log_simple_metric_async(name, value)
 
 
 class TimeseriesTimer:

helpers/tests/unit/test_telemetry.py

@@ -47,6 +47,7 @@ def make_metric_context(dbsession):
 
 
 @pytest.mark.asyncio
+@pytest.mark.real_metric_context
 async def test_fire_and_forget():
     """
     @fire_and_forget wraps an async function in a sync function that schedules
@@ -84,6 +85,7 @@ async def fn():
 
 
 class TestMetricContext:
+    @pytest.mark.real_metric_context
     def test_populate_complete(self, dbsession, mocker):
         mc = make_metric_context(dbsession)
         assert mc.repo_id is not None
@@ -106,6 +108,7 @@ def test_populate_complete(self, dbsession, mocker):
         )
         assert mc.commit_id is not None
 
+    @pytest.mark.real_metric_context
     def test_populate_no_repo(self, dbsession, mocker):
         mc = make_metric_context(dbsession)
         mc.repo_id = None
@@ -119,6 +122,7 @@ def test_populate_no_repo(self, dbsession, mocker):
         assert mc.commit_id is None
 
     @pytest.mark.django_db(databases={"default", "timeseries"})
+    @pytest.mark.real_metric_context
     def test_log_simple_metric(self, dbsession, mocker):
         mc = make_metric_context(dbsession)
 
@@ -148,6 +152,7 @@ def test_log_simple_metric(self, dbsession, mocker):
         """
 
     @pytest.mark.asyncio
+    @pytest.mark.real_metric_context
     async def test_attempt_log_simple_metric(self, dbsession, mocker):
         mc = make_metric_context(dbsession)
         mock_fn = mocker.patch.object(mc, "log_simple_metric")
@@ -161,6 +166,7 @@ async def test_attempt_log_simple_metric(self, dbsession, mocker):
 
 
 class TestTimeseriesTimer:
+    @pytest.mark.real_metric_context
     def test_sync(self, dbsession, mocker):
         mc = Mock()
         time1 = datetime.fromisoformat("2023-11-21").replace(tzinfo=timezone.utc)
@@ -177,6 +183,7 @@ def test_sync(self, dbsession, mocker):
         assert ("test_sync_timer", expected_value) in mc.log_simple_metric.call_args
 
     @pytest.mark.asyncio
+    @pytest.mark.real_metric_context
     async def test_async(self, dbsession, mocker):
         mc = Mock()
         time1 = datetime.fromisoformat("2023-11-21").replace(tzinfo=timezone.utc)

pytest.ini

@@ -4,3 +4,5 @@ addopts = --sqlalchemy-connect-url="postgresql://postgres@postgres:5432/backgrou
 markers=
     integration: integration tests (includes tests with vcrs)
     real_checkpoint_logger: prevents use of stubbed CheckpointLogger
+    real_metric_context: prevents use of stubbed MetricContext
+    real_feature: prevents use of stubbed Feature

services/report/__init__.py

@@ -767,7 +767,7 @@ async def create_new_report_for_commit(self, commit: Commit) -> Report:
                     "Could not find parent for possible carryforward",
                     extra=dict(commit=commit.commitid, repoid=commit.repoid),
                 )
-                metric_context.attempt_log_simple_metric(
+                await metric_context.log_simple_metric_async(
                     "worker_service_report_carryforward_base_not_found", 1
                 )
                 return Report()
@@ -823,7 +823,7 @@ async def create_new_report_for_commit(self, commit: Commit) -> Report:
             await self._possibly_shift_carryforward_report(
                 carryforward_report, parent_commit, commit
             )
-            metric_context.attempt_log_simple_metric(
+            await metric_context.log_simple_metric_async(
                 "worker_service_report_carryforward_success", 1
             )
             return carryforward_report

services/tests/test_report.py

@@ -4980,6 +4980,7 @@ def fake_get_compare(base, head):
         }
 
     @pytest.mark.asyncio
+    @pytest.mark.django_db(databases={"default", "timeseries"})
     async def test_create_new_report_for_commit_and_shift(
         self, dbsession, sample_report, mocker, mock_repo_provider, mock_storage
     ):