Upload code coverage

temporalio · Feb 10, 2025 · 0361e36 · 0361e36
1 parent ea6a2e6
commit 0361e36
Show file tree

Hide file tree

Showing 4 changed files with 198 additions and 115 deletions.
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -275,6 +275,14 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/cover.summary.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          file: ./.testoutput/cover.summary.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]
@@ -354,6 +362,14 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/cover.summary.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          file: ./.testoutput/cover.summary.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]
@@ -504,6 +520,14 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/cover.summary.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          file: ./.testoutput/cover.summary.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]
@@ -628,6 +652,14 @@ jobs:
           annotate_only: true
           skip_annotations: true
 
+      - name: Generate code coverage
+        run: make ./.testoutput/cover.summary.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          file: ./.testoutput/cover.summary.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 +775,14 @@ jobs:
         timeout-minutes: 15
         run: make functional-test-ndc-coverage
 
+      - name: Generate code coverage
+        run: make ./.testoutput/cover.summary.out
+      - name: Upload code coverage
+        uses: coverallsapp/github-action@v2
+        with:
+          fail-on-error: false
+          file: ./.testoutput/cover.summary.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]

diff --git a/Makefile b/Makefile
@@ -121,21 +121,21 @@ PINNED_DEPENDENCIES := \
 # Code coverage & test report output files.
 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
+SUMMARY_COVER_PROFILE  := $(TEST_OUTPUT_ROOT)/cover.summary.out  # cannot end with `.cover.out` or it will be included recursively
 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,52 +413,57 @@ 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) -- \
+	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)
 
 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) -- \
+	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)
+		-coverprofile=$(NEW_COVER_PROFILE)
 
 # 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) -- \
+	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) \
+		-coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) \
 		-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) -- \
+	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) \
+		-coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) \
 		-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) -- \
+	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) \
+		-coverprofile=$(NEW_COVER_PROFILE) $(COVERPKG_FLAG) \
 		-args -persistenceType=$(PERSISTENCE_TYPE) -persistenceDriver=$(PERSISTENCE_DRIVER)
 
 .PHONY: $(SUMMARY_COVER_PROFILE)
 $(SUMMARY_COVER_PROFILE):
 	@printf $(COLOR) "Combine coverage reports to $(SUMMARY_COVER_PROFILE)..."
 	@rm -f $(SUMMARY_COVER_PROFILE) $(SUMMARY_COVER_PROFILE).html
 	@if [ -z "$(wildcard $(TEST_OUTPUT_ROOT)/*.cover.out)" ]; then \
-		echo "No coverage data, aborting!" && exit 1; \
+		echo "No coverage data, skipping!"; \
 	fi
 	@echo "mode: atomic" > $(SUMMARY_COVER_PROFILE)
 	$(foreach 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,32 @@ import (
 	"github.com/google/uuid"
 )
 
+const (
+	codeCoverageExtension = ".cover.out"
+	retriesFlag           = "--retries="
+	coverProfileFlag      = "-coverprofile="
+	junitReportFlag       = "--junitfile="
+	packagesFlag          = "--packages="
+)
+
 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 +79,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 +93,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 +131,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())),
 		},