From 90b91e6bf94b96c0a501445c22d2f2c00dfd63fb Mon Sep 17 00:00:00 2001 From: Liam Galvin Date: Tue, 22 Mar 2022 21:16:42 +0000 Subject: [PATCH] feat: Add file path to failing tests to make debugging failing tests easier (#4461) Signed-off-by: Liam Galvin --- tester/reporter.go | 20 +++++++++---------- tester/reporter_test.go | 43 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/tester/reporter.go b/tester/reporter.go index 129f88504e..58a94708a6 100644 --- a/tester/reporter.go +++ b/tester/reporter.go @@ -11,10 +11,9 @@ import ( "io" "strings" - "github.com/open-policy-agent/opa/topdown" - "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/cover" + "github.com/open-policy-agent/opa/topdown" ) // Reporter defines the interface for reporting test results. @@ -71,19 +70,20 @@ func (r PrettyReporter) Report(ch chan *Result) error { } // Report individual tests. + var lastFile string for _, tr := range results { + if tr.Pass() && r.BenchmarkResults { dirty = true fmt.Fprintln(r.Output, r.fmtBenchmark(tr)) - } else if r.Verbose { - dirty = true - fmt.Fprintln(r.Output, tr) - if len(tr.Output) > 0 { - fmt.Fprintln(r.Output) - fmt.Fprintln(newIndentingWriter(r.Output), strings.TrimSpace(string(tr.Output))) - fmt.Fprintln(r.Output) + } else if r.Verbose || !tr.Pass() { + if tr.Location != nil && tr.Location.File != lastFile { + if lastFile != "" { + fmt.Fprintln(r.Output, "") + } + fmt.Fprintf(r.Output, "%s:\n", tr.Location.File) + lastFile = tr.Location.File } - } else if !tr.Pass() { dirty = true fmt.Fprintln(r.Output, tr) if len(tr.Output) > 0 { diff --git a/tester/reporter_test.go b/tester/reporter_test.go index 9976d2a174..afaf527e82 100644 --- a/tester/reporter_test.go +++ b/tester/reporter_test.go @@ -36,29 +36,44 @@ func TestPrettyReporterVerbose(t *testing.T) { Package: "data.foo.bar", Name: "test_baz", Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_qux", Error: fmt.Errorf("some err"), Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_corge", Fail: true, Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy2.rego", + }, }, { Package: "data.foo.bar", Name: "todo_test_qux", Skip: true, Trace: nil, + Location: &ast.Location{ + File: "policy2.rego", + }, }, { Package: "data.foo.bar", Name: "test_contains_print", Output: []byte("fake print output\n"), + Location: &ast.Location{ + File: "policy3.rego", + }, }, } @@ -80,11 +95,16 @@ data.foo.bar.test_corge: FAIL (0s) SUMMARY -------------------------------------------------------------------------------- +policy1.rego: data.foo.bar.test_baz: PASS (0s) data.foo.bar.test_qux: ERROR (0s) some err + +policy2.rego: data.foo.bar.test_corge: FAIL (0s) data.foo.bar.todo_test_qux: SKIPPED + +policy3.rego: data.foo.bar.test_contains_print: PASS (0s) fake print output @@ -113,35 +133,53 @@ func TestPrettyReporter(t *testing.T) { Package: "data.foo.bar", Name: "test_baz", Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_qux", Error: fmt.Errorf("some err"), Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_corge", Fail: true, Trace: getFakeTraceEvents(), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "todo_test_qux", Skip: true, Trace: nil, + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_contains_print_pass", Output: []byte("fake print output\n"), + Location: &ast.Location{ + File: "policy1.rego", + }, }, { Package: "data.foo.bar", Name: "test_contains_print_fail", Fail: true, Output: []byte("fake print output2\n"), + Location: &ast.Location{ + File: "policy2.rego", + }, }, } @@ -154,10 +192,13 @@ func TestPrettyReporter(t *testing.T) { t.Fatal(err) } - exp := `data.foo.bar.test_qux: ERROR (0s) + exp := `policy1.rego: +data.foo.bar.test_qux: ERROR (0s) some err data.foo.bar.test_corge: FAIL (0s) data.foo.bar.todo_test_qux: SKIPPED + +policy2.rego: data.foo.bar.test_contains_print_fail: FAIL (0s) fake print output2