[uss_qualifier] Fix execution error validation and hidden bugs (#308)

* Add execution errors to report validation

* Fix previously-hidden bugs

monitoring/monitorlib/rid_v2.py

@@ -48,16 +48,8 @@ def make_volume_4d(
                 "altitude_lower": make_altitude(alt_lo),
                 "altitude_upper": make_altitude(alt_hi),
             },
-            **(
-                {"time_start": StringBasedDateTime(start_time)}
-                if start_time is not None
-                else {}
-            ),
-            **(
-                {"time_end": StringBasedDateTime(end_time)}
-                if end_time is not None
-                else {}
-            ),
+            **({"time_start": make_time(start_time)} if start_time is not None else {}),
+            **({"time_end": make_time(end_time)} if end_time is not None else {}),
         },
         Volume4D,
     )

monitoring/uss_qualifier/configurations/dev/f3548_self_contained.yaml

@@ -118,9 +118,15 @@ v1:
   # test run report.  All of the criteria must be met to return a successful code.
   validation:
     criteria:
-        # applicability indicates which test report elements the pass_condition applies to
+      # applicability indicates which test report elements the pass_condition applies to
       - applicability:
-          # We want to make sure there are no failed checks...
+          # We want to make sure no test scenarios had execution errors
+          test_scenarios: {}
+        pass_condition:
+          each_element:
+            has_execution_error: false
+      - applicability:
+          # We also want to make sure there are no failed checks...
           failed_checks:
             # ...at least, no failed checks with severity higher than "Low".
             has_severity:

monitoring/uss_qualifier/configurations/dev/library/validation.yaml

@@ -1,6 +1,11 @@
 normal_test:
   $content_schema: monitoring/uss_qualifier/reports/validation/report_validation/ValidationConfiguration.json
   criteria:
+    - applicability:
+        test_scenarios: {}
+      pass_condition:
+        each_element:
+          has_execution_error: false
     - applicability:
         failed_checks:
           has_severity:

monitoring/uss_qualifier/reports/validation/definitions.py

@@ -32,6 +32,12 @@ class NumericComparison(ImplicitDict):
 # ===== Applicability =====
 
 
+class TestScenarioApplicability(ImplicitDict):
+    """TestScenarioReport test report elements are applicable according to this specification."""
+
+    pass
+
+
 class FailedCheckApplicability(ImplicitDict):
     """FailedCheck test report elements are applicable according to this specification."""
 
@@ -64,6 +70,9 @@ class ValidationCriterionApplicability(ImplicitDict):
 
     Exactly one field must be specified."""
 
+    test_scenarios: Optional[TestScenarioApplicability]
+    """Only this kind of TestScenarioReport elements are applicable."""
+
     failed_checks: Optional[FailedCheckApplicability]
     """Only this kind of FailedCheck elements are applicable."""
 
@@ -92,6 +101,9 @@ class EachElementCondition(ImplicitDict):
     has_severity: Optional[SeverityComparison]
     """The element must be a FailedCheck that has this specified kind of severity."""
 
+    has_execution_error: Optional[bool]
+    """The element must be a TestScenarioReport that either must have or must not have an execution error."""
+
 
 class ElementGroupCondition(ImplicitDict):
     """A group of applicable elements must meet this condition.  Exactly one field must be specified."""

monitoring/uss_qualifier/reports/validation/report_validation.py

@@ -34,7 +34,7 @@
 
 @dataclass
 class TestReportElement(object):
-    element: Union[FailedCheck, SkippedActionReport]
+    element: Union[FailedCheck, SkippedActionReport, TestScenarioReport]
     location: JSONAddress
 
 
@@ -78,7 +78,12 @@ def _compare_severity(severity: Severity, comparison: SeverityComparison) -> boo
 def _is_applicable(
     element: TestReportElement, applicability: ValidationCriterionApplicability
 ) -> bool:
-    if "failed_checks" in applicability and applicability.failed_checks is not None:
+    if "test_scenarios" in applicability and applicability.test_scenarios is not None:
+        if not isinstance(element.element, TestScenarioReport):
+            return False
+        return True
+
+    elif "failed_checks" in applicability and applicability.failed_checks is not None:
         if not isinstance(element.element, FailedCheck):
             return False
         if (
@@ -124,6 +129,9 @@ def _get_applicable_elements_from_test_scenario(
     report: TestScenarioReport,
     location: JSONAddress,
 ) -> Iterator[TestReportElement]:
+    element = TestReportElement(element=report, location=location)
+    if _is_applicable(element, applicability):
+        yield element
     for i, (fc_location, fc) in enumerate(report.query_failed_checks()):
         element = TestReportElement(
             element=fc, location=JSONAddress(location + "." + fc_location)
@@ -211,6 +219,18 @@ def _evaluate_element_condition(
             )
             return False
 
+    elif (
+        "has_execution_error" in condition and condition.has_execution_error is not None
+    ):
+        if isinstance(element.element, TestScenarioReport):
+            has_error = (
+                "execution_error" in element.element
+                and element.element.execution_error is not None
+            )
+            return condition.has_execution_error == has_error
+        else:
+            return not condition.has_execution_error
+
     else:
         raise ValueError(
             "Invalid EachElementCondition; must specify exactly one of the options"

monitoring/uss_qualifier/scenarios/astm/netrid/common/dss/isa_subscription_interactions.py

@@ -10,6 +10,7 @@
 from monitoring.uss_qualifier.scenarios.astm.netrid.common.dss import utils
 from monitoring.uss_qualifier.scenarios.astm.netrid.dss_wrapper import DSSWrapper
 from monitoring.uss_qualifier.scenarios.scenario import GenericTestScenario
+from monitoring.uss_qualifier.suites.suite import ExecutionContext
 
 
 class ISASubscriptionInteractions(GenericTestScenario):
@@ -42,7 +43,7 @@ def __init__(
         self._isa_end_time = self._isa.shifted_time_end(now)
         self._isa_area = [vertex.as_s2sphere() for vertex in self._isa.footprint]
 
-    def run(self):
+    def run(self, context: ExecutionContext):
         self.begin_test_scenario()
 
         self._setup_case()

monitoring/uss_qualifier/scenarios/astm/netrid/common/dss/subscription_simple.py

@@ -17,6 +17,7 @@
 from monitoring.uss_qualifier.scenarios.scenario import (
     GenericTestScenario,
 )
+from monitoring.uss_qualifier.suites.suite import ExecutionContext
 
 TIME_TOLERANCE_SEC = 1
 
@@ -91,7 +92,7 @@ def __init__(
             for vertex in problematically_big_area.specification.vertices
         ]
 
-    def run(self):
+    def run(self, context: ExecutionContext):
         self.begin_test_scenario()
 
         loguru.logger.info("setup")

monitoring/uss_qualifier/scenarios/astm/netrid/store_flight_data.py

@@ -9,6 +9,7 @@
     FlightDataStorageResource,
 )
 from monitoring.uss_qualifier.scenarios.scenario import TestScenario
+from monitoring.uss_qualifier.suites.suite import ExecutionContext
 
 
 class StoreFlightData(TestScenario):
@@ -24,7 +25,7 @@ def __init__(
         self._flights_data = flights_data
         self._storage_config = storage_configuration
 
-    def run(self):
+    def run(self, context: ExecutionContext):
         self.begin_test_scenario()
         self.record_note(
             "Flight count",
@@ -49,6 +50,10 @@ def run(self):
             in self._storage_config.storage_configuration
             and self._storage_config.storage_configuration.flight_record_collection_path
         ):
+            dirname = os.path.dirname(
+                self._storage_config.storage_configuration.flight_record_collection_path
+            )
+            os.makedirs(dirname, exist_ok=True)
             with open(
                 self._storage_config.storage_configuration.flight_record_collection_path,
                 "w",

schemas/monitoring/uss_qualifier/reports/validation/definitions/EachElementCondition.json

@@ -7,6 +7,13 @@
       "description": "Path to content that replaces the $ref",
       "type": "string"
     },
+    "has_execution_error": {
+      "description": "The element must be a TestScenarioReport that either must have or must not have an execution error.",
+      "type": [
+        "boolean",
+        "null"
+      ]
+    },
     "has_severity": {
       "description": "The element must be a FailedCheck that has this specified kind of severity.",
       "oneOf": [

schemas/monitoring/uss_qualifier/reports/validation/definitions/TestScenarioApplicability.json

@@ -0,0 +1,12 @@
+{
+  "$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/reports/validation/definitions/TestScenarioApplicability.json",
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "description": "TestScenarioReport test report elements are applicable according to this specification.\n\nmonitoring.uss_qualifier.reports.validation.definitions.TestScenarioApplicability, as defined in monitoring/uss_qualifier/reports/validation/definitions.py",
+  "properties": {
+    "$ref": {
+      "description": "Path to content that replaces the $ref",
+      "type": "string"
+    }
+  },
+  "type": "object"
+}

schemas/monitoring/uss_qualifier/reports/validation/definitions/ValidationCriterionApplicability.json

@@ -68,6 +68,17 @@
           "$ref": "SkippedCheckApplicability.json"
         }
       ]
+    },
+    "test_scenarios": {
+      "description": "Only this kind of TestScenarioReport elements are applicable.",
+      "oneOf": [
+        {
+          "type": "null"
+        },
+        {
+          "$ref": "TestScenarioApplicability.json"
+        }
+      ]
     }
   },
   "type": "object"