PR comments

Author: Shastick

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

@@ -156,7 +156,7 @@ Verify that the version of the subscription returned by the DSS is as expected
 
 Attempt to query and search for the deleted subscription in various ways
 
-#### 🛑 Secondary DSS should not return the deleted subscription check
+#### 🛑 DSS should not return the deleted subscription check
 
 If a DSS returns a subscription that was previously successfully deleted from the primary DSS,
 either one of the primary DSS or the DSS that returned the subscription is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**.
@@ -177,15 +177,9 @@ Verify that the subscription returned by the DSS via the deletion is properly fo
 
 Verify that the version of the subscription returned by the DSS is as expected
 
-#### 🛑 Secondary DSS should not return the deleted subscription check
+#### 🛑 DSS should not return the deleted subscription check
 
 If a DSS returns a subscription that was previously successfully deleted from the primary DSS,
 either one of the primary DSS or the DSS that returned the subscription is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**.
 
-#### 🛑 Primary DSS should not return the deleted subscription check
-
-If the primary DSS returns a subscription that was previously successfully deleted from a secondary DSS,
-either one of the secondary or primary DSS is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**.
-
-
 ## [Cleanup](../clean_workspace.md)

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

@@ -220,8 +220,6 @@ def _ensure_no_active_subs_exist(self):
     def _step_create_subscriptions(self):
         # Create the 'main' test subscription:
         main_sub = self._create_sub_with_params(self._sub_params)
-        if main_sub is None:
-            return
 
         self._current_subscription = main_sub
 
@@ -230,14 +228,12 @@ def _step_create_subscriptions(self):
             params = self._sub_params.copy()
             params.sub_id = sub_id
             extra_sub = self._create_sub_with_params(params)
-            if extra_sub is None:
-                return
             self._subs_for_deletion[sub_id] = extra_sub
             self._subs_for_deletion_params[sub_id] = params
 
     def _create_sub_with_params(
         self, creation_params: SubscriptionParams
-    ) -> Optional[Subscription]:
+    ) -> Subscription:
 
         # TODO migrate to the try/except pattern for queries
         newly_created = self._dss.upsert_subscription(
@@ -255,7 +251,6 @@ def _create_sub_with_params(
                     details=f"Subscription creation failed with status code {newly_created.status_code}",
                     query_timestamps=[newly_created.request.timestamp],
                 )
-                return None
 
         with self.check(
             "Create subscription response is correct", [self._primary_pid]
@@ -556,7 +551,7 @@ def _compare_upsert_resp_with_params(
 
     def _mutate_subscription_with_dss(
         self, dss_instance: DSSInstance, new_params: SubscriptionParams
-    ) -> bool:
+    ):
         """
         Mutate the subscription on the given DSS instance using the given parameters.
         Also updates the internal state of the scenario to reflect the new subscription.
@@ -574,7 +569,6 @@ def _mutate_subscription_with_dss(
                     details=f"Subscription mutation failed with status code {mutated_sub_response.status_code}",
                     query_timestamps=[mutated_sub_response.request.timestamp],
                 )
-                return False
 
         # Check that what we get back is valid and corresponds to what we want to create
         self._compare_upsert_resp_with_params(
@@ -584,7 +578,6 @@ def _mutate_subscription_with_dss(
         self._current_subscription = mutated_sub_response.subscription
         # Update the parameters we used for that subscription
         self._sub_params = new_params
-        return True
 
     def _step_mutate_subscriptions_broadcast_shift_time(self):
         """Mutate the subscription on the primary DSS by adding 10 seconds to its start and end times"""
@@ -599,12 +592,10 @@ def _step_mutate_subscriptions_secondaries_shift_time(self):
 
         for secondary_dss in self._dss_read_instances:
             # Mutate the subscription on the secondary DSS
-            if not self._mutate_subscription_with_dss(
+            self._mutate_subscription_with_dss(
                 secondary_dss,
                 shift_time_params(self._sub_params, timedelta(seconds=10)),
-            ):
-                # If the mutation failed but the scenario has not terminated, we end this step here.
-                return
+            )
             # Check that the mutation was propagated to every DSS:
             self._query_secondaries_and_compare(self._sub_params)
 
@@ -674,29 +665,42 @@ def _step_delete_subscriptions_on_secondaries(self):
                 # If the deletion failed but the scenario has not terminated, we end this step here.
                 return
             # Check that the primary knows about the deletion:
-            self._confirm_dss_has_no_sub(self._dss, sub_id, is_primary=True)
+            self._confirm_dss_has_no_sub(
+                self._dss, sub_id, secondary_dss.participant_id
+            )
             # Check that the deletion was propagated to every DSS:
-            self._confirm_no_secondary_has_sub(sub_id)
+            self._confirm_no_secondary_has_sub(sub_id, secondary_dss.participant_id)
 
     def _step_get_deleted_sub(self):
-        self._confirm_no_secondary_has_sub(self._sub_id)
+        self._confirm_no_secondary_has_sub(self._sub_id, self._dss.participant_id)
 
-    def _confirm_no_secondary_has_sub(self, sub_id: str):
+    def _confirm_no_secondary_has_sub(
+        self, sub_id: str, deleted_on_participant_id: str
+    ):
+        """Confirm that no secondary DSS has the subscription.
+        deleted_on_participant_id specifies the participant_id of the DSS where the subscription was deleted."""
         for secondary_dss in self._dss_read_instances:
-            self._confirm_dss_has_no_sub(secondary_dss, sub_id, is_primary=False)
+            self._confirm_dss_has_no_sub(
+                secondary_dss, sub_id, deleted_on_participant_id
+            )
 
     def _confirm_dss_has_no_sub(
-        self, dss_instance: DSSInstance, sub_id: str, is_primary: bool = False
+        self,
+        dss_instance: DSSInstance,
+        sub_id: str,
+        other_participant_id: Optional[str],
     ):
-        check_name = (
-            "Primary DSS should not return the deleted subscription"
-            if is_primary
-            else "Secondary DSS should not return the deleted subscription"
+        """Confirm that a DSS has no subscription.
+        other_participant_id may be specified if a failed check may be caused by it."""
+        participants = (
+            [dss_instance.participant_id, other_participant_id]
+            if other_participant_id
+            else None
         )
         fetched_sub = dss_instance.get_subscription(sub_id)
         with self.check(
-            check_name,
-            [dss_instance.participant_id],
+            "DSS should not return the deleted subscription",
+            participants,
         ) as check:
             if fetched_sub.status_code != 404:
                 check.record_failed(

monitoring/uss_qualifier/scenarios/astm/utm/dss/test_step_fragments.py

@@ -51,7 +51,7 @@ def cleanup_sub(
         if existing_sub.status_code not in [200, 404]:
             check.record_failed(
                 summary=f"Could not query subscription {sub_id}",
-                details=f"When attempting to query subscription {sub_id} from the DSS, received {existing_sub.status_code}: {existing_sub.json_result}",
+                details=f"When attempting to query subscription {sub_id} from the DSS, received {existing_sub.status_code}: {existing_sub.error_message}",
                 query_timestamps=[existing_sub.request.timestamp],
             )