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

make coverage threshold show lines missing coverage #2562

Closed
grosser opened this issue Jul 20, 2020 · 4 comments · Fixed by #6272
Closed

make coverage threshold show lines missing coverage #2562

grosser opened this issue Jul 20, 2020 · 4 comments · Fixed by #6272

Comments

@grosser
Copy link

grosser commented Jul 20, 2020

Running with --threshold can fail and then shows "expected 100% got 98%", which then means I have to get out the coverage data and figure out where the problem is.
Ideally it would show missing coverage:\nfoo.rego:12
this could be overwhelming when lots of things are uncovered, so it should be opt in or only show the first N lines with missing coverage.

/cc contributors from threshold #1029 @EliuX @tsandall @flavio @srenatus

@tsandall
Copy link
Member

This would be a nice enhancement and is a good area for contribution. At a high-level, I'd expect we would want to:

  1. Update opa test so that the "pretty" format reports text instead of JSON
  2. Update the "pretty" output mode to include the missing lines (sorted by file/line #)

I'm not too worried about the # of missing lines since I would expect most policies to be completely covered (and in my experience this is often the case.)

@Syn3rman
Copy link
Contributor

Syn3rman commented Aug 7, 2020

Hey @tsandall,

Can I take this up if it is available?
From my understanding, implementing a PrettyCoverageReporter similar to the JSONCoverageReporter should suffice, right?

Also, does the following output format work?

$ opa test -c --threshold 80 ex_tests/ -v

ex_tests/example.rego 57.15%
--------------------------------------------------------------------------------
Not covered: Line 3 to 5

Code coverage threshold not met: got 76.90 instead of 80.00
$ opa test -c ex_tests/ -v
Coverage: 76.9%

ex_tests/example_test.rego 100%
--------------------------------------------------------------------------------
Covered: Line 3 to 4
Covered: Line 7 to 8
Covered: Line 11 to 12

ex_tests/example.rego 57.15%
--------------------------------------------------------------------------------
Covered: Line 8 to 8
Covered: Line 10 to 12
Not covered: Line 3 to 5

@grosser
Copy link
Author

grosser commented Aug 7, 2020

  • showing covered lines is redundant
  • showing full file + number makes it easy to copy and navigate to in editor
missing coverage:
foo.rego:123-125
foo.rego:127

Syn3rman added a commit to Syn3rman/opa that referenced this issue Aug 12, 2020
With the --threshold flag set, opa did not report the missing lines,
making it difficult to understand where the problem is. This commit
modifies opa test so that the pretty format reports text instead of
JSON, and shows the lines not covered if the threshold is not met.

Fixes open-policy-agent#2562
Signed-off-by: Aditya <aditya10699@gmail.com>
@johanfylling johanfylling self-assigned this Oct 3, 2023
johanfylling added a commit to johanfylling/opa that referenced this issue Oct 4, 2023
when `--verbose` flag is enabled.

Fixes: open-policy-agent#2562
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
johanfylling added a commit that referenced this issue Oct 4, 2023
)

Printing lines not covered when test coverage threshold isn't met for `opa test --threshold`, and `--verbose` flag is enabled.

Fixes: #2562
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@grosser
Copy link
Author

grosser commented Oct 4, 2023

thx, that looks perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment