interuss / monitoring

InterUSS Platform USS monitoring tools for federated UTM, including automated testing.
Apache License 2.0
13 stars 19 forks source link

Data validation scenario behaves incorrectly under mock_uss failure #772

Open BenjaminPelletier opened 2 months ago

BenjaminPelletier commented 2 months ago

Observed behavior USS under test fails Awareness of relevant operational intents scenario because mock_uss has a pre-existing operational intent with intentionally-incorrect data (priority = 1.2) from the previous scenario, Data Validation of GET operational intents by USS, and the USS under test (correctly) cannot parse this invalid operational intent.

Test check N/A

Difference from expected behavior

  1. Data Validation of GET operational intents by USS should clean up any flights/operational intents that could be in existence during the cleanup phase, but instead doesn't clean up if the flight intent creation by mock_uss returns 500 even though the flight is successfully created. a. We should trigger the flight clean up upon scenario failure even if the flight creation call returns 500 (because the flight might have been created before the error was encountered)
  2. The 500 error is being caused by attempting to access an ImplicitDict field on an object that is a mere dict and not an ImplicitDict type (the .reference field access) -- we should not attempt to treat a plain dict like an ImplicitDict type a. The underlying issue is that the addition of the parse_result argument broke the type-hint contract of the function. The apply_overrides type hinting indicates that it returns an ImplicitDictType (specifically, the same type as the provided base_object), but when parse_result is false, it does not do this -- instead, it returns a plain dict. This is a problem when op_intent_from_flightrecord should be returning an f3548_v21.OperationalIntent and instead returns a dict.

Additional context See attached report for an example of this problem:

sequence.zip

Shastick commented 2 months ago

@BenjaminPelletier, regarding point 2. above: I'm assuming that passing parse_result=True in op_intent_from_flightrecord is not going to be a sufficient fix: If I do set the flag to true, the flight planning request fails (at least on f3548_self_contained) – This makes sense if we're trying to build incorrect objects, just mentiong in case we would actually expect the parsing to succeed.

I'll look into a way of obtaining a proper f3548_v21.OperationalIntent object that does not involve parsing the object.

BenjaminPelletier commented 2 months ago

Yep, it's a larger conceptual issue -- can't parse an intentionally-invalid object.

Shastick commented 2 months ago

775 proposes to directly mutate the base_object. This seems to work.

If mutation is not desirable I assume we could use copy.deepcopy(), although I have not tested this yet.

Shastick commented 2 months ago

@BenjaminPelletier this should be solved as the problems identifies by this issue have been addressed by #775 and #774, but could you by any change confirm this on the environment where the problem was originally discovered? I'll leave the issue open until then.