cmd/testwrapper: move from corp; mark magicsock test as flaky

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ibab5860f5797b3db151d3c27855333e43a9088a4

.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)

.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)

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)
+		}
+	})
+}

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)
+		}
+	}
+}

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)
+}

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,