-
Notifications
You must be signed in to change notification settings - Fork 16
cleanup: update test beforeAll #675
Conversation
Shabirmean
left a comment
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.
Some minor comments and suggestions. Is there a way we can test? Are we creating clusters in the JDST project and re-triggering the CI on this PR?
google-cloud-container/src/test/java/com/google/cloud/container/v1/it/Util.java
Show resolved
Hide resolved
google-cloud-container/src/test/java/com/google/cloud/container/v1/it/Util.java
Outdated
Show resolved
Hide resolved
google-cloud-container/src/test/java/com/google/cloud/container/v1/it/Util.java
Show resolved
Hide resolved
|
LGTM. But address Shabir's comments before merging. |
Shabirmean
left a comment
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!
I see that the latest commits haven't still updated the code based on the comments. Maybe leave a feedback if you choose to not change them
It seems owlbot overwrote the changes I made regarding the |
Background
In this PR https://github.com/googleapis/java-container/issues/656 we saw that the quota for our clusters hit a max since tests were not cleaning up after themselves. Per Neenu's suggestion, we added a cleanup to the
beforeAllto ensure that old clusters are deleted