cmd/testwrapper, tstest: move test sharding out of test code

Previously, sharding required tests to opt in by calling tstest.Shard,
which used a process-global counter to assign each test to a shard.
This had two problems: most tests didn't call it, so they ran on every
shard (defeating the purpose), and shard assignments were unstable
(depended on call order, so adding a test could reshuffle others).

Remove tstest.Shard and tstest.SkipOnUnshardedCI entirely. Instead,
have testwrapper implement sharding automatically for all tests: when
TS_TEST_SHARD=N/M is set, it uses "go list -json" (no compilation) to
find test source files, scans them for top-level Test/Benchmark/
Example/Fuzz function names, and filters by fnv32a(name) % M == N-1.
The filtered names are passed as an anchored -run regex to go test.

Using go list instead of "go test -list" avoids linking the test binary
twice (Go's build cache does not cache test binary linking).

Fixes #19886

Change-Id: I62ab7b3d757324d4c5fd0b5de50c1e3742681791
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

tstest/integration/capmap_test.go

@@ -14,7 +14,6 @@ import (
 
 // TestPeerCapMap tests that the node capability map (CapMap) is included in peer information.
 func TestPeerCapMap(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 
@@ -97,7 +96,6 @@ func TestPeerCapMap(t *testing.T) {
 
 // TestSetNodeCapMap tests that SetNodeCapMap updates are propagated to peers.
 func TestSetNodeCapMap(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 

tstest/integration/integration_test.go

@@ -79,7 +79,6 @@ func fetchNetMapForTest(ctx context.Context, lc *local.Client) (*netmap.NetworkM
 // Tests that tailscaled starts up in TUN mode, and also without data races:
 // https://github.com/tailscale/tailscale/issues/7894
 func TestTUNMode(t *testing.T) {
-	tstest.Shard(t)
 	tstest.RequireRoot(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
@@ -97,7 +96,6 @@ func TestTUNMode(t *testing.T) {
 }
 
 func TestOneNodeUpNoAuth(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -115,7 +113,6 @@ func TestOneNodeUpNoAuth(t *testing.T) {
 }
 
 func TestOneNodeExpiredKey(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -152,7 +149,6 @@ func TestOneNodeExpiredKey(t *testing.T) {
 }
 
 func TestControlKnobs(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -183,7 +179,6 @@ func TestControlKnobs(t *testing.T) {
 }
 
 func TestExpectedFeaturesLinked(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -205,7 +200,6 @@ func TestExpectedFeaturesLinked(t *testing.T) {
 }
 
 func TestCollectPanic(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n := NewTestNode(t, env)
@@ -248,7 +242,6 @@ func TestCollectPanic(t *testing.T) {
 }
 
 func TestControlTimeLogLine(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	env.LogCatcher.StoreRawJSON()
@@ -272,7 +265,6 @@ func TestControlTimeLogLine(t *testing.T) {
 
 // test Issue 2321: Start with UpdatePrefs should save prefs to disk
 func TestStateSavedOnStart(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -473,7 +465,6 @@ func TestOneNodeUpAuth(t *testing.T) {
 			},
 		},
 	} {
-		tstest.Shard(t)
 		t.Run(tt.name, func(t *testing.T) {
 			tstest.Parallel(t)
 
@@ -559,7 +550,6 @@ func isNonZeroExitCode(err error) bool {
 // If we interrupt `tailscale up` and then run it again, we should only
 // print a single auth URL.
 func TestOneNodeUpInterruptedAuth(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 
 	env := NewTestEnv(t, ConfigureControl(
@@ -638,7 +628,6 @@ func TestOneNodeUpInterruptedAuth(t *testing.T) {
 // complete the device approval, we should see the device approval URL
 // when we run `tailscale up` a second time.
 func TestOneNodeUpInterruptedDeviceApproval(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 
 	env := NewTestEnv(t, ConfigureControl(
@@ -718,8 +707,6 @@ func TestOneNodeUpInterruptedDeviceApproval(t *testing.T) {
 }
 
 func TestConfigFileAuthKey(t *testing.T) {
-	tstest.SkipOnUnshardedCI(t)
-	tstest.Shard(t)
 	t.Parallel()
 	const authKey = "opensesame"
 	env := NewTestEnv(t, ConfigureControl(func(control *testcontrol.Server) {
@@ -745,7 +732,6 @@ func TestConfigFileAuthKey(t *testing.T) {
 }
 
 func TestTwoNodes(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 
@@ -831,7 +817,6 @@ func TestTwoNodes(t *testing.T) {
 // tests two nodes where the first gets a incremental MapResponse (with only
 // PeersRemoved set) saying that the second node disappeared.
 func TestIncrementalMapUpdatePeersRemoved(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 
@@ -919,7 +904,6 @@ func TestIncrementalMapUpdatePeersRemoved(t *testing.T) {
 // This covers VIP additions at runtime, where the VIP route is not reachable
 // before the map mutation but is reachable over TSMP afterward.
 func TestIncrementalMapUpdatePeerAllowedIPsReachability(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 
@@ -1005,7 +989,6 @@ func TestIncrementalMapUpdatePeerAllowedIPsReachability(t *testing.T) {
 }
 
 func TestNodeAddressIPFields(t *testing.T) {
-	tstest.Shard(t)
 	flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/7008")
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
@@ -1033,7 +1016,6 @@ func TestNodeAddressIPFields(t *testing.T) {
 }
 
 func TestAddPingRequest(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -1086,7 +1068,6 @@ func TestAddPingRequest(t *testing.T) {
 }
 
 func TestC2NPingRequest(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 
 	env := NewTestEnv(t)
@@ -1148,7 +1129,6 @@ func TestC2NPingRequest(t *testing.T) {
 // Issue 2434: when "down" (WantRunning false), tailscaled shouldn't
 // be connected to control.
 func TestNoControlConnWhenDown(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -1197,7 +1177,6 @@ func TestNoControlConnWhenDown(t *testing.T) {
 // Issue 2137: make sure Windows tailscaled works with the CLI alone,
 // without the GUI to kick off a Start.
 func TestOneNodeUpWindowsStyle(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)
@@ -1218,7 +1197,6 @@ func TestOneNodeUpWindowsStyle(t *testing.T) {
 // node can initiate connections to the jailed node.
 func TestClientSideJailing(t *testing.T) {
 	flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/17419")
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	registerNode := func() (*TestNode, key.NodePublic) {
@@ -1331,7 +1309,6 @@ func TestClientSideJailing(t *testing.T) {
 // tries to do bi-directional pings between them.
 func TestNATPing(t *testing.T) {
 	flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/12169")
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	for _, v6 := range []bool{false, true} {
 		env := NewTestEnv(t)
@@ -1459,7 +1436,6 @@ func TestNATPing(t *testing.T) {
 }
 
 func TestLogoutRemovesAllPeers(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	// Spin up some nodes.
@@ -1521,7 +1497,6 @@ func TestAutoUpdateDefaults_cap(t *testing.T) { testAutoUpdateDefaults(t, true)
 func testAutoUpdateDefaults(t *testing.T, useCap bool) {
 	t.Cleanup(feature.HookCanAutoUpdate.SetForTest(func() bool { return true }))
 
-	tstest.Shard(t)
 	env := NewTestEnv(t)
 
 	var (
@@ -1654,7 +1629,6 @@ func testAutoUpdateDefaults(t *testing.T, useCap bool) {
 // gVisor/netstack.
 // https://github.com/tailscale/corp/issues/22511
 func TestDNSOverTCPIntervalResolver(t *testing.T) {
-	tstest.Shard(t)
 	tstest.RequireRoot(t)
 	env := NewTestEnv(t)
 	env.tunMode = true
@@ -1724,7 +1698,6 @@ func TestDNSOverTCPIntervalResolver(t *testing.T) {
 // TestNetstackTCPLoopback tests netstack loopback of a TCP stream, in both
 // directions.
 func TestNetstackTCPLoopback(t *testing.T) {
-	tstest.Shard(t)
 	tstest.RequireRoot(t)
 
 	env := NewTestEnv(t)
@@ -1864,7 +1837,6 @@ func TestNetstackTCPLoopback(t *testing.T) {
 // TestNetstackUDPLoopback tests netstack loopback of UDP packets, in both
 // directions.
 func TestNetstackUDPLoopback(t *testing.T) {
-	tstest.Shard(t)
 	tstest.RequireRoot(t)
 
 	env := NewTestEnv(t)
@@ -2013,7 +1985,6 @@ func TestEncryptStateMigration(t *testing.T) {
 	if runtime.GOOS != "linux" && runtime.GOOS != "windows" {
 		t.Skip("--encrypt-state for tailscaled state not supported on this platform")
 	}
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n := NewTestNode(t, env)
@@ -2069,7 +2040,6 @@ func TestEncryptStateMigration(t *testing.T) {
 // expected values.
 func TestPeerRelayPing(t *testing.T) {
 	flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/17251")
-	tstest.Shard(t)
 	tstest.Parallel(t)
 
 	env := NewTestEnv(t, ConfigureControl(func(server *testcontrol.Server) {
@@ -2209,7 +2179,6 @@ func TestPeerRelayPing(t *testing.T) {
 }
 
 func TestC2NDebugNetmap(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t, ConfigureControl(func(s *testcontrol.Server) {
 		s.CollectServices = opt.False
@@ -2352,7 +2321,6 @@ func TestTailnetLock(t *testing.T) {
 	// If you run `tailscale lock log` on a node where Tailnet Lock isn't
 	// enabled, you get an error explaining that.
 	t.Run("log-when-not-enabled", func(t *testing.T) {
-		tstest.Shard(t)
 		t.Parallel()
 
 		env := NewTestEnv(t)
@@ -2390,7 +2358,6 @@ func TestTailnetLock(t *testing.T) {
 	// the signed nodes can talk to each other but the unsigned node cannot
 	// talk to anybody.
 	t.Run("node-connectivity", func(t *testing.T) {
-		tstest.Shard(t)
 		t.Parallel()
 
 		env := NewTestEnv(t)
@@ -2466,7 +2433,6 @@ func TestTailnetLock(t *testing.T) {
 	t.Run("no-keys-is-error", func(t *testing.T) {
 		for _, verb := range []string{"add", "remove", "revoke-keys"} {
 			t.Run(verb, func(t *testing.T) {
-				tstest.Shard(t)
 				t.Parallel()
 
 				env := NewTestEnv(t)
@@ -2493,7 +2459,6 @@ func TestTailnetLock(t *testing.T) {
 }
 
 func TestNodeWithBadStateFile(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 	n1 := NewTestNode(t, env)

tstest/integration/whois_test.go

@@ -21,7 +21,6 @@ import (
 // netstack forwards the connection to localhost, and the listener
 // calls WhoIs on n2's LocalAPI to identify the remote peer as n1.
 func TestUserspaceWhoIsProxyMap(t *testing.T) {
-	tstest.Shard(t)
 	tstest.Parallel(t)
 	env := NewTestEnv(t)
 

tstest/largetailnet/delta_test.go

@@ -57,7 +57,6 @@ func metricByName(t testing.TB, name string) *clientmetric.Metric {
 // [tstest/largetailnet/BenchmarkGiantTailnet], which only measures cost
 // of the same fast path — this test verifies correctness.
 func TestNetmapDeltaFastPath(t *testing.T) {
-	tstest.Shard(t)
 
 	logf := logger.Discard
 	if testing.Verbose() {
@@ -105,7 +104,7 @@ func TestNetmapDeltaFastPath(t *testing.T) {
 	// Snapshot baseline metric values; we'll assert deltas against
 	// these. Globals make per-test isolation impossible, but deltas
 	// are robust against interleaving (assuming no other test runs in
-	// parallel here, hence tstest.Shard above).
+	// parallel here).
 	mFast := metricByName(t, "controlclient_map_response_handled_incrementally")
 	mFull := metricByName(t, "controlclient_map_response_handled_full_rebuild")
 	mUpsert := metricByName(t, "localbackend_netmap_delta_peer_upserted")

tstest/tstest.go

@@ -8,16 +8,12 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strconv"
-	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
 	"tailscale.com/envknob"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/backoff"
-	"tailscale.com/util/cibuild"
 )
 
 // AssertNotParallel asserts that t has not been marked as parallel.
@@ -48,7 +44,6 @@ func Replace[T any](t testing.TB, target *T, val T) {
 	})
 
 	*target = val
-	return
 }
 
 // WaitFor retries try for up to maxWait.
@@ -68,38 +63,6 @@ func WaitFor(maxWait time.Duration, try func() error) error {
 	return err
 }
 
-var testNum atomic.Int32
-
-// Shard skips t if it's not running if the TS_TEST_SHARD test shard is set to
-// "n/m" and this test execution number in the process mod m is not equal to n-1.
-// That is, to run with 4 shards, set TS_TEST_SHARD=1/4, ..., TS_TEST_SHARD=4/4
-// for the four jobs.
-func Shard(t testing.TB) {
-	e := os.Getenv("TS_TEST_SHARD")
-	a, b, ok := strings.Cut(e, "/")
-	if !ok {
-		return
-	}
-	wantShard, _ := strconv.ParseInt(a, 10, 32)
-	shards, _ := strconv.ParseInt(b, 10, 32)
-	if wantShard == 0 || shards == 0 {
-		return
-	}
-
-	shard := ((testNum.Add(1) - 1) % int32(shards)) + 1
-	if shard != int32(wantShard) {
-		t.Skipf("skipping shard %d/%d (process has TS_TEST_SHARD=%q)", shard, shards, e)
-	}
-}
-
-// SkipOnUnshardedCI skips t if we're in CI and the TS_TEST_SHARD
-// environment variable isn't set.
-func SkipOnUnshardedCI(t testing.TB) {
-	if cibuild.On() && os.Getenv("TS_TEST_SHARD") == "" {
-		t.Skip("skipping on CI without TS_TEST_SHARD")
-	}
-}
-
 var serializeParallel = envknob.RegisterBool("TS_SERIAL_TESTS")
 
 // Parallel calls t.Parallel, unless TS_SERIAL_TESTS is set true.