From cbf3852b5d3081ec44c924b0045fc9476ea7aea1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 17 Feb 2025 08:58:38 -0800 Subject: [PATCH] cmd/testwrapper: temporarily remove test coverage support testwrapper doesn't work with Go 1.24 and the coverage support is making it harder to debug. Updates #15015 Updates tailscale/corp#26659 Change-Id: I0125e881d08c92f1ecef88b57344f6bbb571b569 Signed-off-by: Brad Fitzpatrick --- .github/workflows/test.yml | 8 +- cmd/testwrapper/testwrapper.go | 149 +-------------------------------- go.mod | 4 - go.sum | 8 -- 4 files changed, 2 insertions(+), 167 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a368afc67..7142c86b9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -64,7 +64,6 @@ jobs: matrix: include: - goarch: amd64 - coverflags: "-coverprofile=/tmp/coverage.out" - goarch: amd64 buildflags: "-race" shard: '1/3' @@ -119,15 +118,10 @@ jobs: - name: build test wrapper run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper - name: test all - run: NOBASHDEBUG=true PATH=$PWD/tool:$PATH /tmp/testwrapper ${{matrix.coverflags}} ./... ${{matrix.buildflags}} + run: NOBASHDEBUG=true PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}} env: GOARCH: ${{ matrix.goarch }} TS_TEST_SHARD: ${{ matrix.shard }} - - name: Publish to coveralls.io - if: matrix.coverflags != '' # only publish results if we've tracked coverage - uses: shogo82148/actions-goveralls@v1 - with: - path-to-profile: /tmp/coverage.out - name: bench all run: ./tool/go test ${{matrix.buildflags}} -bench=. -benchtime=1x -run=^$ $(for x in $(git grep -l "^func Benchmark" | xargs dirname | sort | uniq); do echo "./$x"; done) env: diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index 91aea904e..67b8a1483 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -22,13 +22,7 @@ "sort" "strings" "time" - "unicode" - "github.com/dave/courtney/scanner" - "github.com/dave/courtney/shared" - "github.com/dave/courtney/tester" - "github.com/dave/patsy" - "github.com/dave/patsy/vos" "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/util/slicesx" ) @@ -238,30 +232,6 @@ type nextRun struct { fmt.Printf("%s\t%s\t%.3fs\n", outcome, pkg, runtime.Seconds()) } - // Check for -coverprofile argument and filter it out - combinedCoverageFilename := "" - filteredGoTestArgs := make([]string, 0, len(goTestArgs)) - preceededByCoverProfile := false - for _, arg := range goTestArgs { - if arg == "-coverprofile" { - preceededByCoverProfile = true - } else if preceededByCoverProfile { - combinedCoverageFilename = strings.TrimSpace(arg) - preceededByCoverProfile = false - } else { - filteredGoTestArgs = append(filteredGoTestArgs, arg) - } - } - goTestArgs = filteredGoTestArgs - - runningWithCoverage := combinedCoverageFilename != "" - if runningWithCoverage { - fmt.Printf("Will log coverage to %v\n", combinedCoverageFilename) - } - - // Keep track of all test coverage files. With each retry, we'll end up - // with additional coverage files that will be combined when we finish. - coverageFiles := make([]string, 0) for len(toRun) > 0 { var thisRun *nextRun thisRun, toRun = toRun[0], toRun[1:] @@ -275,27 +245,13 @@ type nextRun struct { fmt.Printf("\n\nAttempt #%d: Retrying flaky tests:\n\nflakytest failures JSON: %s\n\n", thisRun.attempt, j) } - goTestArgsWithCoverage := testArgs - if runningWithCoverage { - coverageFile := fmt.Sprintf("/tmp/coverage_%d.out", thisRun.attempt) - coverageFiles = append(coverageFiles, coverageFile) - goTestArgsWithCoverage = make([]string, len(goTestArgs), len(goTestArgs)+2) - copy(goTestArgsWithCoverage, goTestArgs) - goTestArgsWithCoverage = append( - goTestArgsWithCoverage, - fmt.Sprintf("-coverprofile=%v", coverageFile), - "-covermode=set", - "-coverpkg=./...", - ) - } - toRetry := make(map[string][]*testAttempt) // pkg -> tests to retry for _, pt := range thisRun.tests { ch := make(chan *testAttempt) runErr := make(chan error, 1) go func() { defer close(runErr) - runErr <- runTests(ctx, thisRun.attempt, pt, goTestArgsWithCoverage, testArgs, ch) + runErr <- runTests(ctx, thisRun.attempt, pt, goTestArgs, testArgs, ch) }() var failed bool @@ -372,107 +328,4 @@ type nextRun struct { } toRun = append(toRun, nextRun) } - - if runningWithCoverage { - intermediateCoverageFilename := "/tmp/coverage.out_intermediate" - if err := combineCoverageFiles(intermediateCoverageFilename, coverageFiles); err != nil { - fmt.Printf("error combining coverage files: %v\n", err) - os.Exit(2) - } - - if err := processCoverageWithCourtney(intermediateCoverageFilename, combinedCoverageFilename, testArgs); err != nil { - fmt.Printf("error processing coverage with courtney: %v\n", err) - os.Exit(3) - } - - fmt.Printf("Wrote combined coverage to %v\n", combinedCoverageFilename) - } -} - -func combineCoverageFiles(intermediateCoverageFilename string, coverageFiles []string) error { - combinedCoverageFile, err := os.OpenFile(intermediateCoverageFilename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) - if err != nil { - return fmt.Errorf("create /tmp/coverage.out: %w", err) - } - defer combinedCoverageFile.Close() - w := bufio.NewWriter(combinedCoverageFile) - defer w.Flush() - - for fileNumber, coverageFile := range coverageFiles { - f, err := os.Open(coverageFile) - if err != nil { - return fmt.Errorf("open %v: %w", coverageFile, err) - } - defer f.Close() - in := bufio.NewReader(f) - line := 0 - for { - r, _, err := in.ReadRune() - if err != nil { - if err != io.EOF { - return fmt.Errorf("read %v: %w", coverageFile, err) - } - break - } - - // On all but the first coverage file, skip the coverage file header - if fileNumber > 0 && line == 0 { - continue - } - if r == '\n' { - line++ - } - - // filter for only printable characters because coverage file sometimes includes junk on 2nd line - if unicode.IsPrint(r) || r == '\n' { - if _, err := w.WriteRune(r); err != nil { - return fmt.Errorf("write %v: %w", combinedCoverageFile.Name(), err) - } - } - } - } - - return nil -} - -// processCoverageWithCourtney post-processes code coverage to exclude less -// meaningful sections like 'if err != nil { return err}', as well as -// anything marked with a '// notest' comment. -// -// instead of running the courtney as a separate program, this embeds -// courtney for easier integration. -func processCoverageWithCourtney(intermediateCoverageFilename, combinedCoverageFilename string, testArgs []string) error { - env := vos.Os() - - setup := &shared.Setup{ - Env: vos.Os(), - Paths: patsy.NewCache(env), - TestArgs: testArgs, - Load: intermediateCoverageFilename, - Output: combinedCoverageFilename, - } - if err := setup.Parse(testArgs); err != nil { - return fmt.Errorf("parse args: %w", err) - } - - s := scanner.New(setup) - if err := s.LoadProgram(); err != nil { - return fmt.Errorf("load program: %w", err) - } - if err := s.ScanPackages(); err != nil { - return fmt.Errorf("scan packages: %w", err) - } - - t := tester.New(setup) - if err := t.Load(); err != nil { - return fmt.Errorf("load: %w", err) - } - if err := t.ProcessExcludes(s.Excludes); err != nil { - return fmt.Errorf("process excludes: %w", err) - } - if err := t.Save(); err != nil { - return fmt.Errorf("save: %w", err) - } - - return nil } diff --git a/go.mod b/go.mod index e0c945f83..0845008bb 100644 --- a/go.mod +++ b/go.mod @@ -21,8 +21,6 @@ require ( github.com/coreos/go-iptables v0.7.1-0.20240112124308-65c67c9f46e6 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf github.com/creack/pty v1.1.23 - github.com/dave/courtney v0.4.0 - github.com/dave/patsy v0.0.0-20210517141501-957256f50cba github.com/dblohm7/wingoes v0.0.0-20240119213807-a09d6be7affa github.com/digitalocean/go-smbios v0.0.0-20180907143718-390a4f403a8e github.com/distribution/reference v0.6.0 @@ -135,8 +133,6 @@ require ( github.com/ccojocar/zxcvbn-go v1.0.2 // indirect github.com/ckaznocha/intrange v0.1.0 // indirect github.com/cyphar/filepath-securejoin v0.3.6 // indirect - github.com/dave/astrid v0.0.0-20170323122508-8c2895878b14 // indirect - github.com/dave/brenda v1.1.0 // indirect github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-units v0.5.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect diff --git a/go.sum b/go.sum index 6ea727014..7be4c3eaf 100644 --- a/go.sum +++ b/go.sum @@ -240,14 +240,6 @@ github.com/cyphar/filepath-securejoin v0.3.6 h1:4d9N5ykBnSp5Xn2JkhocYDkOpURL/18C github.com/cyphar/filepath-securejoin v0.3.6/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI= github.com/daixiang0/gci v0.12.3 h1:yOZI7VAxAGPQmkb1eqt5g/11SUlwoat1fSblGLmdiQc= github.com/daixiang0/gci v0.12.3/go.mod h1:xtHP9N7AHdNvtRNfcx9gwTDfw7FRJx4bZUsiEfiNNAI= -github.com/dave/astrid v0.0.0-20170323122508-8c2895878b14 h1:YI1gOOdmMk3xodBao7fehcvoZsEeOyy/cfhlpCSPgM4= -github.com/dave/astrid v0.0.0-20170323122508-8c2895878b14/go.mod h1:Sth2QfxfATb/nW4EsrSi2KyJmbcniZ8TgTaji17D6ms= -github.com/dave/brenda v1.1.0 h1:Sl1LlwXnbw7xMhq3y2x11McFu43AjDcwkllxxgZ3EZw= -github.com/dave/brenda v1.1.0/go.mod h1:4wCUr6gSlu5/1Tk7akE5X7UorwiQ8Rij0SKH3/BGMOM= -github.com/dave/courtney v0.4.0 h1:Vb8hi+k3O0h5++BR96FIcX0x3NovRbnhGd/dRr8inBk= -github.com/dave/courtney v0.4.0/go.mod h1:3WSU3yaloZXYAxRuWt8oRyVb9SaRiMBt5Kz/2J227tM= -github.com/dave/patsy v0.0.0-20210517141501-957256f50cba h1:1o36L4EKbZzazMk8iGC4kXpVnZ6TPxR2mZ9qVKjNNAs= -github.com/dave/patsy v0.0.0-20210517141501-957256f50cba/go.mod h1:qfR88CgEGLoiqDaE+xxDCi5QA5v4vUoW0UCX2Nd5Tlc= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=