opa icon indicating copy to clipboard operation
opa copied to clipboard

regression: coverage change from 0.63.0+

Open adammw opened this issue 1 year ago • 4 comments

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

adammw avatar Jun 04 '24 02:06 adammw

This seems like the correct behavior. Maybe could add an example in the test coverage docs. @johanfylling wdyt?

ashutosh-narkar avatar Jun 04 '24 20:06 ashutosh-narkar

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 👍 .

johanfylling avatar Jun 05 '24 08:06 johanfylling

@adammw if you'd like to update the docs feel free to submit a PR. Thanks!

ashutosh-narkar avatar Jun 05 '24 15:06 ashutosh-narkar

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.

stale[bot] avatar Jul 06 '24 07:07 stale[bot]