chore: enable strict mode for test CI

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2023-03-25 16:48:33 +08:00 · 2023-03-25 16:48:33 +08:00 · 55bce22e97
commit 55bce22e97
parent 7230b943d0
13 changed files with 77 additions and 52 deletions
--- a/.github/workflows/e2e.yaml
+++ b/.github/workflows/e2e.yaml
@ -18,15 +18,15 @@ jobs:
    - env:
        TARGET: ${{ matrix.target }}
      run: |
+        set -euo pipefail
+
        echo "${TARGET}"
        case "${TARGET}" in
          linux-amd64-e2e)
-            PASSES='build release e2e' MANUAL_VER=v3.4.7 CPU='4' EXPECT_DEBUG='true' COVER='false' RACE='true' ./test.sh 2>&1 | tee test.log
-            ! egrep "(--- FAIL:|DATA RACE|panic: test timed out|appears to have leaked)" -B50 -A10 test.log
+            PASSES='build release e2e' MANUAL_VER=v3.4.7 CPU='4' EXPECT_DEBUG='true' COVER='false' RACE='true' ./test.sh
            ;;
          linux-386-e2e)
-            GOARCH=386 PASSES='build e2e' CPU='4' EXPECT_DEBUG='true' COVER='false' RACE='true' ./test.sh 2>&1 | tee test.log
-            ! egrep "(--- FAIL:|DATA RACE|panic: test timed out|appears to have leaked)" -B50 -A10 test.log
+            GOARCH=386 PASSES='build e2e' CPU='4' EXPECT_DEBUG='true' COVER='false' RACE='true' ./test.sh
            ;;
          *)
            echo "Failed to find target"
--- a/.github/workflows/functional.yaml
+++ b/.github/workflows/functional.yaml
@ -17,6 +17,8 @@ jobs:
    - env:
        TARGET: ${{ matrix.target }}
      run: |
+        set -euo pipefail
+
        echo "${TARGET}"
        case "${TARGET}" in
          linux-amd64-functional)
--- a/.github/workflows/grpcproxy.yaml
+++ b/.github/workflows/grpcproxy.yaml
@ -17,11 +17,12 @@ jobs:
    - env:
        TARGET: ${{ matrix.target }}
      run: |
+        set -euo pipefail
+
        echo "${TARGET}"
        case "${TARGET}" in
          linux-amd64-grpcproxy)
-            PASSES='build grpcproxy'  CPU='4' COVER='false' RACE='true' ./test.sh 2>&1 | tee test.log
-            ! egrep "(--- FAIL:|DATA RACE|panic: test timed out|appears to have leaked)" -B50 -A10 test.log
+            PASSES='build grpcproxy'  CPU='4' COVER='false' RACE='true' ./test.sh
            ;;
          *)
            echo "Failed to find target"
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@ -23,6 +23,8 @@ jobs:
    - env:
        TARGET: ${{ matrix.target }}
      run: |
+        set -euo pipefail
+
        echo "${TARGET}"
        case "${TARGET}" in
          linux-amd64-fmt)
--- a/2
+++ b/2
@ -1,5 +1,7 @@
 #!/usr/bin/env bash

+set -euo pipefail
+
 echo -e "\\e[91mDEPRECATED!!! Use build.sh script instead.\\e[0m\\n"
 sleep 1

--- a/build.sh
+++ b/build.sh
@ -1,18 +1,24 @@
 #!/usr/bin/env bash

+set -euo pipefail
+
 source ./scripts/test_lib.sh

 GIT_SHA=$(git rev-parse --short HEAD || echo "GitNotFound")
-if [[ -n "$FAILPOINTS" ]]; then
+if [[ -n "${FAILPOINTS:-}" ]]; then
  GIT_SHA="$GIT_SHA"-FAILPOINTS
 fi

 VERSION_SYMBOL="${ROOT_MODULE}/api/v3/version.GitSHA"

+# use go env if noset
+GOOS=${GOOS:-$(go env GOOS)}
+GOARCH=${GOARCH:-$(go env GOARCH)}
+
 # Set GO_LDFLAGS="-s" for building without symbols for debugging.
 # shellcheck disable=SC2206
-GO_LDFLAGS=(${GO_LDFLAGS} "-X=${VERSION_SYMBOL}=${GIT_SHA}")
-GO_BUILD_ENV=("CGO_ENABLED=0" "GO_BUILD_FLAGS=${GO_BUILD_FLAGS}" "GOOS=${GOOS}" "GOARCH=${GOARCH}")
+GO_LDFLAGS=(${GO_LDFLAGS:-} "-X=${VERSION_SYMBOL}=${GIT_SHA}")
+GO_BUILD_ENV=("CGO_ENABLED=0" "GO_BUILD_FLAGS=${GO_BUILD_FLAGS:-}" "GOOS=${GOOS}" "GOARCH=${GOARCH}")

 # enable/disable failpoints
 toggle_failpoints() {
@ -27,13 +33,13 @@ toggle_failpoints() {

 toggle_failpoints_default() {
  mode="disable"
-  if [[ -n "$FAILPOINTS" ]]; then mode="enable"; fi
+  if [[ -n "${FAILPOINTS:-}" ]]; then mode="enable"; fi
  toggle_failpoints "$mode"
 }

 etcd_build() {
  out="bin"
-  if [[ -n "${BINDIR}" ]]; then out="${BINDIR}"; fi
+  if [[ -n "${BINDIR:-}" ]]; then out="${BINDIR}"; fi
  toggle_failpoints_default

  run rm -f "${out}/etcd"
@ -41,7 +47,7 @@ etcd_build() {
    cd ./server
    # Static compilation is useful when etcd is run in a container. $GO_BUILD_FLAGS is OK
    # shellcheck disable=SC2086
-    run env "${GO_BUILD_ENV[@]}" go build $GO_BUILD_FLAGS \
+    run env "${GO_BUILD_ENV[@]}" go build ${GO_BUILD_FLAGS:-} \
      -trimpath \
      -installsuffix=cgo \
      "-ldflags=${GO_LDFLAGS[*]}" \
@ -52,7 +58,7 @@ etcd_build() {
  # shellcheck disable=SC2086
  (
    cd ./etcdutl
-    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS}" "${GO_BUILD_ENV[@]}" go build $GO_BUILD_FLAGS \
+    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS:-}" "${GO_BUILD_ENV[@]}" go build ${GO_BUILD_FLAGS:-} \
      -trimpath \
      -installsuffix=cgo \
      "-ldflags=${GO_LDFLAGS[*]}" \
@ -63,7 +69,7 @@ etcd_build() {
  # shellcheck disable=SC2086
  (
    cd ./etcdctl
-    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS}" "${GO_BUILD_ENV[@]}" go build $GO_BUILD_FLAGS \
+    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS:-}" "${GO_BUILD_ENV[@]}" go build ${GO_BUILD_FLAGS:-} \
      -trimpath \
      -installsuffix=cgo \
      "-ldflags=${GO_LDFLAGS[*]}" \
@ -84,7 +90,7 @@ etcd_build() {

 tools_build() {
  out="bin"
-  if [[ -n "${BINDIR}" ]]; then out="${BINDIR}"; fi
+  if [[ -n "${BINDIR:-}" ]]; then out="${BINDIR}"; fi
  tools_path="tools/benchmark
    tools/etcd-dump-db
    tools/etcd-dump-logs
@ -94,7 +100,7 @@ tools_build() {
    echo "Building" "'${tool}'"...
    run rm -f "${out}/${tool}"
    # shellcheck disable=SC2086
-    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS}" CGO_ENABLED=0 go build ${GO_BUILD_FLAGS} \
+    run env GO_BUILD_FLAGS="${GO_BUILD_FLAGS:-}" CGO_ENABLED=0 go build ${GO_BUILD_FLAGS:-} \
      -trimpath \
      -installsuffix=cgo \
      "-ldflags=${GO_LDFLAGS[*]}" \
@ -105,7 +111,7 @@ tools_build() {

 tests_build() {
  out="bin"
-  if [[ -n "${BINDIR}" ]]; then out="${BINDIR}"; fi
+  if [[ -n "${BINDIR:-}" ]]; then out="${BINDIR}"; fi
  tools_path="
    functional/cmd/etcd-agent
    functional/cmd/etcd-proxy
@ -118,7 +124,7 @@ tests_build() {
      run rm -f "../${out}/${tool}"

      # shellcheck disable=SC2086
-      run env CGO_ENABLED=0 GO_BUILD_FLAGS="${GO_BUILD_FLAGS}" go build ${GO_BUILD_FLAGS} \
+      run env CGO_ENABLED=0 GO_BUILD_FLAGS="${GO_BUILD_FLAGS:-}" go build ${GO_BUILD_FLAGS:-} \
        -installsuffix=cgo \
        "-ldflags=${GO_LDFLAGS[*]}" \
        -o="../${out}/${tool}" "./${tool}" || return 2
--- a/scripts/build-binary
+++ b/scripts/build-binary
@ -1,18 +1,17 @@
 #!/usr/bin/env bash

-set -e
+set -euo pipefail

 source ./scripts/test_lib.sh

-VER=$1
+VER=${1:-}
 REPOSITORY="${REPOSITORY:-git@github.com:etcd-io/etcd.git}"

-if [ -z "$1" ]; then
+if [ -z "${VER}" ]; then
  echo "Usage: ${0} VERSION" >> /dev/stderr
  exit 255
 fi

-set -u

 function setup_env {
  local ver=${1}
@ -38,7 +37,7 @@ function package {
    srcdir="${ccdir}"
  fi
  local ext=""
-  if [ "${GOOS}" == "windows" ]; then
+  if [ "${GOOS:-}" == "windows" ]; then
    ext=".exe"
  fi
  for bin in etcd etcdctl etcdutl; do
@ -60,7 +59,7 @@ function main {
  cd release
  setup_env "${VER}" "${proj}"

-  tarcmd=tar
+  local tarcmd=tar
  if [[ $(go env GOOS) == "darwin" ]]; then
      echo "Please use linux machine for release builds."
    exit 1
--- a/scripts/build-docker
+++ b/scripts/build-docker
@ -1,6 +1,6 @@
 #!/usr/bin/env bash

-set -e
+set -euo pipefail

 if [ "$#" -ne 1 ]; then
  echo "Usage: $0 VERSION" >&2
@ -11,7 +11,7 @@ ARCH=$(go env GOARCH)
 VERSION="${1}-${ARCH}"
 DOCKERFILE="Dockerfile-release.${ARCH}"

-if [ -z "${BINARYDIR}" ]; then
+if [ -z "${BINARYDIR:-}" ]; then
  RELEASE="etcd-${1}"-$(go env GOOS)-$(go env GOARCH)
  BINARYDIR="${RELEASE}"
  TARFILE="${RELEASE}.tar.gz"
@ -34,7 +34,7 @@ cp "${BINARYDIR}"/etcd "${BINARYDIR}"/etcdctl "${BINARYDIR}"/etcdutl "${IMAGEDIR

 cat ./"${DOCKERFILE}" > "${IMAGEDIR}"/Dockerfile

-if [ -z "$TAG" ]; then
+if [ -z "${TAG:-}" ]; then
    # Fix incorrect image "Architecture" using buildkit
    # From https://stackoverflow.com/q/72144329/
    DOCKER_BUILDKIT=1 docker build -t "gcr.io/etcd-development/etcd:${VERSION}" "${IMAGEDIR}"
--- a/scripts/build-release.sh
+++ b/scripts/build-release.sh
@ -3,11 +3,11 @@
 # Build all release binaries and images to directory ./release.
 # Run from repository root.
 #
-set -e
+set -euo pipefail

 source ./scripts/test_lib.sh

-VERSION=$1
+VERSION=${1:-}
 if [ -z "${VERSION}" ]; then
  echo "Usage: ${0} VERSION" >> /dev/stderr
  exit 255
--- a/scripts/test_lib.sh
+++ b/scripts/test_lib.sh
@ -1,5 +1,7 @@
 #!/usr/bin/env bash

+set -euo pipefail
+
 ROOT_MODULE="go.etcd.io/etcd"

 if [[ "$(go list)" != "${ROOT_MODULE}/v3" ]]; then
@ -107,7 +109,7 @@ function relativePath {

 ####   Discovery of files/packages within a go module #####

-# go_srcs_in_module [package]
+# go_srcs_in_module
 # returns list of all not-generated go sources in the current (dir) module.
 function go_srcs_in_module {
  go list -f "{{with \$c:=.}}{{range \$f:=\$c.GoFiles  }}{{\$c.Dir}}/{{\$f}}{{\"\n\"}}{{end}}{{range \$f:=\$c.TestGoFiles  }}{{\$c.Dir}}/{{\$f}}{{\"\n\"}}{{end}}{{range \$f:=\$c.XTestGoFiles  }}{{\$c.Dir}}/{{\$f}}{{\"\n\"}}{{end}}{{end}}" ./... | grep -vE "(\\.pb\\.go|\\.pb\\.gw.go)"
@ -171,7 +173,7 @@ function module_dirs() {

 # maybe_run [cmd...] runs given command depending on the DRY_RUN flag.
 function maybe_run() {
-  if ${DRY_RUN}; then
+  if ${DRY_RUN:-}; then
    log_warning -e "# DRY_RUN:\\n  % ${*}"
  else
    run "${@}"
@ -243,7 +245,7 @@ function go_test {

  local goTestFlags=""
  local goTestEnv=""
-  if [ "${VERBOSE}" == "1" ]; then
+  if [ "${VERBOSE:-}" == "1" ]; then
    goTestFlags="-v"
  fi

--- a/2
+++ b/2
@ -1,5 +1,7 @@
 #!/usr/bin/env bash

+set -euo pipefail
+
 echo -e "\\e[91mDEPRECATED!!! Use test.sh script instead.\\e[0m\\n"
 sleep 1

--- a/test.sh
+++ b/test.sh
@ -32,12 +32,12 @@
 # $ COVERDIR=coverage PASSES="build build_cov cov" ./test
 # $ go tool cover -html ./coverage/cover.out
 set -e
-set -o pipefail


 # Consider command as failed when any component of the pipe fails:
 # https://stackoverflow.com/questions/1221833/pipe-output-and-capture-exit-status-in-bash
 set -o pipefail
+set -o nounset

 # The test script is not supposed to make any changes to the files
 # e.g. add/update missing dependencies. Such divergences should be 
@ -51,12 +51,12 @@ source ./build.sh
 PASSES=${PASSES:-"fmt bom dep build unit"}
 PKG=${PKG:-}

-if [ -z "$GOARCH" ]; then
+if [ -z "${GOARCH:-}" ]; then
  GOARCH=$(go env GOARCH);
 fi

 # determine whether target supports race detection
-if [ -z "${RACE}" ] ; then
+if [ -z "${RACE:-}" ] ; then
  if [ "$GOARCH" == "amd64" ]; then
    RACE="--race"
  else
@ -68,21 +68,23 @@ fi

 # This options make sense for cases where SUT (System Under Test) is compiled by test.
 COMMON_TEST_FLAGS=("${RACE}")
-if [[ -n "${CPU}" ]]; then
+if [[ -n "${CPU:-}" ]]; then
  COMMON_TEST_FLAGS+=("--cpu=${CPU}")
 fi 

 log_callout "Running with ${COMMON_TEST_FLAGS[*]}"

 RUN_ARG=()
-if [ -n "${TESTCASE}" ]; then
+if [ -n "${TESTCASE:-}" ]; then
  RUN_ARG=("-run=${TESTCASE}")
 fi

 function build_pass {
  log_callout "Building etcd"
  run_for_modules run go build "${@}" || return 2
-  GO_BUILD_FLAGS="-v" etcd_build "${@}"
+  # Previous command will cover etcd_build so that any error will fast-fail by
+  # return 2. Just in case that add return 2 for etcd_build as well.
+  GO_BUILD_FLAGS="-v" etcd_build "${@}" || return 2
  GO_BUILD_FLAGS="-v" tools_build "${@}"
 }

@ -143,12 +145,13 @@ function generic_checker {
 }

 function functional_pass {
-  run ./tests/functional/build
+  run ./tests/functional/build || return $?

  # Clean up any data and logs from previous runs
  rm -rf /tmp/etcd-functional-* /tmp/etcd-functional-*.backup

  # TODO: These ports should be dynamically allocated instead of hard-coded.
+  local agent_pids=""
  for a in 1 2 3; do
    ./bin/etcd-agent --network tcp --address 127.0.0.1:${a}9027 < /dev/null &
    pid="$!"
@ -199,8 +202,8 @@ function grpcproxy_pass {

 # Builds artifacts used by tests/e2e in coverage mode.
 function build_cov_pass {
-  run_for_module "server" run go test -tags cov -c -covermode=set -coverpkg="./..." -o "../bin/etcd_test"
-  run_for_module "etcdctl" run go test -tags cov -c -covermode=set -coverpkg="./..." -o "../bin/etcdctl_test"
+  run_for_module "server" run go test -tags cov -c -covermode=set -coverpkg="./..." -o "../bin/etcd_test" || return $?
+  run_for_module "etcdctl" run go test -tags cov -c -covermode=set -coverpkg="./..." -o "../bin/etcdctl_test" || return $?
  run_for_module "etcdutl" run go test -tags cov -c -covermode=set -coverpkg="./..." -o "../bin/etcdutl_test"
 }

@ -285,7 +288,7 @@ function merge_cov {

 function cov_pass {
  # shellcheck disable=SC2153
-  if [ -z "$COVERDIR" ]; then
+  if [ -z "${COVERDIR:-}" ]; then
    log_error "COVERDIR undeclared"
    return 255
  fi
@ -436,9 +439,10 @@ function govet_pass {
  run_for_modules generic_checker run go vet
 }

+# TODO: should skip *.pb.go
 function govet_shadow_pass {
  local shadow
-  shadow=$(tool_get_bin "golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow")
+  shadow=$(tool_get_bin "golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow") || return $?
  run_for_modules generic_checker run go vet -all -vettool="${shadow}"
 }

@ -458,10 +462,11 @@ function unconvert_pass {
  run_for_modules generic_checker run_go_tool "github.com/mdempsky/unconvert" unconvert -v
 }

+# TODO: should run with package
 function ineffassign_per_package {
-  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module "$1")
+  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module)
  local gofiles=()
-  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module "$1")
+  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module)
  run_go_tool github.com/gordonklaus/ineffassign "${gofiles[@]}"
 }

@ -474,10 +479,11 @@ function nakedret_pass {
 }

 function license_header_pass {
-  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module "$1")
+  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module)
  local gofiles=()
-  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module "$1")
+  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module)
  
+  local licRes=""
  for file in "${gofiles[@]}"; do
    if ! head -n3 "${file}" | grep -Eq "(Copyright|generated|GENERATED)" ; then
      licRes="${licRes}"$(echo -e "  ${file}")
@ -490,9 +496,9 @@ function license_header_pass {
 }

 function receiver_name_for_package {
-  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module "$1")
+  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module)
  local gofiles=()
-  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module "$1")
+  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module)

  recvs=$(grep 'func ([^*]' "${gofiles[@]}"  | tr  ':' ' ' |  \
    awk ' { print $2" "$3" "$4" "$1 }' | sed "s/[a-zA-Z\\.]*go//g" |  sort  | uniq  | \
@ -516,9 +522,9 @@ function receiver_name_pass {
 # checks spelling and comments in the 'package' in the current module
 #
 function goword_for_package {
-  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module "$1")
+  # bash 3.x compatible replacement of: mapfile -t gofiles < <(go_srcs_in_module)
  local gofiles=()
-  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module "$1")
+  while IFS= read -r line; do gofiles+=("$line"); done < <(go_srcs_in_module)
  
  local gowordRes

@ -631,7 +637,7 @@ function release_pass {
  rm -f ./bin/etcd-last-release
  # to grab latest patch release; bump this up for every minor release
  UPGRADE_VER=$(git tag -l --sort=-version:refname "v3.4.*" | head -1)
-  if [ -n "$MANUAL_VER" ]; then
+  if [ -n "${MANUAL_VER:-}" ]; then
    # in case, we need to test against different version
    UPGRADE_VER=$MANUAL_VER
  fi
@ -675,6 +681,7 @@ function mod_tidy_for_module {
  local tmpFileGoModInSync
  diff -C 5 "${tmpModDir}/go.mod" "./go.mod"
  tmpFileGoModInSync="$?"
+  set -e

  # Bring back initial state
  mv "${tmpModDir}/go.mod" "./go.mod"
--- a/tests/functional/build
+++ b/tests/functional/build
@ -1,5 +1,7 @@
 #!/usr/bin/env bash

+set -euo pipefail
+
 if ! [[ "$0" =~ "tests/functional/build" ]]; then
  echo "must be run from repository root"
  exit 255