-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix TestAccContainerCluster_withIPAllocationPolicy test #1065
Fix TestAccContainerCluster_withIPAllocationPolicy test #1065
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.
I think the goal of the test was to hit this error: https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_container_cluster.go#L1112. If it's not a valid error message to be throwing then we should also remove that section of code (although I'm definitely curious why it stopped throwing the error)
One step was expecting the test to fail if the subnetwork defines secondary ip ranges that the cluster doesn't use. However, it is perfectly fine to do so and we don't expect an error.
…licy" This reverts commit af2f369.
af2f369
to
d04231e
Compare
…feature in allocation policy
I fixed the code instead to throw the error at the right location. --- PASS: TestAccContainerCluster_withIPAllocationPolicy (634.96s) |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Step 2 is failing because an empty
ip_allocation_policy
block doesn't fail.It fails to cast as a
map[string]interface{}
but this does not return an error. The if throwing the error could not be reached.