Skip to content

Commit

Permalink
topdown: Fix trace to set location on notes
Browse files Browse the repository at this point in the history
Previously the note events would not have a location on them which
mean they were difficult to track down (you would have to grep for the
message and hope it shows up.) With this change we just include the
location on notes like all other events.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Apr 17, 2020
1 parent a90187a commit 23e2a51
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
4 changes: 2 additions & 2 deletions repl/repl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2032,10 +2032,10 @@ func TestEvalNotes(t *testing.T) {
repl.OneShot(ctx, "p")
expected := strings.TrimSpace(`query:1 Enter data.repl.p = _
query:1 | Enter data.repl.p
note | | Note "x = 2"
query:1 | | Note "x = 2"
query:1 Redo data.repl.p = _
query:1 | Redo data.repl.p
note | | Note "x = 3"
query:1 | | Note "x = 3"
true`)
expected += "\n"
if expected != buffer.String() {
Expand Down
4 changes: 1 addition & 3 deletions topdown/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,6 @@ func numDigits10(n int) int {
}

func formatLocation(event *Event, fileAliases map[string]string) string {
if event.Op == NoteOp {
return fmt.Sprintf("%v", "note")
}

location := event.Location
if location == nil {
Expand Down Expand Up @@ -350,6 +347,7 @@ func builtinTrace(bctx BuiltinContext, args []*ast.Term, iter func(*ast.Term) er

evt := &Event{
Op: NoteOp,
Location: bctx.Location,
QueryID: bctx.QueryID,
ParentID: bctx.ParentID,
Message: string(str),
Expand Down
18 changes: 9 additions & 9 deletions topdown/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func TestPrettyTraceWithLocationTruncatedPaths(t *testing.T) {
compiler := ast.MustCompileModules(map[string]string{
"authz_bundle/com/foo/bar/baz/qux/acme/corp/internal/authz/policies/abac/v1/beta/policy.rego": `
package test
import data.utils.q
p = true { q[x]; plus(x, 1, n) }
Expand Down Expand Up @@ -349,13 +349,13 @@ func TestPrettyTracePartialWithLocationTruncatedPaths(t *testing.T) {
package example_rbac
default allow = false
allow {
data.utils.user_has_role[role_name]
data.utils.role_has_permission[role_name]
}
`,
"authz_bundle/com/foo/bar/baz/qux/acme/corp/internal/authz/policies/utils/user.rego": `
package utils
Expand All @@ -365,7 +365,7 @@ func TestPrettyTracePartialWithLocationTruncatedPaths(t *testing.T) {
role_binding.role = role_name
role_binding.user = input.subject.user
}
role_has_permission[role_name] {
role = data.roles[_]
role.name = role_name
Expand Down Expand Up @@ -679,7 +679,7 @@ query:4 | | | Exit data.test.q
query:3 | | Eval plus(x, 1, n)
query:3 | | Eval sprintf("n= %v", [n], __local0__)
query:3 | | Eval trace(__local0__)
note | | Note "n= 2"
query:3 | | Note "n= 2"
query:3 | | Exit data.test.p
query:1 | Exit data.test.p = _
query:1 Redo data.test.p = _
Expand All @@ -695,7 +695,7 @@ query:4 | | | Exit data.test.q
query:3 | | Eval plus(x, 1, n)
query:3 | | Eval sprintf("n= %v", [n], __local0__)
query:3 | | Eval trace(__local0__)
note | | Note "n= 3"
query:3 | | Note "n= 3"
query:3 | | Exit data.test.p
query:3 | Redo data.test.p
query:3 | | Redo trace(__local0__)
Expand All @@ -708,7 +708,7 @@ query:4 | | | Exit data.test.q
query:3 | | Eval plus(x, 1, n)
query:3 | | Eval sprintf("n= %v", [n], __local0__)
query:3 | | Eval trace(__local0__)
note | | Note "n= 4"
query:3 | | Note "n= 4"
query:3 | | Exit data.test.p
query:3 | Redo data.test.p
query:3 | | Redo trace(__local0__)
Expand All @@ -721,7 +721,7 @@ query:4 | | | Exit data.test.q
query:3 | | Eval plus(x, 1, n)
query:3 | | Eval sprintf("n= %v", [n], __local0__)
query:3 | | Eval trace(__local0__)
note | | Note "n= 5"
query:3 | | Note "n= 5"
query:3 | | Exit data.test.p
query:3 | Redo data.test.p
query:3 | | Redo trace(__local0__)
Expand Down

0 comments on commit 23e2a51

Please sign in to comment.