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.

Sign up for GitHub

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

Jump to bottom

enhancement: Add support for test fixtures #294

Merged
merged 36 commits into from
Sep 20, 2021
Merged

enhancement: Add support for test fixtures #294

merged 36 commits into from
Sep 20, 2021

Conversation

dbuduev
Copy link
Contributor

@dbuduev dbuduev commented Sep 15, 2021

Description

Adds support for test fixtures.
Discussion: #285

Fixes #14

Checklist

  • The PR title has the correct prefix
  • PR is linked to the corresponding issue
  • All commits are signed-off (git commit -s ...) to provide the DCO

@dbuduev
fix a typo
f56bd22 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
wip
ee13f99 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
adds filesystem util
5f8c9f5 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
load test fixture
392eca7 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
getTests
54211fe 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
add load principal tests
3efc4c6 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
Test_testFixture_getTests
2cccd69 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
add test fixture unit test
f6add58 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
json -> yaml
9ebb89a 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
add doVerify tests
3dbabd9 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
refactoring
c99b9eb 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
add tests
0a97853 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
fix formmatting
387fece 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
refactoring
9fe97f1 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
make doVerify test recursive
437281f 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
charithe
charithe reviewed Sep 16, 2021
View reviewed changes
Copy link
Contributor

@charithe charithe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good 👍🏽 I didn't dive too deep because it's still a draft but I pointed out some of the things that stood out to me.

One thing I am wondering is whether we should support inline principal and resource definitions as well. Otherwise, in order to get a test suite up, the user needs to create three files and a directory -- which is a lot of ceremony. I think we should make it very easy to get started writing tests and then refactor them as they get complicated. WDYT?

Finally, once the implementation is done, let's remember to update the testing documentation and the photo-share tutorial to reflect the changes made here.

api/public/cerbos/policy/v1/policy.proto Outdated Show resolved Hide resolved
api/public/cerbos/policy/v1/policy.proto Outdated Show resolved Hide resolved
internal/util/filesystem.go Outdated Show resolved Hide resolved
internal/verify/verify.go Outdated Show resolved Hide resolved
@dbuduev dbuduev marked this pull request as ready for review September 16, 2021 07:15
@dbuduev
update proto definitions
aec7b29 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
update OpenOneOfSupportedFiles
c9690ad 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
refactoring
9beb2d9 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
permit embedding prinicipals & resources in a test suite
5433ac3 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
linting
a5cdbb8 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
Copy link
Contributor Author

dbuduev commented Sep 17, 2021 •
edited
Loading

I have implemented the changes you have proposed including the embedded principals and resources option.

TODO:

  • update the documentation
  • check whether misconfigured tests error reporting has good UX

@dbuduev
refactoring
663c70f 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
charithe
charithe reviewed Sep 17, 2021
View reviewed changes
api/public/cerbos/policy/v1/policy.proto Outdated Show resolved Hide resolved
internal/verify/test_fixture.go Outdated Show resolved Hide resolved
@dbuduev
requestID can be optional
d0fad09 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
requestID can be optional
2716ea7 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
always show TestResult.Failed
2c30b28 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
change formatTestname
a6b182f 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
convert test.Name to a message
7504072 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
change documentation
020311f 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
change documentation
404db3e 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
rename a test result field
762c7b3 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
refactoring
ec53bee 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
formatting
f89fde7 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
charithe
charithe reviewed Sep 20, 2021
View reviewed changes
Copy link
Contributor

@charithe charithe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there. Just a few small things to address.

docs/modules/policies/pages/compile.adoc Outdated Show resolved Hide resolved
docs/modules/policies/pages/compile.adoc Outdated Show resolved Hide resolved
docs/modules/policies/pages/compile.adoc Outdated Show resolved Hide resolved
docs/modules/policies/pages/compile.adoc Show resolved Hide resolved
internal/verify/verify.go Outdated Show resolved Hide resolved
dbuduev and others added 5 commits September 20, 2021 19:39
@dbuduev @charithe
Update docs/modules/policies/pages/compile.adoc
29f34d0 
Co-authored-by: Charith Ellawala <charithe@users.noreply.github.com>
@dbuduev @charithe
Update docs/modules/policies/pages/compile.adoc
80f44c1 
Co-authored-by: Charith Ellawala <charithe@users.noreply.github.com>
@dbuduev @charithe
Update docs/modules/policies/pages/compile.adoc
9c1cebd 
Co-authored-by: Charith Ellawala <charithe@users.noreply.github.com>
@dbuduev
documentation update
38834e0 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
@dbuduev
add more tests
9af426f 
Signed-off-by: Dennis Buduev <dennis@cerbos.dev>
charithe
charithe approved these changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@charithe charithe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice job 👍🏽

@charithe
Copy link
Contributor

I'll do a sudo merge to override the protobuf check.

@charithe charithe merged commit 2596673 into cerbos:main Sep 20, 2021
@dbuduev dbuduev deleted the add-ext-refs-to-tests branch September 21, 2021 00:22
@charithe charithe mentioned this pull request Oct 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charithe charithe charithe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for test fixtures
2 participants
@dbuduev @charithe