integration_tests: bound two timing cases on what they can actually prove

src/ros2_medkit_integration_tests/test/features/test_graph_event_discovery.test.py

@@ -65,8 +65,14 @@
 )
 
 
-# Long backstop so any sub-backstop detection must come from graph events.
-BACKSTOP_INTERVAL_MS = 30000
+# The gateway's maximum accepted backstop interval, and it has to be the
+# maximum. The backstop timer is created during gateway initialisation and its
+# phase relative to a mid-run spawn is arbitrary, so a case that only measures
+# elapsed time cannot tell a graph-event refresh from a backstop sweep that
+# happened to land nearby. Pushing the FIRST sweep as far out as the parameter
+# allows lets the spawn case finish inside a window where no sweep has run yet;
+# PRE_BACKSTOP_BUDGET_SEC below is what keeps it inside that window.
+BACKSTOP_INTERVAL_MS = 60000
 
 # Demo nodes launched at startup.
 INITIAL_NODES = ['temp_sensor', 'rpm_sensor']
@@ -78,16 +84,24 @@
 #
 # Spawn detection is bounded by:
 #   process exec + rclcpp init + DDS announce + 100 ms poll + refresh_cache.
-# 5 s is comfortable; well under the 30 s backstop, so a pass proves the
-# graph-event poll fired the refresh.
-SPAWN_DETECTION_TIMEOUT = 5.0
-
-# Graph-event-driven detection should land in under a second; allow
-# generous CI jitter headroom but still well below the backstop. A
-# detection above this bound proves the backstop, not the graph event,
-# triggered the refresh - which is the regression this test exists to
-# catch.
-GRAPH_EVENT_MAX_LATENCY_SEC = 2.0
+# The poll sits above the latency bound below, so a detection that arrives late
+# reports the time it took instead of a bare timeout.
+SPAWN_DETECTION_TIMEOUT = 15.0
+
+# How long after the gateway first answered /health the spawn case may still
+# measure. The first backstop sweep runs BACKSTOP_INTERVAL_MS after gateway
+# initialisation, which precedes that first answer by well under a second, so a
+# measurement inside this budget is one no sweep could have served. That is what
+# makes the bound below a statement about the graph-event path.
+PRE_BACKSTOP_BUDGET_SEC = 30.0
+
+# The latency of the graph-event path itself, measured from process spawn. It
+# cannot be sub-second: the gateway coalesces graph events behind
+# discovery.refresh_debounce_ms, 1000 ms by default, and a spawn that arrives
+# mid-window waits for the next one, so detection lands on a multiple of the
+# debounce. Measured on a developer machine with the default settings, the
+# spread is roughly 1 s to 3.6 s.
+GRAPH_EVENT_MAX_LATENCY_SEC = 10.0
 
 # Initial discovery shares the budget with full gateway startup.
 INITIAL_DETECTION_TIMEOUT = 30.0
@@ -153,6 +167,10 @@ class TestGraphEventDiscovery(GatewayTestCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
+        # Reference point for PRE_BACKSTOP_BUDGET_SEC: the gateway has answered
+        # /health by the time the base class returns, so initialisation - and
+        # with it the backstop timer - started a moment earlier.
+        cls._health_at = time.monotonic()
         cls._extra_proc = None
 
     @classmethod
@@ -207,9 +225,10 @@ def test_initial_discovery_picks_up_startup_nodes(self):
     def test_new_node_detected_via_graph_event(self):
         """Spawning a node mid-run must propagate within the spawn budget.
 
-        ``BACKSTOP_INTERVAL_MS`` is 30 s; detection within
-        ``SPAWN_DETECTION_TIMEOUT`` (5 s) therefore proves the refresh
-        was triggered by a graph event, not the safety-backstop sweep.
+        The case runs before the first backstop sweep and checks that it did,
+        so the refresh it observes can only have come from a graph event. The
+        measured time starts at process spawn, so it also carries the node's
+        own startup and the gateway's event debounce.
         """
         # Make sure the initial graph is fully settled before spawning.
         for key in INITIAL_NODES:
@@ -240,10 +259,21 @@ def test_new_node_detected_via_graph_event(self):
                 interval=0.1,
             )
             elapsed = time.monotonic() - spawn_time
+            # Establish what was measured before bounding it. Past this budget
+            # a backstop sweep could have served the detection, and then the
+            # bound below would be reporting on the wrong mechanism.
+            since_health = time.monotonic() - type(self)._health_at
+            self.assertLess(
+                since_health, PRE_BACKSTOP_BUDGET_SEC,
+                f'detection landed {since_health:.3f}s after the gateway came up, '
+                f'past the {PRE_BACKSTOP_BUDGET_SEC}s window in which no backstop '
+                f'sweep can have run ({BACKSTOP_INTERVAL_MS}ms backstop), so this '
+                f'run cannot say what triggered the refresh',
+            )
             self.assertLess(
                 elapsed, GRAPH_EVENT_MAX_LATENCY_SEC,
-                f'Spawn detection took {elapsed:.3f}s - expected sub-second '
-                f'via graph-event poll, not backstop-driven '
+                f'Spawn detection took {elapsed:.3f}s - expected the '
+                f'graph-event poll to serve it, not the backstop sweep '
                 f'({BACKSTOP_INTERVAL_MS}ms backstop configured)',
             )
             app_ids = [app.get('id', '') for app in data.get('items', [])]

src/ros2_medkit_integration_tests/test/features/test_peer_recovery.test.py

@@ -825,15 +825,26 @@ def test_07_a_peer_owned_read_succeeds_again_and_the_peer_answered_it(self):
             'test_04 must watch this URL fail before test_07 can claim it recovered',
         )
 
+        # A 200 is not yet an answer here. The member re-subscribes when its
+        # peer comes back, and until its first sample arrives the read is a
+        # well-formed 'metadata_only' body with no payload - the same state
+        # case 1 waits through on the healthy peer. The budget below is what
+        # covers the gap; reading once and calling the empty body a failure
+        # makes the case race the publisher.
         def served():
             answer = self._aggregate_read_of_peer_topic()
-            return answer if answer.status_code == 200 else None
+            if answer.status_code != 200:
+                return None
+            payload = answer.json()
+            if payload.get('x-medkit', {}).get('status') != 'data' or not payload.get('data'):
+                return None
+            return answer
 
         response = _poll(served, timeout=RECOVERY_TIMEOUT)
         self.assertIsNotNone(
             response,
-            f'a read of {PEER_DECLARED_APP} never recovered after its peer came back; '
-            f'last answer was {self._aggregate_read_of_peer_topic().text}',
+            f"a read of {PEER_DECLARED_APP} never carried the member's sample after "
+            f'its peer came back; last answer was {self._aggregate_read_of_peer_topic().text}',
         )
 
         body = response.json()
@@ -869,6 +880,23 @@ def served():
             f'returned before the outage: {body}',
         )
 
+        # The poll above waits the member's subscription warm, which is what a
+        # first read after recovery has to do. Once it is warm a read carries
+        # the sample on the spot: the gateway holds the latest one and answers
+        # from it. A read that comes back empty here is not a cold-start
+        # transient, it is a member that serves nothing until asked twice.
+        warm = self._aggregate_read_of_peer_topic()
+        self.assertEqual(warm.status_code, 200, warm.text)
+        warm_body = warm.json()
+        self.assertEqual(
+            warm_body.get('x-medkit', {}).get('status'), 'data',
+            f'a second read of the recovered member came back without data: {warm_body}',
+        )
+        self.assertTrue(
+            warm_body.get('data'),
+            f'a second read of the recovered member carried an empty payload: {warm_body}',
+        )
+
     def test_08_the_retained_declaration_does_not_linger_beside_the_live_copy(self):
         """Recovery is a replacement, not an addition.