Skip to content
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

chore(compute): add integration tests for compute #4421

Merged
merged 23 commits into from Jul 16, 2021

Conversation

georgiyekkert
Copy link
Contributor

No description provided.

@georgiyekkert georgiyekkert requested a review from a team as a code owner July 12, 2021 20:57
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 12, 2021
@noahdietz noahdietz changed the title tests: add integration tests for compute chore(compute): add integration tests for compute Jul 12, 2021
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these, some initial feedback.

compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
@codyoss
Copy link
Member

codyoss commented Jul 12, 2021

Whoops had an old browser tab open that had not updated. Sorry for any dupes that Noah may have mentioned.

@product-auto-label product-auto-label bot added the api: compute Issues related to the Compute Engine API. label Jul 13, 2021
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it looks like CI is failing do to this file needs to be formatted. A run of goimports should fix this.

compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but @codyoss needs to Approve. Thanks Georgii.

Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only commented on the first test but similar patterns are seen in other tests.

compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Outdated Show resolved Hide resolved
compute/apiv1/smoke_test.go Show resolved Hide resolved
@codyoss codyoss added the automerge Merge the pull request once unit tests and other checks pass. label Jul 14, 2021
@codyoss
Copy link
Member

codyoss commented Jul 14, 2021

@georgiyekkert Thanks for working on these!

@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 15, 2021
@codyoss
Copy link
Member

codyoss commented Jul 15, 2021

@georgiyekkert Please gofmt the file to make CI happy.

@georgiyekkert
Copy link
Contributor Author

georgiyekkert commented Jul 15, 2021

@codyoss I gofmt'ed files but CI still fails. I was running gofmt -w . command

@codyoss
Copy link
Member

codyoss commented Jul 15, 2021

@georgiyekkert It not looks like goimports is failing:

+ goimports -l .
+ tee /dev/stderr
+ read
compute/apiv1/smoke_test.go

You can run

go install golang.org/x/tools/cmd/goimports@latest
goimports -w .

@codyoss codyoss added the automerge Merge the pull request once unit tests and other checks pass. label Jul 15, 2021
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 16, 2021
@georgiyekkert
Copy link
Contributor Author

@codyoss could you rerun CI jobs please? looks like CLA job got stuck

@noahdietz noahdietz merged commit 64fff7a into googleapis:master Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: compute Issues related to the Compute Engine API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants