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.

Sign up for GitHub

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

Jump to bottom

Add Make rules and bash for Go tests #23837

Merged
merged 4 commits into from
Oct 11, 2021
Merged

Add Make rules and bash for Go tests #23837

merged 4 commits into from
Oct 11, 2021

Conversation

chaodaiG
Copy link
Contributor

@chaodaiG chaodaiG commented Sep 29, 2021 •
edited
Loading

Fixes: #23828

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Sep 29, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2021
@chaodaiG
Copy link
Contributor Author

/uncc @fejta @mpherman2

@chaodaiG
Copy link
Contributor Author

/test all

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG chaodaiG force-pushed the go-make-rule branch 2 times, most recently from 530849f to 7a2cfe5 Compare September 30, 2021 13:38
@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

1 similar comment
@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

1 similar comment
@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

@k8s-ci-robot k8s-ci-robot added the area/config Issues or PRs related to code in /config label Sep 30, 2021
spiffxp
spiffxp suggested changes Oct 5, 2021
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

I am FWIW seeing the same thing as bazel, though I can't quite figure why, it looks like you're doing the right thing

$ make test
hack/make-rules/go-test/all.sh
+ mkdir -p /Users/spiffxp/w/kubernetes/test-infra/_output
+ /Users/spiffxp/w/kubernetes/test-infra/_bin/gotestsum --junitfile=/Users/spiffxp/w/kubernetes/test-infra/_output/junit-unit.xml -- ./...
hack/tools.go:22:2: import "github.com/client9/misspell/cmd/misspell" is a program, not an importable package
hack/tools.go:24:2: import "k8s.io/code-generator/cmd/client-gen" is a program, not an importable package
hack/tools.go:25:2: import "k8s.io/code-generator/cmd/deepcopy-gen" is a program, not an importable package
hack/tools.go:26:2: import "k8s.io/code-generator/cmd/informer-gen" is a program, not an importable package
hack/tools.go:27:2: import "k8s.io/code-generator/cmd/lister-gen" is a program, not an importable package
hack/tools.go:28:2: import "sigs.k8s.io/controller-tools/cmd/controller-gen" is a program, not an importable package

hack/build/setup-go.sh Outdated Show resolved Hide resolved
hack/make-rules/go-test/all.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@chaodaiG chaodaiG force-pushed the go-make-rule branch 4 times, most recently from 27417d9 to c97cee5 Compare October 5, 2021 19:05
@chaodaiG
Copy link
Contributor Author

chaodaiG commented Oct 5, 2021

/test pull-test-infra-test-beta

@chaodaiG
Copy link
Contributor Author

chaodaiG commented Oct 5, 2021

/test pull-test-infra-test-beta

@BenTheElder
Copy link
Member

BenTheElder commented Oct 5, 2021 •
edited
Loading

I think the issue is hack/tools.go still exists with underscore imports, but it should be migrated to hack/tools/tools.go? (which is now a new file with a subset)

spiffxp
spiffxp approved these changes Oct 8, 2021
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm
/hold
Wasn't sure if you were looking for signoff from anyone else. Remove hold when ready to merge

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 8, 2021
alvaroaleman
alvaroaleman reviewed Oct 8, 2021
View reviewed changes
.go-version Outdated Show resolved Hide resolved
hack/build/setup-go.sh Show resolved Hide resolved
@chaodaiG @spiffxp
Apply suggestions from code review
4113848 
Co-authored-by: Aaron Crickenberger <spiffxp@google.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2021
@chaodaiG
Copy link
Contributor Author

/test pull-test-infra-test-beta

alvaroaleman
alvaroaleman approved these changes Oct 11, 2021
View reviewed changes
Copy link
Member

@alvaroaleman alvaroaleman left a comment

Choose a reason for hiding this comment

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

/label tide/merge-method-squash
thanks!

@k8s-ci-robot k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 11, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, chaodaiG, spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@chaodaiG
Copy link
Contributor Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 11, 2021
@k8s-ci-robot k8s-ci-robot merged commit c6a5018 into kubernetes:master Oct 11, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Oct 11, 2021
@chaodaiG chaodaiG mentioned this pull request Oct 11, 2021
Merged
@chaodaiG chaodaiG deleted the go-make-rule branch October 12, 2021 16:04
@palnabarun palnabarun mentioned this pull request Nov 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvaroaleman alvaroaleman alvaroaleman approved these changes

@spiffxp spiffxp spiffxp approved these changes

@BenTheElder BenTheElder Awaiting requested review from BenTheElder

Assignees

@spiffxp spiffxp

@cjwagner cjwagner

@alvaroaleman alvaroaleman

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/config Issues or PRs related to code in /config area/prow/bazel-deprecation area/testgrid cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

Build and test Go without bazel
6 participants
@chaodaiG @k8s-ci-robot @BenTheElder @spiffxp @alvaroaleman @cjwagner