-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
bumping k8s versions in test files #3477
bumping k8s versions in test files #3477
Conversation
/approve |
6a0d3c5
to
df9f621
Compare
cmd/kops/integration_test.go
Outdated
@@ -73,7 +73,7 @@ func TestComplex(t *testing.T) { | |||
func TestMinimalCloudformation(t *testing.T) { | |||
//runTestCloudformation(t, "minimal.example.com", "../../tests/integration/minimal", "v1alpha0", false) | |||
//runTestCloudformation(t, "minimal.example.com", "../../tests/integration/minimal", "v1alpha1", false) | |||
runTestCloudformation(t, "minimal.example.com", "../../tests/integration/minimal", "v1alpha2", false) | |||
runTestCloudformation(t, "minimal.example.com", "../../tests/integration/cloudformation", "v1alpha2", false) |
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.
minimal is the only one where we actually test CF output - in theory we should test them all, but I'm not convinced it is worth it.
Do we have to split off the cloudformation directory? I don't understand why, and it isn't entirely right in that we actually should have a CF test in every dir, just that I think we get most of the value from a TF test |
/retest |
@chrislovecnm PR needs rebase |
edffdb0
to
6aad3d8
Compare
The other tests are far more flexible. For code maintainability breaking out the CF stuff makes sense in my head. Put the complex stuff into its own area, so a new developer is not trying to figure out which test is broken and why. I can move it back, but would rather iterate on it. I have another PR to bump the versions, which requires more edits to the CF stuff. It made sense to me to break it out, but I can put it back if it is a show stopper for you. |
6aad3d8
to
5ed0d47
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrislovecnm, justinsb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue. |
updating k8s versions in test YAML files so that we do not get warnings during testing