-
Notifications
You must be signed in to change notification settings - Fork 102
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
Proposal for E2E and CLI testing via test harness. #625
Proposal for E2E and CLI testing via test harness. #625
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.
This can be also very interesting for testing demos in kudobuilder/operators
. I like!
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.
LGTM - just one small nit.
Also I am not sure how are we going to actually install the kudo CLI into kubectl prior to tests? But I guess that's an implementation detail :)
@@ -151,7 +151,9 @@ type TestSuite struct { | |||
ManifestsDirs []string `json:"manifestsDirs"` | |||
// Directories containing test cases to run. | |||
TestDirs []string | |||
// Whether or not to start a local etcd and kubernetes API server for the tests (cannot be set with StartKIND | |||
// Kubectl specifies a list of kubectl commands to run prior to running the tests. | |||
Kubectl []string `json:"kubectl"` |
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.
can we name this KubectlCmd
then or something like that?
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'd originally had kubectlCommands
but thought it was verbose/confusing (as a user: "should it be pluralized? camel-cased? did they spell out "commands" or not?"), so shortened to just kubectl
. What do you think?
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.
IMO let's discuss this one more on the implementation and we can adjust it there.
My thinking right now was that the CLI would just be expected to be installed and then we can have a makefile target that installs the CLI and runs the test. Of course, this presents a risk that someone might have the wrong version installed when testing. |
What type of PR is this?
/kind kep
What this PR does / why we need it:
This documents how we will do E2E testing as well as a design for running CLI commands as a part of test steps.
Which issue(s) this PR fixes:
Fixes #530
Special notes for your reviewer:
In this round of the design, I have focused on including CLI commands in test steps - e.g., creating, updating, and deleting resources in Kubernetes. It does not focus on CLI commands in test asserts - e.g., reading resources and testing the outputs of the CLI. I'm still thinking about how the design there should work, but this covers the majority of the use-cases we would want to test.
Does this PR introduce a user-facing change?: