[uss_qualifier] Make skipped actions first-class actions in report (#278

)

* Make skipped actions first-class actions in report

* Fix tested_requirements

* Fix lingering issue from #270

monitoring/mock_uss/interaction_logging/logger.py

@@ -3,11 +3,10 @@
 
 import flask
 import json
-from loguru import logger
 
 from monitoring.mock_uss import webapp, require_config_value
 from monitoring.mock_uss.interaction_logging.config import KEY_INTERACTIONS_LOG_DIR
-from monitoring.mock_uss.interaction_logging.interactions import (
+from monitoring.monitorlib.clients.mock_uss.interactions import (
     Interaction,
     QueryDirection,
 )

monitoring/mock_uss/interaction_logging/routes_interactions_log.py

@@ -10,7 +10,7 @@
 from monitoring.monitorlib.scd_automated_testing.scd_injection_api import (
     SCOPE_SCD_QUALIFIER_INJECT,
 )
-from monitoring.mock_uss.interaction_logging.interactions import (
+from monitoring.monitorlib.clients.mock_uss.interactions import (
     Interaction,
     ListLogsResponse,
 )

monitoring/uss_qualifier/reports/report.py

@@ -22,8 +22,6 @@
 from monitoring.uss_qualifier.scenarios.definitions import TestScenarioTypeName
 from monitoring.uss_qualifier.suites.definitions import TestSuiteActionDeclaration
 
-from monitoring.mock_uss.interaction_logging.interactions import Interaction
-
 
 class FailedCheck(ImplicitDict):
     name: str
@@ -368,9 +366,14 @@ class TestSuiteActionReport(ImplicitDict):
     action_generator: Optional[ActionGeneratorReport]
     """If this action was an action generator, this field will hold its report"""
 
+    skipped_action: Optional[SkippedActionReport]
+    """If this action was skipped, this field will hold its report"""
+
     def get_applicable_report(self) -> Tuple[bool, bool, bool]:
         """Determine which type of report is applicable for this action.
 
+        Note that skipped_action is applicable if none of the other return values are true.
+
         Returns:
             * Whether test_suite is applicable
             * Whether test_scenario is applicable
@@ -381,15 +384,21 @@ def get_applicable_report(self) -> Tuple[bool, bool, bool]:
         action_generator = (
             "action_generator" in self and self.action_generator is not None
         )
+        skipped_action = "skipped_action" in self and self.skipped_action is not None
         if (
             sum(
                 1 if case else 0
-                for case in [test_suite, test_scenario, action_generator]
+                for case in [
+                    test_suite,
+                    test_scenario,
+                    action_generator,
+                    skipped_action,
+                ]
             )
             != 1
         ):
             raise ValueError(
-                "Exactly one of `test_suite`, `test_scenario`, or `action_generator` must be populated"
+                "Exactly one of `test_suite`, `test_scenario`, `action_generator`, or `skipped_action` must be populated"
             )
         return test_suite, test_scenario, action_generator
 
@@ -398,6 +407,7 @@ def _conditional(
         test_suite_func: Union[Callable[[TestSuiteReport], Any], Callable[[Any], Any]],
         test_scenario_func: Optional[Callable[[TestScenarioReport], Any]] = None,
         action_generator_func: Optional[Callable[[ActionGeneratorReport], Any]] = None,
+        skipped_action_func: Optional[Callable[[SkippedActionReport], Any]] = None,
     ) -> Any:
         test_suite, test_scenario, action_generator = self.get_applicable_report()
         if test_suite:
@@ -412,11 +422,10 @@ def _conditional(
                 return action_generator_func(self.action_generator)
             else:
                 return test_suite_func(self.action_generator)
-
-        # This line should not be possible to reach
-        raise RuntimeError(
-            "Case selection logic failed for TestSuiteActionReport; none of test_suite, test_scenario, nor action_generator were populated"
-        )
+        if skipped_action_func is not None:
+            return skipped_action_func(self.skipped_action)
+        else:
+            return test_suite_func(self.skipped_action)
 
     def successful(self) -> bool:
         return self._conditional(lambda report: report.successful)
@@ -441,9 +450,7 @@ def query_passed_checks(
             report = self.action_generator
             prefix = "action_generator"
         else:
-            raise RuntimeError(
-                "Case selection logic failed for TestSuiteActionReport; none of test_suite, test_scenario, nor action_generator were populated"
-            )
+            return
 
         for path, pc in report.query_passed_checks(participant_id):
             yield f"{prefix}.{path}", pc
@@ -462,9 +469,7 @@ def query_failed_checks(
             report = self.action_generator
             prefix = "action_generator"
         else:
-            raise RuntimeError(
-                "Case selection logic failed for TestSuiteActionReport; none of test_suite, test_scenario, nor action_generator were populated"
-            )
+            return
 
         for path, fc in report.query_failed_checks(participant_id):
             yield f"{prefix}.{path}", fc
@@ -627,6 +632,30 @@ class SkippedActionReport(ImplicitDict):
     declaration: TestSuiteActionDeclaration
     """Full declaration of the action that was skipped."""
 
+    @property
+    def successful(self) -> bool:
+        return True
+
+    def has_critical_problem(self) -> bool:
+        return False
+
+    def all_participants(self) -> Set[ParticipantID]:
+        return set()
+
+    def queries(self) -> List[fetch.Query]:
+        return []
+
+    def participant_ids(self) -> Set[ParticipantID]:
+        return set()
+
+    @property
+    def start_time(self) -> Optional[StringBasedDateTime]:
+        return self.timestamp
+
+    @property
+    def end_time(self) -> Optional[StringBasedDateTime]:
+        return self.timestamp
+
 
 class TestSuiteReport(ImplicitDict):
     name: str
@@ -644,9 +673,6 @@ class TestSuiteReport(ImplicitDict):
     actions: List[TestSuiteActionReport]
     """Reports from test scenarios and test suites comprising the test suite for this report, in order of execution."""
 
-    skipped_actions: List[SkippedActionReport]
-    """Reports for actions configured but not executed."""
-
     end_time: Optional[StringBasedDateTime]
     """Time at which the test suite completed"""
 

monitoring/uss_qualifier/reports/sequence_view.py

@@ -497,12 +497,6 @@ def _compute_action_node(report: TestSuiteActionReport, indexer: Indexer) -> Act
         )
     elif is_test_suite:
         children = [_compute_action_node(a, indexer) for a in report.test_suite.actions]
-        for skipped_action in report.test_suite.skipped_actions:
-            i = 0
-            for i, a in enumerate(report.test_suite.actions):
-                if a.start_time.datetime > skipped_action.timestamp.datetime:
-                    break
-            children.insert(i, _skipped_action_of(skipped_action))
         return ActionNode(
             name=report.test_suite.name,
             node_type=ActionNodeType.Suite,
@@ -521,9 +515,7 @@ def _compute_action_node(report: TestSuiteActionReport, indexer: Indexer) -> Act
             ],
         )
     else:
-        raise ValueError(
-            "Invalid TestSuiteActionReport; doesn't specify scenario, suite, or action generator"
-        )
+        return _skipped_action_of(report.skipped_action)
 
 
 def _compute_overview_rows(node: ActionNode) -> Iterator[OverviewRow]:

monitoring/uss_qualifier/reports/tested_requirements.py

@@ -405,7 +405,7 @@ def _populate_breakdown_with_action_report(
                 breakdown, subaction, participant_id, req_set
             )
     else:
-        raise ValueError(f"Unsupported test suite report type")
+        pass  # Skipped action
 
 
 def _populate_breakdown_with_scenario_report(

monitoring/uss_qualifier/reports/validation/report_validation.py

@@ -55,17 +55,7 @@ def _validate_action_no_skipped_actions(
     if test_scenario:
         success = True
     elif test_suite:
-        if (
-            "skipped_actions" not in report.test_suite
-            or not report.test_suite.skipped_actions
-        ):
-            success = True
-        else:
-            success = False
-            for sa in report.test_suite.skipped_actions:
-                logger.error(
-                    f"No skipped actions not achieved because {context}.test_suite had a skipped action for action index {sa.action_declaration_index}: {sa.reason}"
-                )
+        success = True
         for i, a in enumerate(report.test_suite.actions):
             success = success and _validate_action_no_skipped_actions(
                 a, JSONAddress(context + f".test_suite.actions[{i}]")
@@ -76,6 +66,11 @@ def _validate_action_no_skipped_actions(
             success = success and _validate_action_no_skipped_actions(
                 a, JSONAddress(context + f".action_generator.actions[{i}]")
             )
+    else:
+        logger.error(
+            f"No skipped actions not achieved because {context}.test_suite had a skipped action for action index {report.skipped_action.action_declaration_index}: {report.skipped_action.reason}"
+        )
+        success = False
     return success
 
 

monitoring/uss_qualifier/resources/interuss/mock_uss/client.py

@@ -1,4 +1,4 @@
-from typing import Tuple, Optional, List
+from typing import Optional
 
 from loguru import logger
 from implicitdict import ImplicitDict
@@ -8,7 +8,7 @@
     GetLocalityResponse,
     PutLocalityRequest,
 )
-from monitoring.monitorlib.fetch import QueryError, Query
+from monitoring.monitorlib.fetch import QueryError
 from monitoring.monitorlib.infrastructure import AuthAdapter, UTMClientSession
 from monitoring.monitorlib.locality import LocalityCode
 from monitoring.monitorlib.scd_automated_testing.scd_injection_api import (
@@ -17,7 +17,7 @@
 from monitoring.uss_qualifier.reports.report import ParticipantID
 from monitoring.uss_qualifier.resources.communications import AuthAdapterResource
 from monitoring.uss_qualifier.resources.resource import Resource
-from monitoring.mock_uss.interaction_logging.interactions import (
+from monitoring.monitorlib.clients.mock_uss.interactions import (
     Interaction,
     ListLogsResponse,
 )

monitoring/uss_qualifier/suites/suite.py

@@ -157,8 +157,7 @@ class TestSuite(object):
     definition: TestSuiteDefinition
     documentation_url: str
     local_resources: Dict[ResourceID, ResourceType]
-    actions: List[TestSuiteAction]
-    skipped_actions: List[SkippedActionReport]
+    actions: List[Union[TestSuiteAction, SkippedActionReport]]
 
     def __init__(
         self,
@@ -213,8 +212,7 @@ def __init__(
                 raise ValueError(
                     f'Test suite "{self.definition.name}" expected resource {resource_id} to be {resource_type}, but instead it was provided {fullname(self.local_resources[resource_id].__class__)}'
                 )
-        actions: List[TestSuiteAction] = []
-        skipped_actions: List[SkippedActionReport] = []
+        actions: List[Union[TestSuiteAction, SkippedActionReport]] = []
         for a, action_dec in enumerate(self.definition.actions):
             try:
                 actions.append(
@@ -224,7 +222,7 @@ def __init__(
                 logger.warning(
                     f"Skipping action {a} ({action_dec.get_action_type()} {action_dec.get_child_type()}) because {str(e)}"
                 )
-                skipped_actions.append(
+                actions.append(
                     SkippedActionReport(
                         timestamp=StringBasedDateTime(arrow.utcnow().datetime),
                         reason=str(e),
@@ -233,7 +231,6 @@ def __init__(
                     )
                 )
         self.actions = actions
-        self.skipped_actions = skipped_actions
 
     def _make_report_evaluation_action(
         self, report: TestSuiteReport
@@ -267,11 +264,10 @@ def run(self) -> TestSuiteReport:
             documentation_url=self.documentation_url,
             start_time=StringBasedDateTime(datetime.utcnow()),
             actions=[],
-            skipped_actions=self.skipped_actions,
             capability_evaluations=[],
         )
 
-        def actions() -> Iterator[TestSuiteAction]:
+        def actions() -> Iterator[Union[TestSuiteAction, SkippedActionReport]]:
             for a in self.actions:
                 yield a
             # Execute report evaluation scenario as last action if specified, otherwise break loop
@@ -312,12 +308,15 @@ def actions() -> Iterator[TestSuiteAction]:
 
 
 def _run_actions(
-    actions: Iterator[TestSuiteAction],
+    actions: Iterator[Union[TestSuiteAction, SkippedActionReport]],
     report: Union[TestSuiteReport, ActionGeneratorReport],
 ) -> None:
     success = True
     for a, action in enumerate(actions):
-        action_report = action.run()
+        if isinstance(action, SkippedActionReport):
+            action_report = TestSuiteActionReport(skipped_action=action)
+        else:
+            action_report = action.run()
         report.actions.append(action_report)
         if action_report.has_critical_problem():
             success = False

schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json

@@ -18,6 +18,17 @@
         }
       ]
     },
+    "skipped_action": {
+      "description": "If this action was skipped, this field will hold its report",
+      "oneOf": [
+        {
+          "type": "null"
+        },
+        {
+          "$ref": "SkippedActionReport.json"
+        }
+      ]
+    },
     "test_scenario": {
       "description": "If this action was a test scenario, this field will hold its report",
       "oneOf": [

schemas/monitoring/uss_qualifier/reports/report/TestSuiteReport.json

@@ -37,13 +37,6 @@
       "description": "Name of this test suite",
       "type": "string"
     },
-    "skipped_actions": {
-      "description": "Reports for actions configured but not executed.",
-      "items": {
-        "$ref": "SkippedActionReport.json"
-      },
-      "type": "array"
-    },
     "start_time": {
       "description": "Time at which the test suite started",
       "format": "date-time",
@@ -63,7 +56,6 @@
     "capability_evaluations",
     "documentation_url",
     "name",
-    "skipped_actions",
     "start_time",
     "suite_type"
   ],