-
Notifications
You must be signed in to change notification settings - Fork 87
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
[KEP-0009] feat: add expression based assertions #576
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but needs a few changes.
pkg/test/utils/kubernetes.go
Outdated
variables[resourceRef.Id] = referencedResource.Object | ||
} | ||
|
||
env, err := cel.NewEnv() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling cel.NewEnv()
may not be enough, you probably want to enable a couple of options/libs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddycharly any hints on what might be useful? 🤔
0fe748e
to
a85752a
Compare
Thanks for the quick review @porridge . I'll add the tests tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks much better, but some more changes are needed. 🙏🏻
Please see inline.
This PR adds CEL-expression based assertions to `TestAsserts`. See https://github.com/kudobuilder/kuttl/blob/main/keps/0009-expression-based-assertions.md for more details. Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
…re present Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
…pace Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
7044f54
to
a7122a6
Compare
There was a problem hiding this 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 bunch of nitpicks for messages and identifier names, and one issue in the test..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to 00-assert.yaml
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm expecting the lexicographically last file to be the assert: 😅
...gration_test_data/assert_expressions/check_expression_for_ephemeral_namespace/00-create.yaml
Outdated
Show resolved
Hide resolved
pkg/test/utils/kubernetes.go
Outdated
variables[resourceRef.Id] = referencedResource.Object | ||
} | ||
|
||
env, err := cel.NewEnv() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddycharly any hints on what might be useful? 🤔
Signed-off-by: Kumar Mallikarjuna <[email protected]>
1752619
to
32bcb9c
Compare
Signed-off-by: Kumar Mallikarjuna <[email protected]>
Signed-off-by: Kumar Mallikarjuna <[email protected]>
What this PR does / why we need it:
This PR adds CEL-expression based assertions to
TestAsserts
. See https://github.com/kudobuilder/kuttl/blob/main/keps/0009-expression-based-assertions.md for more details.Fixes #562