diff --git a/.github/workflows/coverage-ut.yaml b/.github/workflows/coverage-ut.yaml index 86f5ed5..12c2e0d 100644 --- a/.github/workflows/coverage-ut.yaml +++ b/.github/workflows/coverage-ut.yaml @@ -308,7 +308,8 @@ jobs: fi } > "${timeout_diagnostics}" elif { [ "${phase_name}" = 'discover_packages' ] && [ "${phase_status}" != '0' ]; } || \ - { [ "${phase_name}" = 'prepare_cgo' ] && [ "${phase_status}" != '0' ]; }; then + { [ "${phase_name}" = 'prepare_cgo' ] && [ "${phase_status}" != '0' ]; } || \ + { [ "${phase_name}" = 'build_coverage_scope' ] && [ "${phase_status}" != '0' ]; }; then { echo "Coverage UT phase failed: ${phase_name}" echo "phase=${phase_name} status=${phase_status}" @@ -345,14 +346,21 @@ jobs: set -e phase_end "${phase_status}" "${COVERAGE_UT_DISCOVERY_TIMEOUT_SECONDS}" (( phase_status == 0 )) || exit "${phase_status}" - cover_pkgs=$(printf '%s\n' "${test_scope}" | paste -sd, -) - if [ -z "${cover_pkgs}" ]; then - echo 'Coverage UT package scope is empty after exclusions.' > "${phase_failure}" - echo 'phase=build_scope status=1 elapsed_seconds=0' >> "${phase_timing}" - exit 1 - fi + # Use the existing profile policy before instrumentation too. Test + # selection stays complete, including tests in excluded packages. + phase_start build_coverage_scope + coverage_helper="${RUNNER_TEMP}/compact-coverage-profile" + set +e + timeout --kill-after=30s "${COVERAGE_UT_DISCOVERY_TIMEOUT_SECONDS}s" \ + go build -o "${coverage_helper}" "$GITHUB_WORKSPACE/.ci-workflow/scripts/compact_coverage_profile.go" && \ + cover_pkgs=$(printf '%s\n' "${test_scope}" | "${coverage_helper}" -packages) + phase_status=$? + set -e + phase_end "${phase_status}" "${COVERAGE_UT_DISCOVERY_TIMEOUT_SECONDS}" + (( phase_status == 0 )) || exit "${phase_status}" package_count=$(printf '%s\n' "${test_scope}" | awk 'NF { count++ } END { print count + 0 }') - echo "coverage_ut package_count=${package_count} package_parallelism=${COVERAGE_UT_PACKAGE_PARALLELISM}" | tee -a "${phase_timing}" + instrumented_package_count=$(printf '%s\n' "${cover_pkgs}" | awk -F, '{ print NF }') + echo "coverage_ut package_count=${package_count} package_parallelism=${COVERAGE_UT_PACKAGE_PARALLELISM} instrumented_package_count=${instrumented_package_count}" | tee -a "${phase_timing}" phase_start prepare_cgo set +e @@ -533,7 +541,7 @@ jobs: # Go helper ORs exact block hits with memory proportional to unique # blocks, then atomically replaces the raw profile after success. set +e - go run "$GITHUB_WORKSPACE/.ci-workflow/scripts/compact_coverage_profile.go" "${coverage_profile}" + "${coverage_helper}" "${coverage_profile}" phase_status=$? set -e phase_end "${phase_status}" diff --git a/scripts/compact_coverage_profile.go b/scripts/compact_coverage_profile.go index b1df6a6..c7b3ce3 100644 --- a/scripts/compact_coverage_profile.go +++ b/scripts/compact_coverage_profile.go @@ -23,6 +23,44 @@ var excludedPaths = [][]byte{ []byte("yaccpar"), } +func excludedCoveragePath(path []byte) bool { + for _, fragment := range excludedPaths { + if bytes.Contains(path, fragment) { + return true + } + } + return false +} + +// coveragePackages applies the report's exclusions before instrumentation. +// This only selects counters; the workflow still executes every test package. +func coveragePackages(in io.Reader, out io.Writer) error { + reader := bufio.NewScanner(in) + var packages bytes.Buffer + for reader.Scan() { + name := reader.Bytes() + if len(name) == 0 || bytes.ContainsAny(name, " ,\t\r\\") { + return errors.New("invalid coverage package name") + } + if excludedCoveragePath(name) { + continue + } + if packages.Len() > 0 { + packages.WriteByte(',') + } + packages.Write(name) + } + if err := reader.Err(); err != nil { + return err + } + if packages.Len() == 0 { + return errors.New("empty coverage instrumentation scope") + } + packages.WriteByte('\n') + _, err := packages.WriteTo(out) + return err +} + // compact retains one key per distinct (source range, statement count), not // one key per input row. The common duplicate lookup does not retain a copy of // the reader's buffer. @@ -62,15 +100,7 @@ func compact(in io.Reader, out io.Writer) (int, error) { return 0, fmt.Errorf("invalid hit count on coverage line %d", lineNumber) } - path := line[:firstSpace] - excluded := false - for _, fragment := range excludedPaths { - if bytes.Contains(path, fragment) { - excluded = true - break - } - } - if excluded { + if excludedCoveragePath(line[:firstSpace]) { continue } keyBytes := line[:secondSpace] @@ -202,11 +232,17 @@ func compactFile(path string) error { func main() { if len(os.Args) != 2 { - fmt.Fprintln(os.Stderr, "usage: compact_coverage_profile.go COVERAGE_PROFILE") + fmt.Fprintln(os.Stderr, "usage: compact_coverage_profile COVERAGE_PROFILE | -packages < PACKAGE_LIST") os.Exit(2) } - if err := compactFile(os.Args[1]); err != nil { - fmt.Fprintf(os.Stderr, "coverage profile compaction failed: %v\n", err) + var err error + if os.Args[1] == "-packages" { + err = coveragePackages(os.Stdin, os.Stdout) + } else { + err = compactFile(os.Args[1]) + } + if err != nil { + fmt.Fprintf(os.Stderr, "coverage profile processing failed: %v\n", err) os.Exit(1) } } diff --git a/scripts/compact_coverage_profile_test.go b/scripts/compact_coverage_profile_test.go index d059d7d..ae04ffb 100644 --- a/scripts/compact_coverage_profile_test.go +++ b/scripts/compact_coverage_profile_test.go @@ -2,16 +2,110 @@ package main import ( "bytes" + "context" "errors" "io" "os" + "os/exec" "path/filepath" "strings" "testing" + "time" ) const coverageHeader = "mode: set\n" +func TestCoveragePackagesKeepsReportScopeAndRejectsIncompleteInput(t *testing.T) { + input := "example.com/pkg/work\nexample.com/pkg/pb/model\nexample.com/pkg/sql/parsers/goyacc\nexample.com/pkg/untested\n" + var output bytes.Buffer + if err := coveragePackages(strings.NewReader(input), &output); err != nil { + t.Fatal(err) + } + if got, want := output.String(), "example.com/pkg/work,example.com/pkg/untested\n"; got != want { + t.Fatalf("instrumentation scope=%q, want %q", got, want) + } + for _, input := range []string{"", "example.com/pkg/pb/model\n", "example.com/a,example.com/b\n", "example.com/a\n\n"} { + output.Reset() + if err := coveragePackages(strings.NewReader(input), &output); err == nil || output.Len() != 0 { + t.Fatalf("invalid scope %q: error=%v output=%q", input, err, output.String()) + } + } + if err := coveragePackages(failureReader{}, &output); err == nil { + t.Fatal("lost scope read failure") + } + if err := coveragePackages(strings.NewReader("example.com/a\n"), failureWriter{}); err == nil { + t.Fatal("lost scope write failure") + } +} + +func TestCoveragePackagesPreservesCrossPackageHitsAndUntestedBlocks(t *testing.T) { + dir := t.TempDir() + files := map[string]string{ + "go.mod": "module example.com\n\ngo 1.23.0\n", + "pkg/pb/model/model.go": "package model\nfunc Normalize(n int) int { if n < 0 { return -n }; return n }\n", + "pkg/work/work.go": "package work\nimport \"example.com/pkg/pb/model\"\nfunc Process(n int) int { if n == 0 { return 0 }; return model.Normalize(n)+1 }\n", + "pkg/work/work_test.go": "package work\nimport \"testing\"\nfunc TestZero(t *testing.T) { if Process(0) != 0 { t.Fatal(\"zero\") } }\n", + "pkg/pb/model/model_test.go": "package model_test\nimport (\"testing\"; \"example.com/pkg/work\")\nfunc TestDependencyFromExcludedPackage(t *testing.T) { if work.Process(-2) != 3 { t.Fatal(\"dependency\") } }\n", + "pkg/pb/plain/plain.go": "package plain\nfunc Value() int { return 7 }\n", + "pkg/pb/plain/plain_test.go": "package plain\nimport \"testing\"\nfunc TestExcludedOnly(t *testing.T) { if Value() != 7 { t.Fatal(\"value\") } }\n", + "pkg/untested/u.go": "package untested\nfunc Uncalled() int { return 42 }\n", + } + for name, data := range files { + path := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(data), 0600); err != nil { + t.Fatal(err) + } + } + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + var scope bytes.Buffer + if err := coveragePackages(strings.NewReader("example.com/pkg/pb/model\nexample.com/pkg/pb/plain\nexample.com/pkg/untested\nexample.com/pkg/work\n"), &scope); err != nil { + t.Fatal(err) + } + var profiles []string + for _, coverpkg := range []string{"./...", strings.TrimSpace(scope.String())} { + profile := filepath.Join(dir, "coverage.out") + cmd := exec.CommandContext(ctx, "go", "test", "-count=1", "-v", "-covermode=set", "-coverpkg="+coverpkg, "-coverprofile="+profile, "./...") + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GOWORK=off", "GOFLAGS=") + cmd.WaitDelay = time.Second + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go test: %v\n%s", err, output) + } + if !bytes.Contains(output, []byte("--- PASS: TestDependencyFromExcludedPackage")) { + t.Fatalf("excluded-package test did not execute:\n%s", output) + } + if !bytes.Contains(output, []byte("--- PASS: TestExcludedOnly")) { + t.Fatalf("excluded-only package test did not execute:\n%s", output) + } + raw, err := os.ReadFile(profile) + if err != nil { + t.Fatal(err) + } + var result bytes.Buffer + if _, err := compact(bytes.NewReader(raw), &result); err != nil { + t.Fatal(err) + } + profiles = append(profiles, result.String()) + } + if profiles[0] != profiles[1] { + t.Fatalf("published blocks/hits changed:\nbaseline:\n%scandidate:\n%s", profiles[0], profiles[1]) + } + retainedZero := false + for _, line := range strings.Split(profiles[1], "\n") { + if strings.HasPrefix(line, "example.com/pkg/untested/u.go:") && strings.HasSuffix(line, " 1 0") { + retainedZero = true + } + } + if !retainedZero { + t.Fatalf("lost untested-package denominator:\n%s", profiles[1]) + } +} + func TestCompactUnionsExactBlocksAndExclusions(t *testing.T) { input := coverageHeader + "example.com/a.go:1.1,1.2 1 0\n" +