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

Add verify-gofmt as a Bazel test. #41280

Merged
merged 1 commit into from Feb 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions .bazelrc
Expand Up @@ -2,8 +2,5 @@
build --verbose_failures
test --test_output=errors

# Retry tests up to 3 times if they fail.
test --flaky_test_attempts=3

# Include git version info
build --workspace_status_command hack/print-workspace-status.sh
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -500,7 +500,7 @@ endef
@echo "$$BAZEL_TEST_HELP_INFO"
else
bazel-test:
bazel test //cmd/... //pkg/... //federation/... //plugin/... //third_party/... //hack/... //hack:verify-all
bazel test --flaky_test_attempts=3 //cmd/... //pkg/... //federation/... //plugin/... //third_party/... //hack/... //hack:verify-all
endif

ifeq ($(PRINT_HELP),y)
Expand Down
11 changes: 11 additions & 0 deletions hack/BUILD
Expand Up @@ -27,10 +27,21 @@ sh_test(
tags = ["manual"],
)

sh_test(
name = "verify-gofmt",
srcs = ["verify-gofmt.sh"],
data = [
"//:all-srcs",
"@io_bazel_rules_go_toolchain//:toolchain",
],
tags = ["manual"],
)

test_suite(
name = "verify-all",
tags = ["manual"],
tests = [
"verify-boilerplate",
"verify-gofmt",
],
)
18 changes: 11 additions & 7 deletions hack/verify-gofmt.sh
Expand Up @@ -23,10 +23,15 @@ set -o pipefail
KUBE_ROOT=$(dirname "${BASH_SOURCE}")/..
source "${KUBE_ROOT}/hack/lib/init.sh"

kube::golang::verify_go_version

cd "${KUBE_ROOT}"

# Prefer bazel's gofmt.
gofmt="external/io_bazel_rules_go_toolchain/bin/gofmt"
Copy link
Member

Choose a reason for hiding this comment

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

this makes me a little sad, but I'm not sure if there's a better way (besides adding this to the PATH?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think rules_go can expose this more nicely along with the raw go tool, but until that happens this is about as good as we're going to do.

if [[ ! -x "${gofmt}" ]]; then
gofmt=$(which gofmt)
kube::golang::verify_go_version
fi

find_files() {
find . -not \( \
\( \
Expand All @@ -38,14 +43,13 @@ find_files() {
-o -wholename '*/third_party/*' \
-o -wholename '*/vendor/*' \
-o -wholename './staging' \
-o -wholename '*/bindata.go' \
Copy link
Member

Choose a reason for hiding this comment

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

why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're actually generating test/e2e/generated/bindata.go, and I guess it needs gofmt, as in here...

\) -prune \
\) -name '*.go'
}

GOFMT="gofmt -s -w"
bad_files=$(find_files | xargs $GOFMT -l)
if [[ -n "${bad_files}" ]]; then
echo "!!! '$GOFMT' needs to be run on the following files: "
echo "${bad_files}"
diff=$(find_files | xargs ${gofmt} -d -s 2>&1)
if [[ -n "${diff}" ]]; then
echo "${diff}"
exit 1
fi