From 0bde93060c809a9a7295f1c11916b91db78a5c5c Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Fri, 30 Jun 2023 16:48:12 -0400 Subject: [PATCH 1/6] cmd/testwrapper: Use testwrapper output in github workflows Fixes #8493 Signed-off-by: Claire Wang --- .github/workflows/test.yml | 7 +++++++ cmd/testwrapper/testwrapper.go | 25 ++++++++++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8e6f77a0f..504f0d9d8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -93,6 +93,13 @@ jobs: run: PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}} env: GOARCH: ${{ matrix.goarch }} + - name: upload test output + uses: actions/upload-artifact@v3 + with: + name: ${{ github.job }}-${{ runner.os }}-${{ matrix.goarch }}-test-attempts.json + path: test_attempts.json + - name: remove upload test output + run: rm test_attempts.json - name: bench all run: PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}} -bench=. -benchtime=1x -run=^$ env: diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index de472fffd..9b1425d55 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -28,18 +28,21 @@ import ( const maxAttempts = 3 +// testAttempt keeps track of the test name, outcome, logs, and if the test is flakey. +// After running the tests, each testAttempt is written to a json file. type testAttempt struct { - name testName - outcome string // "pass", "fail", "skip" + name testName `json:"testName,omitempty"` + outcome string `json:"outcome,omitempty"` // "pass", "fail", "skip" logs bytes.Buffer isMarkedFlaky bool // set if the test is marked as flaky pkgFinished bool } +// testName keeps track of the test name and its package. type testName struct { - pkg string // "tailscale.com/types/key" - name string // "TestFoo" + Pkg string `json:"pkg,omitempty"` // "tailscale.com/types/key" + Name string `json:"name,omitempty"` // "TestFoo" } type packageTests struct { @@ -120,11 +123,11 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st continue } name := testName{ - pkg: goOutput.Package, - name: goOutput.Test, + Pkg: goOutput.Package, + Name: goOutput.Test, } if test, _, isSubtest := strings.Cut(goOutput.Test, "/"); isSubtest { - name.name = test + name.Name = test if goOutput.Action == "output" { resultMap[name].logs.WriteString(goOutput.Output) } @@ -135,19 +138,23 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st // ignore case "run": resultMap[name] = &testAttempt{ - name: name, + Name: name, } case "skip", "pass", "fail": resultMap[name].outcome = goOutput.Action ch <- resultMap[name] case "output": if strings.TrimSpace(goOutput.Output) == flakytest.FlakyTestLogMessage { - resultMap[name].isMarkedFlaky = true + resultMap[name].IsMarkedFlaky = true } else { resultMap[name].logs.WriteString(goOutput.Output) } } } + for _, result := range resultMap { + testAttemptJson, _ := json.Marshal(result) + f.Write(testAttemptJson) + } <-done } From fb2db85b809f9b69e2df403d9f5f458bbe5c26ea Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Fri, 30 Jun 2023 16:22:08 -0400 Subject: [PATCH 3/6] wip --- cmd/testwrapper/testwrapper.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index 9b1425d55..7eea9949f 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -239,6 +239,11 @@ func main() { failed := false toRetry := make(map[string][]string) // pkg -> tests to retry + f, err := os.Create("test_attempts.json") + if err != nil { + log.Printf("error creating test attempt json file: %v", err) + } + defer f.Close() for _, pt := range thisRun.tests { ch := make(chan *testAttempt) go runTests(ctx, thisRun.attempt, pt, otherArgs, ch) From 0b71d2c11aa15e26ac4cdd7ef5edff16dabb153e Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Fri, 30 Jun 2023 20:35:23 -0400 Subject: [PATCH 4/6] wip Signed-off-by: Claire Wang --- cmd/testwrapper/testwrapper.go | 80 ++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index 7eea9949f..b679b6982 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -31,12 +31,12 @@ const maxAttempts = 3 // testAttempt keeps track of the test name, outcome, logs, and if the test is flakey. // After running the tests, each testAttempt is written to a json file. type testAttempt struct { - name testName `json:"testName,omitempty"` - outcome string `json:"outcome,omitempty"` // "pass", "fail", "skip" + Name testName `json:"name,omitempty,inline"` + Outcome string `json:"outcome,omitempty"` // "pass", "fail", "skip" logs bytes.Buffer - isMarkedFlaky bool // set if the test is marked as flaky - - pkgFinished bool + IsMarkedFlaky bool `json:"is_marked_flaky,omitempty"` // set if the test is marked as flaky + AttemptCount int `json:"attempt_count,omitempty"` + PkgFinished bool `json:"pkg_finished,omitempty"` } // testName keeps track of the test name and its package. @@ -62,13 +62,17 @@ type goTestOutput struct { Output string } +type options struct { + w io.Writer // optional writer if the -w flag is set. +} + var debug = os.Getenv("TS_TESTWRAPPER_DEBUG") != "" // runTests runs the tests in pt and sends the results on ch. It sends a -// testAttempt for each test and a final testAttempt per pkg with pkgFinished +// testAttempt for each test and a final testAttempt per pkg with PkgFinished // set to true. // It calls close(ch) when it's done. -func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []string, ch chan<- *testAttempt) { +func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []string, ch chan<- *testAttempt, opt options) { defer close(ch) args := []string{"test", "-json", pt.pattern} args = append(args, otherArgs...) @@ -113,11 +117,11 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st switch goOutput.Action { case "fail", "pass", "skip": ch <- &testAttempt{ - name: testName{ - pkg: goOutput.Package, + Name: testName{ + Pkg: goOutput.Package, }, - outcome: goOutput.Action, - pkgFinished: true, + Outcome: goOutput.Action, + PkgFinished: true, } } continue @@ -138,10 +142,11 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st // ignore case "run": resultMap[name] = &testAttempt{ - Name: name, + Name: name, + AttemptCount: attempt, } case "skip", "pass", "fail": - resultMap[name].outcome = goOutput.Action + resultMap[name].Outcome = goOutput.Action ch <- resultMap[name] case "output": if strings.TrimSpace(goOutput.Output) == flakytest.FlakyTestLogMessage { @@ -151,9 +156,14 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st } } } - for _, result := range resultMap { - testAttemptJson, _ := json.Marshal(result) - f.Write(testAttemptJson) + if opt.w != nil { + for _, result := range resultMap { + testAttemptJson, _ := json.Marshal(result) + _, err = opt.w.Write(testAttemptJson) + if err != nil { + log.Printf("error appending to test attempt json file: %v", err) + } + } } <-done } @@ -161,14 +171,14 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st func main() { ctx := context.Background() - // We only need to parse the -v flag to figure out whether to print the logs - // for a test. We don't need to parse any other flags, so we just use the - // flag package to parse the -v flag and then pass the rest of the args - // through to 'go test'. + // We need to parse the -v flag to figure out whether to print the logs + // for a test. + // The -w flag is to indicate whether we want to write the json test results to a file. // We run `go test -json` which returns the same information as `go test -v`, // but in a machine-readable format. So this flag is only for testwrapper's // output. v := flag.Bool("v", false, "verbose") + w := flag.Bool("w", false, "write") flag.Usage = func() { fmt.Println("usage: testwrapper [testwrapper-flags] [pattern] [build/test flags & test binary flags]") @@ -179,6 +189,7 @@ func main() { fmt.Println("examples:") fmt.Println("\ttestwrapper -v ./... -count=1") fmt.Println("\ttestwrapper ./pkg/foo -run TestBar -count=1") + fmt.Println("\ttestwrapper -w ./pkg/foo -run TestBar -count=1") fmt.Println() fmt.Println("Unlike 'go test', testwrapper requires a package pattern as the first positional argument and only supports a single pattern.") } @@ -224,7 +235,15 @@ func main() { } fmt.Printf("%s\t%s\n", outcome, pkg) } - + opts := &options{} + if *w { + f, err := os.Create("test_attempts.json") + if err != nil { + log.Printf("error creating test attempt json file: %v", err) + } + defer f.Close() + opts.w = f + } for len(toRun) > 0 { var thisRun *nextRun thisRun, toRun = toRun[0], toRun[1:] @@ -239,27 +258,22 @@ func main() { failed := false toRetry := make(map[string][]string) // pkg -> tests to retry - f, err := os.Create("test_attempts.json") - if err != nil { - log.Printf("error creating test attempt json file: %v", err) - } - defer f.Close() for _, pt := range thisRun.tests { ch := make(chan *testAttempt) - go runTests(ctx, thisRun.attempt, pt, otherArgs, ch) + go runTests(ctx, thisRun.attempt, pt, otherArgs, ch, *opts) for tr := range ch { - if tr.pkgFinished { - printPkgOutcome(tr.name.pkg, tr.outcome, thisRun.attempt) + if tr.PkgFinished { + printPkgOutcome(tr.Name.Pkg, tr.Outcome, thisRun.attempt) continue } - if *v || tr.outcome == "fail" { + if *v || tr.Outcome == "fail" { io.Copy(os.Stdout, &tr.logs) } - if tr.outcome != "fail" { + if tr.Outcome != "fail" { continue } - if tr.isMarkedFlaky { - toRetry[tr.name.pkg] = append(toRetry[tr.name.pkg], tr.name.name) + if tr.IsMarkedFlaky { + toRetry[tr.Name.Pkg] = append(toRetry[tr.Name.Pkg], tr.Name.Name) } else { failed = true } From e2dca1c8b4540f697b090ec7e2a68f28bb50a696 Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Wed, 5 Jul 2023 13:20:23 -0400 Subject: [PATCH 5/6] wip Signed-off-by: Claire Wang --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 504f0d9d8..a863e5ab3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,7 +90,7 @@ jobs: - name: build test wrapper run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper - name: test all - run: PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}} + run: PATH=$PWD/tool:$PATH /tmp/testwrapper -w ./... ${{matrix.buildflags}} env: GOARCH: ${{ matrix.goarch }} - name: upload test output From 67fd260317cc806edc7e594bd2d1f963cebb6662 Mon Sep 17 00:00:00 2001 From: Claire Wang Date: Wed, 5 Jul 2023 14:27:50 -0400 Subject: [PATCH 6/6] wip Signed-off-by: Claire Wang --- .github/workflows/test.yml | 2 +- cmd/testwrapper/testwrapper.go | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a863e5ab3..961b8683c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,7 +90,7 @@ jobs: - name: build test wrapper run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper - name: test all - run: PATH=$PWD/tool:$PATH /tmp/testwrapper -w ./... ${{matrix.buildflags}} + run: PATH=$PWD/tool:$PATH /tmp/testwrapper -json test_attempts.json ./... ${{matrix.buildflags}} env: GOARCH: ${{ matrix.goarch }} - name: upload test output diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index b679b6982..07d4ffa4d 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -31,18 +31,18 @@ const maxAttempts = 3 // testAttempt keeps track of the test name, outcome, logs, and if the test is flakey. // After running the tests, each testAttempt is written to a json file. type testAttempt struct { - Name testName `json:"name,omitempty,inline"` - Outcome string `json:"outcome,omitempty"` // "pass", "fail", "skip" + Name testName `json:",inline"` + Outcome string `json:",omitempty"` // "pass", "fail", "skip" logs bytes.Buffer - IsMarkedFlaky bool `json:"is_marked_flaky,omitempty"` // set if the test is marked as flaky - AttemptCount int `json:"attempt_count,omitempty"` - PkgFinished bool `json:"pkg_finished,omitempty"` + IsMarkedFlaky bool `json:",omitempty"` // set if the test is marked as flaky + AttemptCount int `json:",omitempty"` + PkgFinished bool `json:",omitempty"` } // testName keeps track of the test name and its package. type testName struct { - Pkg string `json:"pkg,omitempty"` // "tailscale.com/types/key" - Name string `json:"name,omitempty"` // "TestFoo" + Pkg string `json:",omitempty"` // "tailscale.com/types/key" + Name string `json:",omitempty"` // "TestFoo" } type packageTests struct { @@ -173,12 +173,12 @@ func main() { // We need to parse the -v flag to figure out whether to print the logs // for a test. - // The -w flag is to indicate whether we want to write the json test results to a file. + // The -json flag is to indicate whether we want to write the json test results to a provided file. // We run `go test -json` which returns the same information as `go test -v`, // but in a machine-readable format. So this flag is only for testwrapper's // output. v := flag.Bool("v", false, "verbose") - w := flag.Bool("w", false, "write") + jsonFile := flag.String("json", "", "if set, the file to store all test results in") flag.Usage = func() { fmt.Println("usage: testwrapper [testwrapper-flags] [pattern] [build/test flags & test binary flags]") @@ -189,7 +189,7 @@ func main() { fmt.Println("examples:") fmt.Println("\ttestwrapper -v ./... -count=1") fmt.Println("\ttestwrapper ./pkg/foo -run TestBar -count=1") - fmt.Println("\ttestwrapper -w ./pkg/foo -run TestBar -count=1") + fmt.Println("\ttestwrapper -json=test_attempts.json ./pkg/foo -run TestBar -count=1") fmt.Println() fmt.Println("Unlike 'go test', testwrapper requires a package pattern as the first positional argument and only supports a single pattern.") } @@ -236,8 +236,8 @@ func main() { fmt.Printf("%s\t%s\n", outcome, pkg) } opts := &options{} - if *w { - f, err := os.Create("test_attempts.json") + if *jsonFile != "" { + f, err := os.Create(*jsonFile) if err != nil { log.Printf("error creating test attempt json file: %v", err) }