Skip to content

Commit

Permalink
Use structured representation for trace
Browse files Browse the repository at this point in the history 
Previoulsy the trace was stored on the test result as a string which
meant that when it was serialized into JSON, the caller would receive a
pretty printed version of the trace. Callers requesting the JSON
representation typically consume the output programatically, so it's
better to return the raw JSON representation of the trace in that case.

Also, update the JSON format test to check output equality using
reflect.DeepEqual instead of string comparison which is subject to
whitespace.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
@tsandall
tsandall committed Oct 2, 2018
1 parent 2b17188 commit 728c929
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 36 deletions.
7 changes: 5 additions & 2 deletions tester/reporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"io"
"strings"

"github.com/open-policy-agent/opa/topdown"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cover"
)
Expand Down Expand Up @@ -54,10 +56,11 @@ func (r PrettyReporter) Report(ch chan *Result) error {
for _, failure := range failures {
fmt.Fprintln(r.Output, failure)
fmt.Fprintln(r.Output)
fmt.Fprintln(newIndentingWriter(r.Output), failure.Trace)
topdown.PrettyTrace(newIndentingWriter(r.Output), failure.Trace)
fmt.Fprintln(r.Output)
}

fmt.Fprintln(r.Output, "\nSUMMARY")
fmt.Fprintln(r.Output, "SUMMARY")
r.hl()
}

Expand Down
151 changes: 133 additions & 18 deletions tester/reporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,34 @@ package tester
import (
"bytes"
"fmt"
"reflect"
"testing"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/topdown"
"github.com/open-policy-agent/opa/util"
)

func getFakeTraceEvents() []*topdown.Event {
return []*topdown.Event{
{
Op: topdown.FailOp,
Node: ast.MustParseExpr("true = false"),
QueryID: 0,
ParentID: 0,
},
}
}

func TestPrettyReporterVerbose(t *testing.T) {
var buf bytes.Buffer

// supply fake trace events for each kind of event to ensure that only failures
// report traces.
ts := []*Result{
{nil, "data.foo.bar", "test_baz", false, nil, 0, "trace test_baz"},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, "trace test_qux"},
{nil, "data.foo.bar", "test_corge", true, nil, 0, "trace test_corge"},
{nil, "data.foo.bar", "test_baz", false, nil, 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_corge", true, nil, 0, getFakeTraceEvents()},
}

r := PrettyReporter{
Expand All @@ -28,7 +47,7 @@ func TestPrettyReporterVerbose(t *testing.T) {
--------------------------------------------------------------------------------
data.foo.bar.test_corge: FAIL (0s)
trace test_corge
| Fail true = false
SUMMARY
--------------------------------------------------------------------------------
Expand All @@ -49,10 +68,13 @@ ERROR: 1/3

func TestPrettyReporter(t *testing.T) {
var buf bytes.Buffer

// supply fake trace events to verify that traces are suppressed without verbose
// flag.
ts := []*Result{
{nil, "data.foo.bar", "test_baz", false, nil, 0, "trace test_baz"},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, "trace test_qux"},
{nil, "data.foo.bar", "test_corge", true, nil, 0, "trace test_corge"},
{nil, "data.foo.bar", "test_baz", false, nil, 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_corge", true, nil, 0, getFakeTraceEvents()},
}

r := PrettyReporter{
Expand Down Expand Up @@ -81,48 +103,141 @@ ERROR: 1/3
func TestJSONReporter(t *testing.T) {
var buf bytes.Buffer
ts := []*Result{
{nil, "data.foo.bar", "test_baz", false, nil, 0, "trace test_baz"},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, "trace test_qux"},
{nil, "data.foo.bar", "test_corge", true, nil, 0, "trace test_corge"},
{nil, "data.foo.bar", "test_baz", false, nil, 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0, getFakeTraceEvents()},
{nil, "data.foo.bar", "test_corge", true, nil, 0, getFakeTraceEvents()},
}

r := JSONReporter{
Output: &buf,
}

ch := resultsChan(ts)

if err := r.Report(ch); err != nil {
t.Fatal(err)
}

exp := `[
exp := util.MustUnmarshalJSON([]byte(`[
{
"location": null,
"package": "data.foo.bar",
"name": "test_baz",
"duration": 0,
"trace": "trace test_baz"
"trace": [
{
"Op": "Fail",
"Node": {
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "boolean",
"value": true
},
{
"type": "boolean",
"value": false
}
]
},
"QueryID": 0,
"ParentID": 0,
"Locals": null,
"Message": ""
}
]
},
{
"location": null,
"package": "data.foo.bar",
"name": "test_qux",
"error": {},
"duration": 0,
"trace": "trace test_qux"
"trace": [
{
"Op": "Fail",
"Node": {
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "boolean",
"value": true
},
{
"type": "boolean",
"value": false
}
]
},
"QueryID": 0,
"ParentID": 0,
"Locals": null,
"Message": ""
}
]
},
{
"location": null,
"package": "data.foo.bar",
"name": "test_corge",
"fail": true,
"duration": 0,
"trace": "trace test_corge"
}
"trace": [
{
"Op": "Fail",
"Node": {
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "boolean",
"value": true
},
{
"type": "boolean",
"value": false
}
]
},
"QueryID": 0,
"ParentID": 0,
"Locals": null,
"Message": ""
}
] }
]
`
`))

if exp != buf.String() {
t.Fatalf("Expected:\n\n%v\n\nGot:\n\n%v", exp, buf.String())
result := util.MustUnmarshalJSON(buf.Bytes())

if !reflect.DeepEqual(result, exp) {
t.Fatalf("Expected:\n\n%v\n\nGot:\n\n%v", exp, result)
}
}

Expand Down
28 changes: 12 additions & 16 deletions tester/runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package tester

import (
"bytes"
"context"
"fmt"
"sort"
Expand Down Expand Up @@ -49,16 +48,16 @@ func RunWithFilter(ctx context.Context, filter loader.Filter, paths ...string) (

// Result represents a single test case result.
type Result struct {
Location *ast.Location `json:"location"`
Package string `json:"package"`
Name string `json:"name"`
Fail bool `json:"fail,omitempty"`
Error error `json:"error,omitempty"`
Duration time.Duration `json:"duration"`
Trace string `json:"trace,omitempty"`
Location *ast.Location `json:"location"`
Package string `json:"package"`
Name string `json:"name"`
Fail bool `json:"fail,omitempty"`
Error error `json:"error,omitempty"`
Duration time.Duration `json:"duration"`
Trace []*topdown.Event `json:"trace,omitempty"`
}

func newResult(loc *ast.Location, pkg, name string, duration time.Duration, trace string) *Result {
func newResult(loc *ast.Location, pkg, name string, duration time.Duration, trace []*topdown.Event) *Result {
return &Result{
Location: loc,
Package: pkg,
Expand Down Expand Up @@ -174,14 +173,11 @@ func (r *Runner) runTest(ctx context.Context, mod *ast.Module, rule *ast.Rule) (
rs, err := rego.Eval(ctx)
dt := time.Since(t0)

var trace string
if btr, ok := r.tracer.(*topdown.BufferTracer); ok {
buf := bytes.Buffer{}
topdown.PrettyTrace(&buf, *btr)
trace = buf.String()
var trace []*topdown.Event

// "reset" tracer
r.tracer = topdown.NewBufferTracer()
if buf, ok := r.tracer.(*topdown.BufferTracer); ok {
trace = *buf
r.tracer = topdown.NewBufferTracer() // reset the tracer for each run
}

tr := newResult(rule.Loc(), mod.Package.Path.String(), string(rule.Head.Name), dt, trace)
Expand Down

0 comments on commit 728c929

Please sign in to comment.