OPA is reporting back the wrong code coverage percentage and duplicating not_covered rows

[gianna7wu]

Short description

The OPA CLI is reporting incorrect code coverage percentages and duplicating not_covered rows in its JSON response. This is with the OPA CLI newly downloaded (01 March 2022) on macOS.

Examples:

Here is an example bundle (titled general.rego):

package animals

available = true 

aussie_animals = {
  "quokka": true
}

australian = true {
  aussie_animals[input.animal]
} else  = false {
  true
}

Here is an example test (titled test.rego):

package animals

test_animals{
	australian == true with input as {"animal": "quokka"}
	australian == false with input as {"animal": "panda"}
}

The results are returned like so:

{
  "files": {
    "general.rego": {
      "covered": [
        {
          "start": {
            "row": 5
          },
          "end": {
            "row": 5
          }
        },
        {
          "start": {
            "row": 9
          },
          "end": {
            "row": 12
          }
        }
      ],
      "not_covered": [
        {
          "start": {
            "row": 3
          },
          "end": {
            "row": 3
          }
        },
        {
          "start": {
            "row": 3
          },
          "end": {
            "row": 3
          }
        }
      ],
      "coverage": 71.45
    },
    "test.rego": {
      "covered": [
        {
          "start": {
            "row": 3
          },
          "end": {
            "row": 5
          }
        }
      ],
      "coverage": 100
    }
  },
  "coverage": 80
}

You can see that the coverage for general.rego is reported as 71.45% when it should be something closer to 80%. Additionally, you can see that in the not_covered field for general.rego, row 3 is specified twice in not_covered, which I think is what's affecting the coverage score.

This is the command that was run to get that output (assuming you have general.rego and test.rego saved in the same directory):
./opa test --coverage --format=json general.rego test.rego

Steps To Reproduce

1. On macOS, run curl -L -o opa https://openpolicyagent.org/downloads/v0.37.2/opa_darwin_amd64 to download the OPA CLI (following the instructions here).
2. Run chmod 755 ./opa
3. Ensure that you have general.rego and test.rego saved in the same directory.
4. ./opa test --coverage --format=json general.rego test.rego
5. The incorrect output is specified.

Expected behavior

I expect to see the code coverage be closer to 80% and row 3 not to be duplicated in the not_covered section.

Additional context

N/A

[anderseknert]

Hi @gianna7wu 👋 And thanks for reporting this. The duplicate entry in not_covered looks like a bug to me as well.

[anderseknert]

From a cursory look, it seems that the available = true assignment is hit by both ast.WalkRules and ast.WalkExprs resulting in both of them adding the same location to not_covered. I think the latter should be excluded in the includeExprInCoverage function, but I'm not sure how to differentiate that from e.g. an assignment expression inside of a rule (which I presume would not be hit by WalkRules). @srenatus or @johanfylling, any pointers? 🙂

[johanneslarsson]

I usually get those variables covered by writing a test without input.

test_no_input {
 available
}

[anderseknert]

Yeah, avoiding any line to be not_covered is one way to go about it I suppose 😆 The duplicates, and how they are both counted towards the total coverage, is a nevertheless a bug to be fixed.

[srenatus]

👀 I suppose it would make sense to try deduplicating both Covered and NotCovered. Since expressions are often expanded, and still retain there original row/col location data, there may be more cases where this happens..

Perhaps deduplication would help? With this

		switch {
		case curr.Row == end.Row: // skip
		case curr.Row == end.Row+1:
			end = curr
		default:
			result = append(result, Range{start, end})
			start, end = curr, curr
		}

instead of

opa/cover/cover.go

Lines 255 to 260 in 64a03c1

	if curr.Row == end.Row+1 {
	end = curr
	} else {
	result = append(result, Range{start, end})
	start, end = curr, curr
	}

I get that report:

{
  "files": {
    "general.rego": {
      "covered": [
        {
          "start": {
            "row": 5
          },
          "end": {
            "row": 5
          }
        },
        {
          "start": {
            "row": 9
          },
          "end": {
            "row": 12
          }
        }
      ],
      "not_covered": [
        {
          "start": {
            "row": 3
          },
          "end": {
            "row": 3
          }
        }
      ],
      "coverage": 83.35
    },
    "test.rego": {
      "covered": [
        {
          "start": {
            "row": 3
          },
          "end": {
            "row": 5
          }
        }
      ],
      "coverage": 100
    }
  },
  "coverage": 88.9
}

[anderseknert]

Neat, yeah I think that would do it 👍