Upload code coverage

temporalio · Feb 12, 2025 · e3b0dcf · e3b0dcf
1 parent e0ef18f
commit e3b0dcf
Show file tree

Hide file tree

Showing 4 changed files with 205 additions and 127 deletions.
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -60,7 +60,6 @@ env:
   PR_BASE_COMMIT: ${{ github.event.pull_request.base.sha }}
   DOCKER_COMPOSE_FILE: ./develop/github/docker-compose.yml
   TEMPORAL_VERSION_CHECK_DISABLED: 1
-  BUILDKITE_ANALYTICS_TOKEN: ${{ secrets.BUILDKITE_ANALYTICS_TOKEN }}
 
 jobs:
   set-up-single-test:
@@ -232,8 +231,6 @@ jobs:
     strategy:
       fail-fast: false
     runs-on: ubuntu-20.04
-    env:
-      BUILDKITE_MESSAGE: '{"job": "unit-test"}'
     steps:
       - uses: actions/checkout@v4
         with:
@@ -275,6 +272,15 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/summary.cover.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          parallel: true
+          fail-on-error: false  # don't want to fail the run if coveralls has an issue
+          file: ./.testoutput/summary.cover.out
+
       - name: Upload test results
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/[email protected]
@@ -305,8 +311,6 @@ jobs:
     strategy:
       fail-fast: false
     runs-on: ubuntu-20.04
-    env:
-      BUILDKITE_MESSAGE: '{"job": "integration-test"}'
     steps:
       - uses: actions/checkout@v4
         with:
@@ -354,6 +358,15 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/summary.cover.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          parallel: true
+          fail-on-error: false  # don't want to fail the run if coveralls has an issue
+          file: ./.testoutput/summary.cover.out
+
       - name: Upload test results
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/[email protected]
@@ -431,7 +444,6 @@ jobs:
       PERSISTENCE_TYPE: ${{ matrix.persistence_type }}
       PERSISTENCE_DRIVER: ${{ matrix.persistence_driver }}
       TEST_TIMEOUT: ${{ needs.set-up-single-test.outputs.test_timeout }}
-      BUILDKITE_MESSAGE: '{"job": "functional-test", "db": "${{ matrix.persistence_driver }}"}'
     steps:
       - uses: ScribeMD/[email protected]
         if: ${{ inputs.run_single_functional_test != true || (inputs.run_single_functional_test == true && contains(fromJSON(needs.set-up-single-test.outputs.dbs), env.PERSISTENCE_DRIVER)) }}
@@ -504,6 +516,15 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/summary.cover.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          parallel: true
+          fail-on-error: false  # don't want to fail the run if coveralls has an issue
+          file: ./.testoutput/summary.cover.out
+
       - name: Upload test results
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/[email protected]
@@ -564,7 +585,6 @@ jobs:
     env:
       PERSISTENCE_TYPE: ${{ matrix.persistence_type }}
       PERSISTENCE_DRIVER: ${{ matrix.persistence_driver }}
-      BUILDKITE_MESSAGE: '{"job": "functional-test-xdc", "db": "${{ matrix.persistence_driver }}"}'
       TEST_PARALLEL_FLAGS: ${{ matrix.parallel_flags }}
     steps:
       - uses: actions/checkout@v4
@@ -628,6 +648,15 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/summary.cover.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          parallel: true
+          fail-on-error: false  # don't want to fail the run if coveralls has an issue
+          file: ./.testoutput/summary.cover.out
+
       - name: Upload test results
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/[email protected]
@@ -743,6 +772,15 @@ jobs:
         timeout-minutes: 15
         run: make functional-test-ndc-coverage
 
+      - name: Generate code coverage
+        run: make ./.testoutput/summary.cover.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          parallel: true
+          fail-on-error: false  # don't want to fail the run if coveralls has an issue
+          file: ./.testoutput/summary.cover.out
+
       - name: Upload test results
         # Can't pin to major because the action linter doesn't recognize the include-hidden-files flag.
         uses: actions/[email protected]
@@ -779,6 +817,11 @@ jobs:
     env:
       RESULTS: ${{ toJSON(needs.*.result) }}
     steps:
+      - name: Complete code coverage uploads
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          parallel-finished: true
       - name: Check results
         run: |
           if [[ -n $(echo "$RESULTS" | jq '.[] | select (. != "success")') ]]; then

diff --git a/Makefile b/Makefile
@@ -123,19 +123,19 @@ TEST_OUTPUT_ROOT        := ./.testoutput
 NEW_COVER_PROFILE       = $(TEST_OUTPUT_ROOT)/$(shell xxd -p -l 16 /dev/urandom).cover.out   # generates a new filename each time it's substituted
 SUMMARY_COVER_PROFILE  := $(TEST_OUTPUT_ROOT)/summary.cover.out
 NEW_REPORT              = $(TEST_OUTPUT_ROOT)/$(shell xxd -p -l 16 /dev/urandom).junit.xml   # generates a new filename each time it's substituted
+COVERPKG 				= $(shell go list ./... | \
+							  grep -v /server/api/ | \
+							  grep -v /server/cmd/ | \
+							  grep -v /server/tests/ | \
+							  grep -v /server/tools/ | \
+							  grep -v /server/commong/testing/ | \
+							  paste -sd "," -) # exclude generated and test-only packages
+COVERPKG_FLAG 		    = -coverpkg=$(COVERPKG) # individual generated files are removed when generating summary
 
 # DB
 SQL_USER ?= temporal
 SQL_PASSWORD ?= temporal
 
-# Need the following option to have integration and functional tests count towards coverage. godoc below:
-# -coverpkg pkg1,pkg2,pkg3
-#   Apply coverage analysis in each test to the given list of packages.
-#   The default is for each test to analyze only the package being tested.
-#   Packages are specified as import paths.
-INTEGRATION_TEST_COVERPKG := -coverpkg="$(MODULE_ROOT)/common/persistence/..."
-FUNCTIONAL_TEST_COVERPKG := -coverpkg="$(MODULE_ROOT)/client/...,$(MODULE_ROOT)/common/...,$(MODULE_ROOT)/service/...,$(MODULE_ROOT)/temporal/..."
-
 # Only prints output if the exit code is non-zero
 define silent_exec
     @output=$$($(1) 2>&1); \
@@ -413,39 +413,34 @@ prepare-coverage-test: $(GOTESTSUM) $(TEST_OUTPUT_ROOT)
 
 unit-test-coverage: prepare-coverage-test
 	@printf $(COLOR) "Run unit tests with coverage..."
-	go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(UNIT_TEST_DIRS)" -- \
-		$(COMPILED_TEST_ARGS) \
-		-coverprofile=$(NEW_COVER_PROFILE)
+	go run ./cmd/tools/test-runner $(GOTESTSUM) --retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) -- \
+		$(COMPILED_TEST_ARGS) -coverprofile=$(NEW_COVER_PROFILE) $(UNIT_TEST_DIRS)
 
 integration-test-coverage: prepare-coverage-test
 	@printf $(COLOR) "Run integration tests with coverage..."
-	go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(INTEGRATION_TEST_DIRS)" -- \
-		$(COMPILED_TEST_ARGS) \
-		-coverprofile=$(NEW_COVER_PROFILE) $(INTEGRATION_TEST_COVERPKG)
+	go run ./cmd/tools/test-runner $(GOTESTSUM) --retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) -- \
+		$(COMPILED_TEST_ARGS) -coverprofile=$(NEW_COVER_PROFILE) $(INTEGRATION_TEST_DIRS)
 
 # This should use the same build flags as functional-test-coverage and functional-test-{xdc,ndc}-coverage for best build caching.
 pre-build-functional-test-coverage: prepare-coverage-test
-	go test -c -o /dev/null $(FUNCTIONAL_TEST_ROOT) $(TEST_ARGS) $(TEST_TAG_FLAG) $(FUNCTIONAL_TEST_COVERPKG)
+	go test -c -cover -o /dev/null $(FUNCTIONAL_TEST_ROOT) $(TEST_ARGS) $(TEST_TAG_FLAG) $(COVERPKG_FLAG)
 
 functional-test-coverage: prepare-coverage-test
 	@printf $(COLOR) "Run functional tests with coverage with $(PERSISTENCE_DRIVER) driver..."
-	go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_ROOT)" -- \
-		$(COMPILED_TEST_ARGS) \
-		-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
+	go run ./cmd/tools/test-runner $(GOTESTSUM) --retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) -- \
+		$(COMPILED_TEST_ARGS) -coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) $(FUNCTIONAL_TEST_ROOT) \
 		-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)
 
 functional-test-xdc-coverage: prepare-coverage-test
 	@printf $(COLOR) "Run functional test for cross DC with coverage with $(PERSISTENCE_DRIVER) driver..."
-	go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_XDC_ROOT)" -- \
-		$(COMPILED_TEST_ARGS) \
-		-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
+	go run ./cmd/tools/test-runner $(GOTESTSUM) --retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) -- \
+		$(COMPILED_TEST_ARGS) -coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) $(FUNCTIONAL_TEST_XDC_ROOT) \
 		-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)
 
 functional-test-ndc-coverage: prepare-coverage-test
 	@printf $(COLOR) "Run functional test for NDC with coverage with $(PERSISTENCE_DRIVER) driver..."
-	go run ./cmd/tools/test-runner $(GOTESTSUM) -retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) --packages="$(FUNCTIONAL_TEST_NDC_ROOT)" -- \
-		$(COMPILED_TEST_ARGS) \
-		-coverprofile=$(NEW_COVER_PROFILE) $(FUNCTIONAL_TEST_COVERPKG) \
+	go run ./cmd/tools/test-runner $(GOTESTSUM) --retries=$(FAILED_TEST_RETRIES) --junitfile=$(NEW_REPORT) -- \
+		$(COMPILED_TEST_ARGS) -coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) $(FUNCTIONAL_TEST_NDC_ROOT) \
 		-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)
 
 .PHONY: $(SUMMARY_COVER_PROFILE)
@@ -456,9 +451,14 @@ $(SUMMARY_COVER_PROFILE):
 		echo "No coverage data, aborting!" && exit 1; \
 	fi
 	@echo "mode: atomic" > $(SUMMARY_COVER_PROFILE)
-	$(foreach COVER_PROFILE,$(wildcard $(TEST_OUTPUT_ROOT)/*.cover.out),\
+	$(foreach COVER_PROFILE,$(filter-out $(SUMMARY_COVER_PROFILE),$(wildcard $(TEST_OUTPUT_ROOT)/*.cover.out)),\
 		@printf "Add %s...\n" $(COVER_PROFILE); \
-		@grep -v -e "[Mm]ocks\?.go" -e "^mode: \w\+" $(COVER_PROFILE) >> $(SUMMARY_COVER_PROFILE) || true \
+		grep -v \
+			-e "^mode: \w\+" \
+			-e "_gen.go" \
+			-e ".pb.go" \
+			-e "[Mm]ocks\?.go" \
+			$(COVER_PROFILE) >> $(SUMMARY_COVER_PROFILE) || true \
 	$(NEWLINE))
 
 coverage-report: $(SUMMARY_COVER_PROFILE)

diff --git a/tools/testrunner/testrunner.go b/tools/testrunner/testrunner.go
@@ -39,16 +39,31 @@ import (
 	"github.com/google/uuid"
 )
 
+const (
+	codeCoverageExtension = ".cover.out"
+	retriesFlag           = "--retries="
+	coverProfileFlag      = "-coverprofile="
+	junitReportFlag       = "--junitfile="
+)
+
 type attempt struct {
-	runner      *runner
-	number      int
-	exitErr     *exec.ExitError
-	junitReport *junitReport
+	runner           *runner
+	number           int
+	exitErr          *exec.ExitError
+	junitReport      *junitReport
+	coverProfilePath string
 }
 
 func (a *attempt) run(ctx context.Context, args []string) (string, error) {
-	args = append([]string{"--junitfile", a.junitReport.path}, args...)
-	log.Printf("starting test attempt %d with args: %v", a.number, args)
+	for i, arg := range args {
+		if strings.HasPrefix(arg, coverProfileFlag) {
+			args[i] = coverProfileFlag + a.coverProfilePath
+		} else if strings.HasPrefix(arg, junitReportFlag) {
+			args[i] = junitReportFlag + a.junitReport.path
+		}
+	}
+	log.Printf("starting test attempt #%d: %v %v",
+		a.number, a.runner.gotestsumExecutable, strings.Join(args, " "))
 	cmd := exec.CommandContext(ctx, a.runner.gotestsumExecutable, args...)
 	var output strings.Builder
 	cmd.Stdout = io.MultiWriter(os.Stdout, &output)
@@ -63,6 +78,7 @@ func (a *attempt) run(ctx context.Context, args []string) (string, error) {
 type runner struct {
 	gotestsumExecutable string
 	junitOutputPath     string
+	coverProfilePath    string
 	attempts            []*attempt
 	retries             int
 }
@@ -76,61 +92,36 @@ func newRunner(gotestsumExecutable string) *runner {
 
 func (r *runner) sanitizeAndParseArgs(args []string) ([]string, error) {
 	var sanitizedArgs []string
-	type action struct {
-		f   func(string) error
-		err string
-	}
-	var next *action
-	for i, arg := range args {
-		if next != nil {
-			if err := next.f(arg); err != nil {
-				return nil, err
-			}
-			next = nil
-			continue
-		} else if arg == "-retries" {
-			next = &action{
-				f: func(arg string) error {
-					var err error
-					r.retries, err = strconv.Atoi(arg)
-					return err
-				},
-				err: "got -retries flag with no value",
-			}
-			continue
-		} else if strings.HasPrefix(arg, "-retries=") {
+	for _, arg := range args {
+		if strings.HasPrefix(arg, retriesFlag) {
 			var err error
-			r.retries, err = strconv.Atoi(arg[len("-retries="):])
+			r.retries, err = strconv.Atoi(strings.Split(arg, "=")[1])
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("invalid argument %q: %w", retriesFlag, err)
 			}
-			continue
-		} else if arg == "--junitfile" {
-			// --junitfile is used by gotestsum
-			next = &action{
-				f: func(arg string) error {
-					r.junitOutputPath = arg
-					return nil
-				},
-				err: "got --junitfile flag with no value",
+			if r.retries == 0 {
+				return nil, fmt.Errorf("invalid argument %q: must be greater than zero", retriesFlag)
 			}
-			continue
-		} else if strings.HasPrefix(arg, "--junitfile=") {
+			continue // this is a `testrunner` only arg and not passed through
+		}
+
+		if strings.HasPrefix(arg, coverProfileFlag) {
+			r.coverProfilePath = strings.Split(arg, "=")[1]
+		} else if strings.HasPrefix(arg, junitReportFlag) {
 			// --junitfile is used by gotestsum
-			r.junitOutputPath = arg[len("--junitfile="):]
-			continue
-		} else if arg == "--" {
-			// Forward all arguments from -- on.
-			sanitizedArgs = append(sanitizedArgs, args[i:]...)
-			break
+			r.junitOutputPath = strings.Split(arg, "=")[1]
 		}
+
 		sanitizedArgs = append(sanitizedArgs, arg)
 	}
-	if next != nil {
-		return nil, fmt.Errorf("incomplete command line arguments: %s", next.err)
-	}
 	if r.junitOutputPath == "" {
-		return nil, fmt.Errorf("missing required argument --junitfile")
+		return nil, fmt.Errorf("missing required argument %q", junitReportFlag)
+	}
+	if r.coverProfilePath == "" {
+		return nil, fmt.Errorf("missing required argument %q", coverProfileFlag)
+	}
+	if r.retries == 0 {
+		return nil, fmt.Errorf("missing required argument %q", retriesFlag)
 	}
 	return sanitizedArgs, nil
 }
@@ -139,6 +130,11 @@ func (r *runner) newAttempt() *attempt {
 	a := &attempt{
 		runner: r,
 		number: len(r.attempts) + 1,
+		coverProfilePath: fmt.Sprintf(
+			"%v_%v%v",
+			strings.TrimSuffix(r.coverProfilePath, codeCoverageExtension),
+			len(r.attempts),
+			codeCoverageExtension),
 		junitReport: &junitReport{
 			path: filepath.Join(os.TempDir(), fmt.Sprintf("temporalio-temporal-%s-junit.xml", uuid.NewString())),
 		},