Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[uss_qualifier] Improve test run report validation #298

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

BenjaminPelletier
Copy link
Member

Currently, all checks in all dev tests (used for CI) pass. This limits test coverage substantially as no logic involving failed checks is ever exercised. This PR reworks test run report validation to allow for more sophisticated test run validation criteria, including allowing (and requiring) the test runs of some configurations to contain failed checks (and, in the future, additional and more specific characteristics).

@BenjaminPelletier BenjaminPelletier marked this pull request as ready for review October 30, 2023 05:39
Copy link
Contributor

@barroco barroco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note the comment inline

else:
raise ValueError(f"Cannot compare Severity to {type(other)}")

if self == Severity.Critical:
Copy link
Contributor

@barroco barroco Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have misunderstood but if we assume that Critical is bigger than Low severity, then it looks like the operator implementation is not completely correct.

self other expected gt implemented gt expected == implemented ?
Critical Critical False False
Critical High True False ?
Critical Medium True False ?
Critical Low True False ?
High Critical False True ?
High High False False
High Medium True False ?
High Low True False ?
Medium Critical False False
Medium High False False
Medium Medium False False
Medium Low True True
Low * False True ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 thanks. Ok, fix verified with:

from monitoring.uss_qualifier.common_data_definitions import Severity

severities = (Severity.Critical, Severity.High, Severity.Medium, Severity.Low)
operators = ("==", "!=", ">", ">=", "<", "<=")

for op in operators:
    corner = f"↓ {op} →"
    print(f"{corner:<10}" + "".join(f"{s.value:<10}" for s in severities))
    for r in severities:
        line = f"{r.value:<10}"
        for c in severities:
            v = eval(f"r {op} c")
            line += f"{v:<10}"
        print(line)
    print()

@BenjaminPelletier BenjaminPelletier merged commit 89aaa4b into interuss:main Oct 31, 2023
8 checks passed
@BenjaminPelletier BenjaminPelletier deleted the improve-validation branch October 31, 2023 18:22
github-actions bot added a commit that referenced this pull request Oct 31, 2023
* Improve test run report validation

* Fix Severity comparison logic 89aaa4b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants