[uss_qualifier] Remove test step management from OpIntentValidator #428
Conversation
| @@ -337,9 +335,9 @@ def _operational_intent_shared_check( | |||
| self, | |||
| flight_intent: Union[InjectFlightRequest | FlightInfo], | |||
| skip_if_not_found: bool, | |||
| ) -> OperationalIntentReference: | |||
| ) -> Optional[OperationalIntentReference]: | |||
There was a problem hiding this comment.
Note FYI: this partially addresses a bug recently introduced. I was planning to make a PR to fix it, but the idea is that if this function returns None, the flow of the calling function should be stopped (previously: the test step ends). The issue does not pop up on the CI unfortunately, but this introduced a regression for some USSes implementation that we addressed in the last months.
| @@ -131,12 +127,18 @@ def _begin_step(self): | |||
| query_timestamps=[self._after_query.request.timestamp], | |||
| ) | |||
|
|
|||
| def expect_removed(self, flight_id: EntityID) -> None: | |||
There was a problem hiding this comment.
Looks like expect_removed is missing its implementation.
There was a problem hiding this comment.
Eek, thanks -- will fix.
|
Note: I'm intending to resync this PR after #444 is merged |
|
@mickmis ready for a second round, I think -- please do be skeptical when reviewing as I've had to merge and patch a couple different times and very well may have missed something or done something wrong, especially since this PR isn't simple to begin with. |
| @@ -78,7 +78,6 @@ def plan_flight_intent( | |||
|
|
|||
There was a problem hiding this comment.
In this there is modify_planned_flight_intent as well that has a test_step parameter that should be removed. The function is not used anywhere though so that should not imply additional changes.
There was a problem hiding this comment.
Sounds good; done.
) * Remove test step management from OpIntentValidator * Finish implementation * Fix merge issues * Fix hygiene * Fix hygiene fix * Remove test_step from unused function per comments 4ac63f9
As part of #380, this PR moves test step management (beginning test steps and ending test steps) from inside OpIntentValidator to outside OpIntentValidator (almost always directly within the scenario). This usually results in combining a flight planning action/fragment with a sharing validation action/fragment within the same test step, and this should make test steps monotonic again (if test step B follows test step A, then the latest event in A should be earlier than the earliest step in B).