opa
opa copied to clipboard
regression: coverage change from 0.63.0+
Short description
Test coverage calculation changed in 0.63.0 when multiple conditions are matched. Previously both condition trees were being marked as covered by a single test, now the first to match seems to be the only one marked as covered.
Steps To Reproduce
Run the follow test case with coverage (opa test --coverage)
package coveragetest
violation[{"msg": msg}] {
needs_big_number(input)
msg := "violated policy"
}
calculation(i) = calc {
a := object.get(i, "a", 0)
b := object.get(i, "b", 0)
calc := a * b
}
needs_big_number(i) {
calculation(i) > 5
}
needs_big_number(a) {
object.get(a, "b", 0) > 2
}
test_violation {
r = { "a": 2, "b": 3 }
count(violation) == 1 with input as r
}
Expected behavior
In OPA 0.62.0 and earlier, coverage is listed as 100%.
In OPA 0.63.0 and later, lines relating to the second needs_big_number condition are treated as not covered, resulting in a reduction in coverage.
Additional context
This could be argued the previous behaviour was a bug and that coverage does indeed need to be increased, but it is a change that was not mentioned in the CHANGELOG if so.
Seems to be caused by the early-exit changes in 143a8e6ac9e679789842ed3442d8c5800c8d5c8b
This seems like the correct behavior. Maybe could add an example in the test coverage docs. @johanfylling wdyt?
I believe this is the correct behavior, yes. Naturally, EE can affect coverage for some policies - which is even more likely now that it's been fixed/improved - but this might not be immediately obvious to the user. Agreed, we should update the docs 👍 .
@adammw if you'd like to update the docs feel free to submit a PR. Thanks!
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.