trace+tester: Adding local var values to trace and test report #6815

johanfylling · 2024-06-12T14:17:55Z

Local variable values in test report

When enabled (--var-values flag), the failure report for tests will contain values for variables local to the failing expression. e.g.:

FAILURES
--------------------------------------------------------------------------------
data.test.test_foo: FAIL (0ms)

  test.rego:8:
    	x == y + z
    	|    |   |
    	|    |   3
    	|    y + z: 5
    	|    y: 2
    	1

SUMMARY
--------------------------------------------------------------------------------
test.rego:
data.test.test_foo: FAIL (0ms)
--------------------------------------------------------------------------------
FAIL: 1/1

Local variable values in pretty trace

When enabled on eval and test commands (--var-values flag), local variable values are added to pretty trace:

query:1          Enter data.test.p = _                                {}
query:1          | Eval data.test.p = _                               {}
query:1          | Index data.test.p (matched 1 rule, early exit)     {}
/test.rego:4     | Enter data.test.p                                  {}
/test.rego:5     | | Eval x = 1                                       {}
/test.rego:6     | | Eval y = 2                                       {}
/test.rego:7     | | Eval z = 3                                       {}
/test.rego:8     | | Eval minus(z, y, __local3__)                     {y: 2, z: 3}
/test.rego:8     | | Eval x = __local3__                              {__local3__: 1, x: 1}
/test.rego:4     | | Exit data.test.p early                           {}
query:1          | Exit data.test.p = _                               {_: true, data.test.p: true}
query:1          Redo data.test.p = _                                 {_: true, data.test.p: true}
query:1          | Redo data.test.p = _                               {_: true, data.test.p: true}
/test.rego:4     | Redo data.test.p                                   {}
/test.rego:8     | | Redo x = __local3__                              {__local3__: 1, x: 1}
/test.rego:8     | | Redo minus(z, y, __local3__)                     {__local3__: 1, y: 2, z: 3}
/test.rego:7     | | Redo z = 3                                       {z: 3}
/test.rego:6     | | Redo y = 2                                       {y: 2}
/test.rego:5     | | Redo x = 1                                       {x: 1}

Fixing: #2546

Local variable values in test report ------------------------------------ When enabled (`--var-values` flag), the failure report for tests will contain values for variables local to the failing expression. e.g.: ``` FAILURES -------------------------------------------------------------------------------- data.test.test_foo: FAIL (0ms) test.rego:8: x == y + z | | | | | 3 | y + z: 5 | y: 2 1 SUMMARY -------------------------------------------------------------------------------- test.rego: data.test.test_foo: FAIL (0ms) -------------------------------------------------------------------------------- FAIL: 1/1 ``` Local variable values in pretty trace ------------------------------------- When enabled on `eval` and `test` commands (`--var-values` flag), local variable values are added to pretty trace: ``` query:1 Enter data.test.p = _ {} query:1 | Eval data.test.p = _ {} query:1 | Index data.test.p (matched 1 rule, early exit) {} /test.rego:4 | Enter data.test.p {} /test.rego:5 | | Eval x = 1 {} /test.rego:6 | | Eval y = 2 {} /test.rego:7 | | Eval z = 3 {} /test.rego:8 | | Eval minus(z, y, __local3__) {y: 2, z: 3} /test.rego:8 | | Eval x = __local3__ {__local3__: 1, x: 1} /test.rego:4 | | Exit data.test.p early {} query:1 | Exit data.test.p = _ {_: true, data.test.p: true} query:1 Redo data.test.p = _ {_: true, data.test.p: true} query:1 | Redo data.test.p = _ {_: true, data.test.p: true} /test.rego:4 | Redo data.test.p {} /test.rego:8 | | Redo x = __local3__ {__local3__: 1, x: 1} /test.rego:8 | | Redo minus(z, y, __local3__) {__local3__: 1, y: 2, z: 3} /test.rego:7 | | Redo z = 3 {z: 3} /test.rego:6 | | Redo y = 2 {y: 2} /test.rego:5 | | Redo x = 1 {x: 1} ``` Fixing: open-policy-agent#2546 Signed-off-by: Johan Fylling <[email protected]>

netlify · 2024-06-12T14:19:18Z

✅ Deploy Preview for openpolicyagent ready!

Name	Link
🔨 Latest commit	`4ea0538`
🔍 Latest deploy log	https://app.netlify.com/sites/openpolicyagent/deploys/667a99d0d1b8ff000899632e
😎 Deploy Preview	https://deploy-preview-6815--openpolicyagent.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Johan Fylling <[email protected]>

johanfylling · 2024-06-12T16:40:59Z

The linter fails because it gets confused by some newer golang stuff: #6817

…iler stage Signed-off-by: Johan Fylling <[email protected]>

Signed-off-by: Johan Fylling <[email protected]>

johanfylling · 2024-06-14T14:35:43Z

cmd/test.go

 	testCommand.Flags().BoolVar(&testParams.benchmark, "bench", false, "benchmark the unit tests")
 	testCommand.Flags().StringVarP(&testParams.runRegex, "run", "r", "", "run only test cases matching the regular expression.")
 	testCommand.Flags().BoolVarP(&testParams.watch, "watch", "w", false, "watch command line files for changes")
+	testCommand.Flags().BoolVar(&testParams.varValues, "var-values", false, "show local variable values in test output")


Naming suggestions appreciated 😄.

srenatus

Some comments after reading through the change, skimming over a few lines here and there 😎

srenatus · 2024-06-19T12:12:40Z

ast/compile.go

+	useTypeCheckAnnotations                  bool                          // whether to provide annotated information (schemas) to the type checker
+	allowUndefinedFuncCalls                  bool                          // don't error on calls to unknown functions.
+	evalMode                                 CompilerEvalMode
+	rewriteTestRulesToCaptureUnboundDynamics bool


"Dynamics" -- it's not a term that's common in ast or topdown, is it? 🤔

True 😄, I think that trickled up from the rewriteDynamics*() naming convention we have in the compiler. Naming suggestions are welcome :). It's a bit too long for my liking too ..

topdown/trace.go

topdown/trace_test.go

srenatus · 2024-06-19T12:17:37Z

ast/compile.go

+			return false
+		})
+	}
+}


Do you think it would be possible to add a few tests cases showing of the rewriting that's happening here? I'm afraid I don't understand it just from the code 😅 Maybe I just haven't found them.

Good point, I'll add some of those 👍 .

I've added some tests for this compiler stage. Hope they make things more clear.

srenatus · 2024-06-19T12:18:06Z

docs/content/policy-testing.md

+              |                     |                          |
+              |                     |                          {"read": ["alice", "bob"], "write": ["jane"]}
+              |                     "jane"
+              ["write"]


👏 This is going to be a tremendous help!

Signed-off-by: Johan Fylling <[email protected]>

docs/content/policy-testing.md

Signed-off-by: Johan Fylling <[email protected]>

johanfylling · 2024-06-20T07:30:08Z

.github/workflows/pull-request.yaml

      matrix:
        os: [ubuntu-22.04, macos-14]
-        version: ["1.20"]
+        version: ["1.21"]


0.20 isn't a supported Go version anyways. I think we simply forgot to bump this when we bumped the build version to 0.22.

Signed-off-by: Johan Fylling <[email protected]>

srenatus

Thanks for the tests and the rename! LGTM

Dropping unused type

a83c5af

Signed-off-by: Johan Fylling <[email protected]>

johanfylling added 2 commits June 14, 2024 11:18

Not rewriting negated expressions in RewriteTestRuleEqualities comp…

29eedf6

…iler stage Signed-off-by: Johan Fylling <[email protected]>

Adding documentation

1b3c532

Signed-off-by: Johan Fylling <[email protected]>

johanfylling commented Jun 14, 2024

View reviewed changes

johanfylling marked this pull request as ready for review June 14, 2024 14:36

srenatus reviewed Jun 19, 2024

View reviewed changes

johanfylling added 2 commits June 19, 2024 15:14

Merge branch 'main' into test_pretty_var_values

126f61f

Adding trace tests

13a385d

Signed-off-by: Johan Fylling <[email protected]>

johanfylling commented Jun 19, 2024

View reviewed changes

docs/content/policy-testing.md Show resolved Hide resolved

Bumping go-compat version to 1.21

b8c03fd

Signed-off-by: Johan Fylling <[email protected]>

johanfylling commented Jun 20, 2024

View reviewed changes

Adding tests for RewriteTestRulesForTracing compiler stage

27f8f49

Signed-off-by: Johan Fylling <[email protected]>

johanfylling requested a review from srenatus June 25, 2024 09:38

Merge branch 'main' into test_pretty_var_values

4ea0538

srenatus approved these changes Jun 25, 2024

View reviewed changes

johanfylling merged commit 96ecf38 into open-policy-agent:main Jun 25, 2024

johanfylling deleted the test_pretty_var_values branch June 25, 2024 12:31

ashutosh-narkar mentioned this pull request Jul 8, 2024

opa test should show readable errors when equality failed #2546

Closed

trace+tester: Adding local var values to trace and test report #6815

trace+tester: Adding local var values to trace and test report #6815

Uh oh!

Conversation

johanfylling commented Jun 12, 2024

Local variable values in test report

Local variable values in pretty trace

Uh oh!

netlify bot commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for openpolicyagent ready!

Uh oh!

johanfylling commented Jun 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srenatus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srenatus left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify bot commented Jun 12, 2024 •

edited

Loading