From 58078998747c750e1f2265a2b25ca9806d97af95 Mon Sep 17 00:00:00 2001 From: Benjamin Pelletier Date: Tue, 24 Oct 2023 12:54:23 -0700 Subject: [PATCH] [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 --- .../mock_uss/interaction_logging/logger.py | 3 +- .../routes_interactions_log.py | 2 +- .../clients/mock_uss}/interactions.py | 0 monitoring/uss_qualifier/reports/report.py | 62 +++++++++++++------ .../uss_qualifier/reports/sequence_view.py | 10 +-- .../reports/tested_requirements.py | 2 +- .../reports/validation/report_validation.py | 17 ++--- .../resources/interuss/mock_uss/client.py | 6 +- monitoring/uss_qualifier/suites/suite.py | 19 +++--- .../reports/report/TestSuiteActionReport.json | 11 ++++ .../reports/report/TestSuiteReport.json | 8 --- 11 files changed, 77 insertions(+), 63 deletions(-) rename monitoring/{mock_uss/interaction_logging => monitorlib/clients/mock_uss}/interactions.py (100%) diff --git a/monitoring/mock_uss/interaction_logging/logger.py b/monitoring/mock_uss/interaction_logging/logger.py index 78f090541f..a4112ae311 100644 --- a/monitoring/mock_uss/interaction_logging/logger.py +++ b/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, ) diff --git a/monitoring/mock_uss/interaction_logging/routes_interactions_log.py b/monitoring/mock_uss/interaction_logging/routes_interactions_log.py index 739858fa50..e4e1a2d1d3 100644 --- a/monitoring/mock_uss/interaction_logging/routes_interactions_log.py +++ b/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, ) diff --git a/monitoring/mock_uss/interaction_logging/interactions.py b/monitoring/monitorlib/clients/mock_uss/interactions.py similarity index 100% rename from monitoring/mock_uss/interaction_logging/interactions.py rename to monitoring/monitorlib/clients/mock_uss/interactions.py diff --git a/monitoring/uss_qualifier/reports/report.py b/monitoring/uss_qualifier/reports/report.py index b9e6ad8ed6..3f3eebbe41 100644 --- a/monitoring/uss_qualifier/reports/report.py +++ b/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""" diff --git a/monitoring/uss_qualifier/reports/sequence_view.py b/monitoring/uss_qualifier/reports/sequence_view.py index c87b3f52a4..eb2f4e7a77 100644 --- a/monitoring/uss_qualifier/reports/sequence_view.py +++ b/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]: diff --git a/monitoring/uss_qualifier/reports/tested_requirements.py b/monitoring/uss_qualifier/reports/tested_requirements.py index 4bc4cfb2df..2d8c043307 100644 --- a/monitoring/uss_qualifier/reports/tested_requirements.py +++ b/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( diff --git a/monitoring/uss_qualifier/reports/validation/report_validation.py b/monitoring/uss_qualifier/reports/validation/report_validation.py index 9f963dcd41..6939617071 100644 --- a/monitoring/uss_qualifier/reports/validation/report_validation.py +++ b/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 diff --git a/monitoring/uss_qualifier/resources/interuss/mock_uss/client.py b/monitoring/uss_qualifier/resources/interuss/mock_uss/client.py index 53c7a104e7..02c9be1a3b 100644 --- a/monitoring/uss_qualifier/resources/interuss/mock_uss/client.py +++ b/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, ) diff --git a/monitoring/uss_qualifier/suites/suite.py b/monitoring/uss_qualifier/suites/suite.py index 3769a4b2d1..b356f7e03b 100644 --- a/monitoring/uss_qualifier/suites/suite.py +++ b/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 diff --git a/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json b/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json index 41becebf3c..cb541861ef 100644 --- a/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json +++ b/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": [ diff --git a/schemas/monitoring/uss_qualifier/reports/report/TestSuiteReport.json b/schemas/monitoring/uss_qualifier/reports/report/TestSuiteReport.json index b068d9b704..9e27dcd742 100644 --- a/schemas/monitoring/uss_qualifier/reports/report/TestSuiteReport.json +++ b/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" ],