From aea251d42a2f953fdd5771d95888b3b051f93b8c Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Wed, 18 Jan 2023 11:41:58 -0500 Subject: [PATCH] cmd/testwrapper: move from corp; mark magicsock test as flaky Signed-off-by: Andrew Dunham Change-Id: Ibab5860f5797b3db151d3c27855333e43a9088a4 --- .github/workflows/linux-race.yml | 5 +- .github/workflows/linux.yml | 5 +- cmd/testwrapper/flakytest/flakytest.go | 46 +++++++++++++++ cmd/testwrapper/flakytest/flakytest_test.go | 27 +++++++++ cmd/testwrapper/testwrapper.go | 63 +++++++++++++++++++++ wgengine/magicsock/magicsock_test.go | 2 + 6 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 cmd/testwrapper/flakytest/flakytest.go create mode 100644 cmd/testwrapper/flakytest/flakytest_test.go create mode 100644 cmd/testwrapper/testwrapper.go diff --git a/.github/workflows/linux-race.yml b/.github/workflows/linux-race.yml index 55abdba20..f34acdb7c 100644 --- a/.github/workflows/linux-race.yml +++ b/.github/workflows/linux-race.yml @@ -29,11 +29,14 @@ jobs: go-version-file: go.mod id: go + - name: Build test wrapper + run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper + - name: Basic build run: go build ./cmd/... - name: Run tests and benchmarks with -race flag on linux - run: go test -race -bench=. -benchtime=1x ./... + run: go test -exec=/tmp/testwrapper -race -bench=. -benchtime=1x ./... - name: Check that no tracked files in the repo have been modified run: git diff --no-ext-diff --name-only --exit-code || (echo "Build/test modified the files above."; exit 1) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 7b0dfd022..0f6588c72 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -32,6 +32,9 @@ jobs: - name: Basic build run: go build ./cmd/... + - name: Build test wrapper + run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper + - name: Build variants run: | go install --tags=ts_include_cli ./cmd/tailscaled @@ -43,7 +46,7 @@ jobs: sudo apt-get -y install qemu-user - name: Run tests on linux - run: go test -bench=. -benchtime=1x ./... + run: go test -exec=/tmp/testwrapper -bench=. -benchtime=1x ./... - name: Check that no tracked files in the repo have been modified run: git diff --no-ext-diff --name-only --exit-code || (echo "Build/test modified the files above."; exit 1) diff --git a/cmd/testwrapper/flakytest/flakytest.go b/cmd/testwrapper/flakytest/flakytest.go new file mode 100644 index 000000000..c210f8027 --- /dev/null +++ b/cmd/testwrapper/flakytest/flakytest.go @@ -0,0 +1,46 @@ +// Copyright (c) 2023 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package flakytest contains test helpers for marking a test as flaky. For +// tests run using cmd/testwrapper, a failed flaky test will cause tests to be +// re-run a few time until they succeed or exceed our iteration limit. +package flakytest + +import ( + "os" + "regexp" + "testing" +) + +// InTestWrapper returns whether or not this binary is running under our test +// wrapper. +func InTestWrapper() bool { + return os.Getenv("TS_IN_TESTWRAPPER") != "" +} + +var issueRegexp = regexp.MustCompile(`\Ahttps://github\.com/tailscale/[a-zA-Z0-9_.-]+/issues/\d+\z`) + +// Mark sets the current test as a flaky test, such that if it fails, it will +// be retried a few times on failure. issue must be a GitHub issue that tracks +// the status of the flaky test being marked, of the format: +// +// https://github.com/tailscale/myRepo-H3re/issues/12345 +func Mark(t *testing.T, issue string) { + if !issueRegexp.MatchString(issue) { + t.Fatalf("bad issue format: %q", issue) + } + + if !InTestWrapper() { + return + } + + t.Cleanup(func() { + if t.Failed() { + t.Logf("flakytest: signaling test wrapper to retry test") + + // Signal to test wrapper that we should restart. + os.Exit(123) + } + }) +} diff --git a/cmd/testwrapper/flakytest/flakytest_test.go b/cmd/testwrapper/flakytest/flakytest_test.go new file mode 100644 index 000000000..7e64a4c6b --- /dev/null +++ b/cmd/testwrapper/flakytest/flakytest_test.go @@ -0,0 +1,27 @@ +// Copyright (c) 2023 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package flakytest + +import "testing" + +func TestIssueFormat(t *testing.T) { + testCases := []struct { + issue string + want bool + }{ + {"https://github.com/tailscale/cOrp/issues/1234", true}, + {"https://github.com/otherproject/corp/issues/1234", false}, + {"https://github.com/tailscale/corp/issues/", false}, + } + for _, testCase := range testCases { + if issueRegexp.MatchString(testCase.issue) != testCase.want { + ss := "" + if !testCase.want { + ss = " not" + } + t.Errorf("expected issueRegexp to%s match %q", ss, testCase.issue) + } + } +} diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go new file mode 100644 index 000000000..a1ec34861 --- /dev/null +++ b/cmd/testwrapper/testwrapper.go @@ -0,0 +1,63 @@ +// Copyright (c) 2023 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// testwrapper is a wrapper for retrying flaky tests, using the -exec flag of +// 'go test'. Tests that are flaky can use the 'flakytest' subpackage to mark +// themselves as flaky and be retried on failure. +package main + +import ( + "context" + "errors" + "log" + "os" + "os/exec" +) + +const ( + retryStatus = 123 + maxIterations = 3 +) + +func main() { + ctx := context.Background() + debug := os.Getenv("TS_TESTWRAPPER_DEBUG") != "" + + log.SetPrefix("testwrapper: ") + if !debug { + log.SetFlags(0) + } + + for i := 1; i <= maxIterations; i++ { + if i > 1 { + log.Printf("retrying flaky tests (%d of %d)", i, maxIterations) + } + cmd := exec.CommandContext(ctx, os.Args[1], os.Args[2:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = append(os.Environ(), "TS_IN_TESTWRAPPER=1") + err := cmd.Run() + if err == nil { + return + } + + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + if debug { + log.Printf("error isn't an ExitError") + } + os.Exit(1) + } + + if code := exitErr.ExitCode(); code != retryStatus { + if debug { + log.Printf("code (%d) != retryStatus (%d)", code, retryStatus) + } + os.Exit(code) + } + } + + log.Printf("test did not pass in %d iterations", maxIterations) + os.Exit(1) +} diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index b0902bdd1..be0b73afd 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -32,6 +32,7 @@ import ( "github.com/tailscale/wireguard-go/tun/tuntest" "go4.org/mem" "golang.org/x/exp/maps" + "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/derp" "tailscale.com/derp/derphttp" "tailscale.com/disco" @@ -602,6 +603,7 @@ func (localhostListener) ListenPacket(ctx context.Context, network, address stri } func TestTwoDevicePing(t *testing.T) { + flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/1277") l, ip := localhostListener{}, netaddr.IPv4(127, 0, 0, 1) n := &devices{ m1: l,