From 34301310b9617df62506caac8b900fbebf9d732e Mon Sep 17 00:00:00 2001 From: Benjamin Pelletier Date: Sat, 21 Oct 2023 02:41:14 +0000 Subject: [PATCH 1/3] Make skipped actions first-class actions in report --- monitoring/uss_qualifier/reports/report.py | 60 ++++++++++++++----- .../uss_qualifier/reports/sequence_view.py | 10 +--- .../reports/validation/report_validation.py | 17 ++---- monitoring/uss_qualifier/suites/suite.py | 19 +++--- .../reports/report/TestSuiteActionReport.json | 41 ++++++++----- .../reports/report/TestSuiteReport.json | 8 --- 6 files changed, 86 insertions(+), 69 deletions(-) diff --git a/monitoring/uss_qualifier/reports/report.py b/monitoring/uss_qualifier/reports/report.py index d22cc1d005..1a1562f7b2 100644 --- a/monitoring/uss_qualifier/reports/report.py +++ b/monitoring/uss_qualifier/reports/report.py @@ -362,9 +362,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 @@ -375,15 +380,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 @@ -392,6 +403,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: @@ -406,11 +418,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) @@ -435,9 +446,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 @@ -456,9 +465,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 @@ -621,6 +628,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 @@ -638,9 +669,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 cba2d75efa..04b51333d5 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/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/suites/suite.py b/monitoring/uss_qualifier/suites/suite.py index 79cedd14d9..9dda92f111 100644 --- a/monitoring/uss_qualifier/suites/suite.py +++ b/monitoring/uss_qualifier/suites/suite.py @@ -156,8 +156,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, @@ -204,8 +203,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( @@ -215,7 +213,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), @@ -224,7 +222,6 @@ def __init__( ) ) self.actions = actions - self.skipped_actions = skipped_actions def _make_report_evaluation_action( self, report: TestSuiteReport @@ -258,11 +255,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 @@ -303,12 +299,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 612cf3e094..cb541861ef 100644 --- a/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json +++ b/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json @@ -1,34 +1,47 @@ { + "$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json", "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", + "description": "monitoring.uss_qualifier.reports.report.TestSuiteActionReport, as defined in monitoring/uss_qualifier/reports/report.py", "properties": { "$ref": { - "type": "string", - "description": "Path to content that replaces the $ref" + "description": "Path to content that replaces the $ref", + "type": "string" }, - "test_scenario": { + "action_generator": { + "description": "If this action was an action generator, this field will hold its report", "oneOf": [ { "type": "null" }, { - "$ref": "TestScenarioReport.json" + "$ref": "ActionGeneratorReport.json" } - ], - "description": "If this action was a test scenario, this field will hold its report" + ] }, - "action_generator": { + "skipped_action": { + "description": "If this action was skipped, this field will hold its report", "oneOf": [ { "type": "null" }, { - "$ref": "ActionGeneratorReport.json" + "$ref": "SkippedActionReport.json" } - ], - "description": "If this action was an action generator, this field will hold its report" + ] + }, + "test_scenario": { + "description": "If this action was a test scenario, this field will hold its report", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "TestScenarioReport.json" + } + ] }, "test_suite": { + "description": "If this action was a test suite, this field will hold its report", "oneOf": [ { "type": "null" @@ -36,10 +49,8 @@ { "$ref": "TestSuiteReport.json" } - ], - "description": "If this action was a test suite, this field will hold its report" + ] } }, - "$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/reports/report/TestSuiteActionReport.json", - "description": "monitoring.uss_qualifier.reports.report.TestSuiteActionReport, as defined in monitoring/uss_qualifier/reports/report.py" + "type": "object" } \ No newline at end of file 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" ], From ed99a7ccced187ec87cd07a5f86304c32e27fc5d Mon Sep 17 00:00:00 2001 From: Benjamin Pelletier Date: Sat, 21 Oct 2023 02:56:08 +0000 Subject: [PATCH 2/3] Fix tested_requirements --- monitoring/uss_qualifier/reports/tested_requirements.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitoring/uss_qualifier/reports/tested_requirements.py b/monitoring/uss_qualifier/reports/tested_requirements.py index 2ec61863df..b3c20e5ef0 100644 --- a/monitoring/uss_qualifier/reports/tested_requirements.py +++ b/monitoring/uss_qualifier/reports/tested_requirements.py @@ -406,7 +406,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( From 185e905c557f00f78ef1d4b6f16ec6c2d4f75be6 Mon Sep 17 00:00:00 2001 From: Benjamin Pelletier Date: Tue, 24 Oct 2023 18:59:56 +0000 Subject: [PATCH 3/3] Fix lingering issue from #270 --- monitoring/mock_uss/interaction_logging/logger.py | 3 +-- .../mock_uss/interaction_logging/routes_interactions_log.py | 2 +- .../clients/mock_uss}/interactions.py | 0 monitoring/uss_qualifier/reports/report.py | 2 -- .../uss_qualifier/resources/interuss/mock_uss/client.py | 6 +++--- 5 files changed, 5 insertions(+), 8 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 eb8da639d3..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 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, )