latest PR comments

Author: Shastick

monitoring/prober/infrastructure.py

@@ -100,7 +100,7 @@ def wrapper_default_scope(*args, **kwargs):
 resource_type_code_descriptions: Dict[ResourceType, str] = {}
 
 
-# Next code: 381
+# Next code: 382
 def register_resource_type(code: int, description: str) -> ResourceType:
     """Register that the specified code refers to the described resource.
 

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

@@ -48,15 +48,18 @@ For the purpose of this scenario, the `second_utm_auth` resource must provide ac
 
 ##### Separate subscription
 
-Note that the subscription (or 'sub' claim, not to be confused with an SCD DSS subscription) of the token that will be obtained for this resource
+Note that the subject (or 'sub' claim) of the token that will be obtained for this resource
 MUST be different from the one of the `dss` resources mentioned above:
 this will be verified at runtime, and the depending checks will not be run if this is not the case.
 
 ## Setup test case
 
 ### [Ensure clean workspace test step](../clean_workspace.md)
 
-This step ensures that no subscription with the known test ID exists in the DSS.
+This step ensures that no subscriptions with the known test IDs exists in the DSS.
+
+This includes the main test subscription used in this test, as well as the extra subscription
+used for testing the `manager` field sync, if the test is configured to test for it.
 
 ## Subscription Synchronization test case
 
@@ -140,6 +143,8 @@ Verify that the version of the subscription returned by every DSS is as expected
 
 This test step attempts to mutate the subscription on every secondary DSS instance (that is, instances through which the subscription has not been created) to confirm that such mutations are properly propagated to every DSS.
 
+Note that this step is repeated for every secondary DSS instance.
+
 #### 🛑 Subscription can be mutated on secondary DSS check
 
 If the secondary DSS does not allow the subscription to be mutated, either the secondary DSS or the primary DSS are in violation of one or both of the following requirements:
@@ -157,6 +162,20 @@ If it does not, it might be in violation of **[astm.f3548.v21.DSS0005,5](../../.
 
 Confirm that the secondary DSS handles the update properly.
 
+#### [Validate subscription](../fragments/sub/validate/correctness.md)
+
+Verify that the subscription returned by the DSS is properly formatted and contains the correct content.
+
+#### [Validate version is updated by mutation](../fragments/sub/validate/mutated.md)
+
+Verify that the version of the subscription is updated after the mutation on the secondary.
+
+### Verify mutation on all secondaries test step
+
+This step verifies that the mutation of the subscription on a secondary instance, from the previous step, is properly propagated to every other DSS.
+
+Note that this step is repeated for every secondary DSS instance.
+
 #### [Subscription is synchronized](../fragments/sub/sync.md)
 
 Confirm that the subscription that was just mutated is properly synchronized across all DSS instances.
@@ -169,32 +188,30 @@ Confirms that the subscription that was just mutated can be retrieved from any D
 
 Verify that the subscription returned by the DSS is properly formatted and contains the correct content.
 
-#### [Validate version is updated by mutation](../fragments/sub/validate/mutated.md)
+#### [Validate version is as expected when read](../fragments/sub/validate/non_mutated.md)
 
-### Verify manager synchronization test step
+Verify that when we are reading the subscription without mutating it, the version is as expected.
 
-Checks that the manager of a subscription is properly synchronized across all DSS instances.
+### Create subscription with different credentials test step
 
-This is done by means of using a separate set of credentials to create a subscription on the primary DSS,
-and then verifying that the main credentials are not able to mutate this subscription via one of the secondary DSS instances
+If the second set of credentials is provided, this test step will create a subscription using these credentials,
+in order to prepare the next step that checks manager synchronization.
 
 #### [Create subscription](../fragments/sub/crud/create.md)
 
-Verify that a subscription can be created on the primary DSS.
-
-#### 🛑 Subscription deletion with different non-managing credentials on secondary DSS fails check
+Verify that a subscription can be created on the primary DSS using the separate set of credentials.
 
-If the subscription can be deleted by a client which did not create it, via a DSS instance to which the subscription was synced
-following its creation on the primary DSS, either one of the primary DSS or the DSS that accepted the deletion failed to properly broadcast, respectively take into account, the manage of the subscription,
-and therefore violates **[astm.f3548.v21.DSS0210,1b](../../../../../requirements/astm/f3548/v21.md)**.
+### Verify manager synchronization test step
 
-### Delete subscription test step
+If the second set of credentials is provided, checks that the manager of a subscription is properly synchronized across all DSS instances.
 
-Verify that the version of the subscription returned by the DSS the subscription was mutated through has been updated.
+This is done by verifying that the main credentials are not able to delete the subscription via any of the secondary DSS instances.
 
-#### [Validate new version is synced](../fragments/sub/validate/non_mutated.md)
+#### ⚠️ Subscription deletion with different non-managing credentials on secondary DSS fails check
 
-Verify that the new version of the subscription has been propagated.
+If the subscription can be deleted by a client which did not create it, via a DSS instance to which the subscription was synced
+following its creation on the primary DSS, either one of the primary DSS or the DSS that accepted the deletion failed to properly broadcast, respectively take into account, the manage of the subscription,
+and therefore violates **[astm.f3548.v21.DSS0210,1b](../../../../../requirements/astm/f3548/v21.md)**.
 
 ### Delete subscription on primary test step
 
@@ -256,3 +273,8 @@ either one of the primary DSS or the DSS that returned the subscription is in vi
 As a result, the DSS pool under test is failing to meet **[astm.f3548.v21.DSS0020](../../../../../requirements/astm/f3548/v21.md)**.
 
 ## [Cleanup](../clean_workspace.md)
+
+This step ensures that no subscriptions with the known test IDs exists in the DSS.
+
+This includes the main test subscription used in this test, as well as the extra subscription
+used for testing the `manager` field sync, if the test is configured to test for it.

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

@@ -46,7 +46,9 @@ class SubscriptionSynchronization(TestScenario):
     """
 
     SUB_TYPE = register_resource_type(379, "Subscription")
-    ACL_SUB_TYPE = register_resource_type(380, "Subscription with different credentials")
+    ACL_SUB_TYPE = register_resource_type(
+        381, "Subscription with different credentials"
+    )
 
     _dss: DSSInstance
 
@@ -174,28 +176,28 @@ def run(self, context: ExecutionContext):
             return
 
         self.begin_test_scenario(context)
-        self._setup_case()
+        self._step_setup_case()
         self.begin_test_case("Subscription Synchronization")
 
         self.begin_test_step("Create subscription validation")
         self._step_create_subscriptions()
         self.end_test_step()
 
         self.begin_test_step("Query newly created subscription")
-        self._query_secondaries_and_compare(self._sub_params)
+        self._step_query_secondaries_and_compare(self._sub_params)
         self.end_test_step()
 
         self.begin_test_step("Mutate subscription broadcast")
         self._step_mutate_subscriptions_broadcast_shift_time()
         self.end_test_step()
 
         self.begin_test_step("Query updated subscription")
-        self._query_secondaries_and_compare(self._sub_params)
+        self._step_query_secondaries_and_compare(self._sub_params)
         self.end_test_step()
 
         if self._dss_separate_creds:
             self.begin_test_step("Create subscription with different credentials")
-
+            self._step_create_sub_separate_creds()
             self.end_test_step()
             self.begin_test_step("Verify manager synchronization")
             self._step_test_delete_sub_with_separate_creds()
@@ -206,9 +208,7 @@ def run(self, context: ExecutionContext):
                 "Skipping manager synchronization check: no extra credentials provided",
             )
 
-        self.begin_test_step("Mutate subscription on secondaries")
         self._step_mutate_subscriptions_secondaries_shift_time()
-        self.end_test_step()
 
         self.begin_test_step("Delete subscription on primary")
         self._step_delete_sub()
@@ -225,7 +225,7 @@ def run(self, context: ExecutionContext):
         self.end_test_case()
         self.end_test_scenario()
 
-    def _setup_case(self):
+    def _step_setup_case(self):
         self.begin_test_case("Setup")
         # Multiple runs of the scenario seem to rely on the same instance of it:
         # thus we need to reset the state of the scenario before running it.
@@ -304,7 +304,9 @@ def _create_sub_with_params(
 
         return newly_created.subscription
 
-    def _query_secondaries_and_compare(self, expected_sub_params: SubscriptionParams):
+    def _step_query_secondaries_and_compare(
+        self, expected_sub_params: SubscriptionParams
+    ):
         for secondary_dss in self._dss_read_instances:
             self._validate_get_sub_from_secondary(
                 secondary_dss=secondary_dss,
@@ -663,7 +665,7 @@ def _step_test_delete_sub_with_separate_creds(self):
                     check.record_failed(
                         "Subscription deletion with main credentials did not fail",
                         details=f"Subscription deletion with main credentials did not fail with the expected "
-                                f"status code of 403; instead returned {deleted_sub.status_code}",
+                        f"status code of 403; instead returned {deleted_sub.status_code}",
                         query_timestamps=[deleted_sub.request.timestamp],
                     )
 
@@ -691,13 +693,17 @@ def _step_mutate_subscriptions_secondaries_shift_time(self):
 
         for secondary_dss in self._dss_read_instances:
             # Mutate the subscription on the secondary DSS
+            self.begin_test_step("Mutate subscription on secondaries")
             self._mutate_subscription_with_dss(
                 secondary_dss,
                 self._sub_params.shift_time(timedelta(seconds=10)),
                 is_primary=False,
             )
+            self.end_test_step()
+            self.begin_test_step("Verify mutation on all secondaries")
             # Check that the mutation was propagated to every DSS:
-            self._query_secondaries_and_compare(self._sub_params)
+            self._step_query_secondaries_and_compare(self._sub_params)
+            self.end_test_step()
 
     def _delete_sub_from_dss(
         self,