From a336ed61e5a4a93f9ec9a07d7c027b38149767fd Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Mon, 11 Jul 2022 08:12:33 -0300 Subject: [PATCH] Add gotestfmt to improve test output and fix flaky tests (#6528) --- .github/workflows/fleetctl-preview.yml | 1 + .github/workflows/test-go.yaml | 18 +++++++++++++++++- Makefile | 25 +++++++++++++++---------- cmd/fleetctl/preview_test.go | 16 +++++++++++++--- pkg/nettest/nettest.go | 5 ++++- 5 files changed, 50 insertions(+), 15 deletions(-) diff --git a/.github/workflows/fleetctl-preview.yml b/.github/workflows/fleetctl-preview.yml index 3fd89dce1..b6a5676c8 100644 --- a/.github/workflows/fleetctl-preview.yml +++ b/.github/workflows/fleetctl-preview.yml @@ -68,6 +68,7 @@ jobs: SLACK_WEBHOOK_TYPE: INCOMING_WEBHOOK - name: Get fleet logs + if: always() run: | FLEET_LICENSE_KEY=foo docker compose -f ~/.fleet/preview/docker-compose.yml logs fleet01 fleet02 > fleet-logs.txt # Copying logs, otherwise the upload-artifact action uploads the logs in a hidden folder (.fleet) diff --git a/.github/workflows/test-go.yaml b/.github/workflows/test-go.yaml index d8a3cc06a..b18dd02bf 100644 --- a/.github/workflows/test-go.yaml +++ b/.github/workflows/test-go.yaml @@ -36,6 +36,9 @@ jobs: with: go-version: ${{ matrix.go-version }} + - name: Set up gotestfmt + run: go install github.com/haveyoudebuggedit/gotestfmt/v2/cmd/gotestfmt@v2.3.2 + - name: Checkout Code uses: actions/checkout@629c2de402a417ea7690ca6ce3f33229e27606a5 # v2 @@ -61,7 +64,12 @@ jobs: - name: Run Go Tests run: | - TEST_LOCK_FILE_PATH=$(pwd)/lock NETWORK_TEST=1 REDIS_TEST=1 MYSQL_TEST=1 RACE_ENABLED=$RACE_ENABLED GO_TEST_TIMEOUT=$GO_TEST_TIMEOUT make test-go + GO_TEST_EXTRA_FLAGS="-v -json -race=$RACE_ENABLED -timeout=$GO_TEST_TIMEOUT" \ + TEST_LOCK_FILE_PATH=$(pwd)/lock \ + NETWORK_TEST=1 \ + REDIS_TEST=1 \ + MYSQL_TEST=1 \ + make test-go 2>&1 | tee /tmp/gotest.log | gotestfmt - name: Upload to Codecov uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 @@ -88,3 +96,11 @@ jobs: env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_G_PLATFORM_WEBHOOK_URL }} SLACK_WEBHOOK_TYPE: INCOMING_WEBHOOK + + - name: Upload test log + if: always() + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v2 + with: + name: test-log + path: /tmp/gotest.log + if-no-files-found: error diff --git a/Makefile b/Makefile index c14615c0b..080256109 100644 --- a/Makefile +++ b/Makefile @@ -10,16 +10,16 @@ REVSHORT = $(shell git rev-parse --short HEAD) USER = $(shell whoami) DOCKER_IMAGE_NAME = fleetdm/fleet -ifdef RACE_ENABLED -RACE_ENABLED_VAR := $(RACE_ENABLED) +ifdef GO_TEST_EXTRA_FLAGS +GO_TEST_EXTRA_FLAGS_VAR := $(GO_TEST_EXTRA_FLAGS) else -RACE_ENABLED_VAR := false +GO_TEST_EXTRA_FLAGS_VAR := endif -ifdef GO_TEST_TIMEOUT -GO_TEST_TIMEOUT_VAR := $(GO_TEST_TIMEOUT) +ifdef GO_BUILD_RACE_ENABLED +GO_BUILD_RACE_ENABLED_VAR := true else -GO_TEST_TIMEOUT_VAR := 10m +GO_BUILD_RACE_ENABLED_VAR := false endif ifneq ($(OS), Windows_NT) @@ -113,13 +113,18 @@ help: build: fleet fleetctl fleet: .prefix .pre-build .pre-fleet - CGO_ENABLED=1 go build -race=${RACE_ENABLED_VAR} -tags full,fts5,netgo -o build/${OUTPUT} -ldflags ${KIT_VERSION} ./cmd/fleet + CGO_ENABLED=1 go build -race=${GO_BUILD_RACE_ENABLED_VAR} -tags full,fts5,netgo -o build/${OUTPUT} -ldflags ${KIT_VERSION} ./cmd/fleet -fleet-dev: RACE_ENABLED_VAR=true +fleet-dev: GO_BUILD_RACE_ENABLED_VAR=true fleet-dev: fleet fleetctl: .prefix .pre-build .pre-fleetctl - CGO_ENABLED=0 go build -o build/fleetctl -ldflags ${KIT_VERSION} ./cmd/fleetctl + # Race requires cgo + $(eval CGO_ENABLED := $(shell [[ "${GO_BUILD_RACE_ENABLED_VAR}" = "true" ]] && echo 1 || echo 0)) + CGO_ENABLED=${CGO_ENABLED} go build -race=${GO_BUILD_RACE_ENABLED_VAR} -o build/fleetctl -ldflags ${KIT_VERSION} ./cmd/fleetctl + +fleetctl-dev: GO_BUILD_RACE_ENABLED_VAR=true +fleetctl-dev: fleetctl lint-js: yarn lint @@ -133,7 +138,7 @@ dump-test-schema: go run ./tools/dbutils ./server/datastore/mysql/schema.sql test-go: dump-test-schema generate-mock - go test -tags full,fts5,netgo -timeout=${GO_TEST_TIMEOUT_VAR} -race=${RACE_ENABLED_VAR} -parallel 8 -coverprofile=coverage.txt -covermode=atomic ./cmd/... ./ee/... ./orbit/pkg/... ./orbit/cmd/orbit ./pkg/... ./server/... ./tools/... + go test -tags full,fts5,netgo ${GO_TEST_EXTRA_FLAGS_VAR} -parallel 8 -coverprofile=coverage.txt -covermode=atomic ./cmd/... ./ee/... ./orbit/pkg/... ./orbit/cmd/orbit ./pkg/... ./server/... ./tools/... analyze-go: go test -tags full,fts5,netgo -race -cover ./... diff --git a/cmd/fleetctl/preview_test.go b/cmd/fleetctl/preview_test.go index 92b5b0abb..a1881c846 100644 --- a/cmd/fleetctl/preview_test.go +++ b/cmd/fleetctl/preview_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "os" "path/filepath" "regexp" @@ -23,12 +24,21 @@ func TestPreview(t *testing.T) { require.Equal(t, "", runAppForTest(t, []string{"preview", "--config", configPath, "stop"})) }) - output := runAppForTest(t, []string{"preview", "--config", configPath, "--tag", "main", "--disable-open-browser"}) + var output *bytes.Buffer + nettest.RunWithNetRetry(t, func() error { + var err error + output, err = runAppNoChecks([]string{ + "preview", "--config", configPath, + "--tag", "main", + "--disable-open-browser", + }) + return err + }) queriesRe := regexp.MustCompile(`applied ([0-9]+) queries`) policiesRe := regexp.MustCompile(`applied ([0-9]+) policies`) - require.True(t, queriesRe.MatchString(output)) - require.True(t, policiesRe.MatchString(output)) + require.True(t, queriesRe.MatchString(output.String())) + require.True(t, policiesRe.MatchString(output.String())) // run some sanity checks on the preview environment diff --git a/pkg/nettest/nettest.go b/pkg/nettest/nettest.go index 875faf935..00c6dc46b 100644 --- a/pkg/nettest/nettest.go +++ b/pkg/nettest/nettest.go @@ -62,8 +62,11 @@ func Retryable(err error) bool { } // Using the exact same error check used in: // https://github.com/golang/go/blob/a5d61be040ed20b5774bff1b6b578c6d393ab332/src/net/http/serve_test.go#L1417 + // + // Also we use raw string matching because this method is used on raw error strings (returned by commands). if errStr := err.Error(); (strings.Contains(errStr, "timeout") && strings.Contains(errStr, "TLS handshake")) || - strings.Contains(errStr, "unexpected EOF") || strings.Contains(errStr, "connection reset by peer") || strings.Contains(errStr, "unexpected http response") { + strings.Contains(errStr, "unexpected EOF") || strings.Contains(errStr, "connection reset by peer") || + strings.Contains(errStr, "unexpected http response") || strings.Contains(errStr, "context deadline exceeded") { return true } return false