USS Qualifier test framework (F3548_self_contained) bug

[hrishiballal]

Note: remove this note and replace all template instructions below with your content before submitting this issue
In the uss_qualifier, for the f3548_self_contained test is run, and the USS reports a "Rejected" response to a operational intent for processing, the qualifier reports a failed check although a Rejected response is allowed (see console screenshot).

Steps to reproduce the behavior:

1. Go to the monitoring repository -> monitoring/uss_qualifier/run_locally.sh configurations.dev.f3548_self_contained
2. Run the tests as above
3. See error in the sequence.html file

sequence.zip

Test check
Identify the test check that unexpectedly succeeded, failed, or wasn't checked -- try to limit a single GitHub issue to a single test check to investigate. "First failed_check" is fine, or a line number in the attached report.json is ideal.

Difference from expected behavior
A rejected response is acceptable and should not appear as a warning / error. The text of the qualifier should be updated and a warning rather than a failure must be reported.
Additional context
Add any other context about the problem here, or remove this section.

[BenjaminPelletier]

@mickmis I'm not sure if we should be producing a Low-severity finding here, but in any case it seems like at least the details message isn't right ("uss1 indicated Rejected rather than the expected NotSupported or Rejected or ConflictWithFlight or ReadyToFly"). Could you take a look?

[mickmis]

Hi @hrishiballal, I will have a look at the issue, but not sure I will be able to figure it out without the "report.json" file, could you include it? Thanks

[mickmis]

About the low severity fail: this is a bit counter-intuitive, but this "failure" was actually meant on purpose to be used as a warning when implementing this scenario. Indeed this is one of those cases were the "Rejected" response is an allowed one, but we cannot validate that the reason for rejecting the planning is valid, so we do not want to validate explicitly the requirement. On the other hand we do not to fail it either, because the rejected may totally be valid. So had opted in the past for this "low-severity fail as a warning".
Whether or not we should change this in the scenarios is a good question and it may be a good idea to introduce a better / more explicit mechanism for such cases. Do you have an opinion on that @BenjaminPelletier ?

About the text details of the low-severity failure:
Indeed the text displayed on the failure is a generic one that is just creating confusing here.
We should be displaying somehow the text written there: https://github.com/interuss/monitoring/blob/ef76e72f860f18ac5d823ff9ea9b8f1fe702bc02/monitoring/uss_qualifier/scenarios/astm/utm/nominal_planning/conflict_higher_priority/conflict_higher_priority.md#modify-activated-flight-1-in-conflict-with-activated-flight-2-test-step
Not sure yet how to change that properly.

In short: what you encountered @hrishiballal is not a bug in itself, but the two problems I highlighted above create confusion.
@BenjaminPelletier I propose to open two separate issues for those and close this one. WDYT?

[BenjaminPelletier]

I've added #409 to hopefully more broadly address the Low-severity issue; @hrishiballal and @mickmis, thoughts welcome. Given that, perhaps we can use this issue to track just the text details and check behavior?

[mickmis]

@BenjaminPelletier with #411 merged and feedback from @hrishiballal that the issue cannot be reproduced anymore, can we close this issue?

[BenjaminPelletier]

I don't think we've done anything that would have incidentally fixed the underlying problem of an incorrect error message. It seems like this message must have come from submit_flight when the calling function included Rejected in the keys of both failed_checks and expected_results. I think we should either check for this condition at entry to submit_flight and throw a ValueError or similar since the calling function is using this function inappropriately, or else construct an error message that is more accurate if that is an expected/allowed use case.

[mickmis]

I don't think we've done anything that would have incidentally fixed the underlying problem of an incorrect error message. It seems like this message must have come from submit_flight when the calling function included Rejected in the keys of both failed_checks and expected_results.

I think we should either check for this condition at entry to submit_flight and throw a ValueError or similar since the calling function is using this function inappropriately, [...]

Actually that is what #411 implements for submit_flight_intent, the function used by the scenario failing in this issue (and adapt its callers accordingly).
And in just-opened #491 I extend this to submit_flight. So once this is merged, I think that is OK.

[BenjaminPelletier]

I believe this issue is fixed, though #409 is still open. Please reopen this issue if something related to this issue but outside the scope of #409 remains unaddressed.