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

tester: Report missing lines in test coverage #2622

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions cmd/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,21 @@ func opaTest(args []string) int {
}
}
} else {
reporter = tester.JSONCoverageReporter{
Cover: cov,
Modules: modules,
Output: os.Stdout,
Threshold: testParams.threshold,
if testParams.outputFormat.String() == "json" {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Use a switch similar to the one above for the non-coverage case.

reporter = tester.JSONCoverageReporter{
Cover: cov,
Modules: modules,
Output: os.Stdout,
Threshold: testParams.threshold,
}
} else {
reporter = tester.PrettyCoverageReporter{
Verbose: testParams.verbose,
Cover: cov,
Modules: modules,
Output: os.Stdout,
Threshold: testParams.threshold,
}
}
}

Expand Down
72 changes: 72 additions & 0 deletions tester/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,78 @@ func (r JSONCoverageReporter) Report(ch chan *Result) error {
return encoder.Encode(report)
}

// PrettyCoverageReporter reports coverage as text.
type PrettyCoverageReporter struct {
Verbose bool
Cover *cover.Cover
Modules map[string]*ast.Module
Output io.Writer
Threshold float64
}

// Report prints the test report to the reporter's output.
func (r PrettyCoverageReporter) Report(ch chan *Result) error {
for tr := range ch {
if !tr.Pass() {
if tr.Error != nil {
return tr.Error
}
return errors.New(tr.String())
}
}
report := r.Cover.Report(r.Modules)
if r.Verbose {
fmt.Fprintf(r.Output, "Got coverage %v%%\n\n", report.Coverage)
for name, coverageReport := range report.Files {
fmt.Fprintf(r.Output, "%v %v%%\n", name, coverageReport.Coverage)
r.hl()
fmt.Fprintln(r.Output, "Covered:")
r.printCoverage(name, coverageReport.Covered)
if len(coverageReport.NotCovered) > 0 {
fmt.Fprintln(r.Output, "Not covered:")
r.printCoverage(name, coverageReport.NotCovered)
}
fmt.Println()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this print into r.Output?

}
} else {
if report.Coverage < r.Threshold {
fmt.Fprintln(r.Output, "Missing coverage:")
for name, coverageReport := range report.Files {
if coverageReport.Coverage != 100 {
r.printCoverage(name, coverageReport.NotCovered)
}
}
return &cover.CoverageThresholdError{
Coverage: report.Coverage,
Threshold: r.Threshold,
}
}
fmt.Fprintf(r.Output, "Expected coverage %v%%, got %v%%\n", r.Threshold, report.Coverage)
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't show "Expected coverage .." when there was no threshold, for example as-is we get:

{10:55} ✘ ~/p/g/s/g/o/opa:Syn3rman-coverage-pretty-reporter ✓ ❯ go run . test /tmp/opa-test -c
Expected coverage 0%, got 83.35%
Missing coverage:
/tmp/opa-test/authz.rego: 8
/tmp/opa-test/authz.rego: 10-11

We are also showing an empty "Missing Coverage:" section when the coverage is 100%:

{11:49} ✘ ~/p/g/s/g/o/opa:Syn3rman-coverage-pretty-reporter ✓ ❯ go run . test /tmp/opa-test -c
Expected coverage 0%, got 100%
Missing coverage:

Im also seeing a non-zero exit code when the tests pass and there is no threshold.. which seems odd. I'm assuming thats from the error("") being returned?

Most of these are things that should be caught by unit tests, please add some tests to cover this new code.

fmt.Fprintln(r.Output, "Missing coverage:")
for name, coverageReport := range report.Files {
if coverageReport.Coverage != 100{
r.printCoverage(name, coverageReport.NotCovered)
}
}
}
return errors.New("")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should return nil for a "success" case

}

// Prints the Coverage for a file as text
func (r PrettyCoverageReporter) printCoverage(name string, ar []cover.Range) {
for _, cov := range ar {
if cov.Start.Row == cov.End.Row {
fmt.Fprintf(r.Output, "%v: %v\n", name, cov.Start.Row)
} else {
fmt.Fprintf(r.Output, "%v: %v-%v\n", name, cov.Start.Row, cov.End.Row)
}
}
}

func (r PrettyCoverageReporter) hl() {
fmt.Fprintln(r.Output, strings.Repeat("-", 80))
}

type indentingWriter struct {
w io.Writer
}
Expand Down