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

Make opa test -v default to showing trace with fails and notes #2068

Closed
patrick-east opened this issue Feb 7, 2020 · 1 comment · Fixed by #2267
Closed

Make opa test -v default to showing trace with fails and notes #2068

patrick-east opened this issue Feb 7, 2020 · 1 comment · Fixed by #2267

Comments

@patrick-east
Copy link
Contributor

Desired Behavior

When some test fails when running opa test and -v is specified you get the default fails explain mode trace. If you then add in some trace() calls into the policies to troubleshoot you have to specify --explain notes to see them in a human friendly way... but you lose the context of the fails trace. It would probably be the most useful default to show both so that someone wouldn't have to use --explain full to see both failure events and note output.

Proposed change

We can add a new --explain option to be the default for opa test and have it combine both notes and fail nodes of the trace. Alternatively we can make the --explain option take in a list of modes and then default to fails and notes, but allow it to be more flexible for other users that may have the need to combine other explain modes in the future.

@tsandall
Copy link
Member

Alternatively we can make the --explain option take in a list of modes and then default to fails and notes, but allow it to be more flexible for other users that may have the need to combine other explain modes in the future.

In terms of flexibility this would be nice however I think that some explanation modes will be mutually exclusive. We could address this with error checking but I'm not sure how usable it will be.

Another option would be to update the code in topdown to expose the predicates that filter for Fail and Note events and then compose them when -v is specified. IOW, do the right thing when -v is specified -- output the notes and the fails.

In conjunction with #2069 (which just makes opa test emit the trace when --explain is given) this would provide a nice improvement to the tester.

patrick-east added a commit to patrick-east/opa that referenced this issue Apr 3, 2020
The default value for `--explain` was used when `--verbose`/`-v` was
specified. Now if only a verbose flag is specified we will show both
"fails" and "notes", which is most likely what someone would want
when troubleshooting some test failure.

Fixes: open-policy-agent#2068
Signed-off-by: Patrick East <east.patrick@gmail.com>
tsandall pushed a commit that referenced this issue Apr 6, 2020
The default value for `--explain` was used when `--verbose`/`-v` was
specified. Now if only a verbose flag is specified we will show both
"fails" and "notes", which is most likely what someone would want
when troubleshooting some test failure.

Fixes: #2068
Signed-off-by: Patrick East <east.patrick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants