all: unify some redundant testing.TB interface copies

I added yet another one in 6d117d64a2 but that new one is at the
best place int he dependency graph and has the best name, so let's use
that one for everything possible.

types/lazy can't use it for circular dependency reasons, so unexport
that copy at least.

Updates #cleanup

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

cmd/stund/depaware.txt

@@ -80,6 +80,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar
         tailscale.com/util/nocasemaps                                from tailscale.com/types/ipproto
         tailscale.com/util/rands                                     from tailscale.com/tsweb
         tailscale.com/util/slicesx                                   from tailscale.com/tailcfg
+        tailscale.com/util/testenv                                   from tailscale.com/types/logger
         tailscale.com/util/vizerror                                  from tailscale.com/tailcfg+
         tailscale.com/version                                        from tailscale.com/envknob+
         tailscale.com/version/distro                                 from tailscale.com/envknob
@@ -186,7 +187,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar
         encoding/pem                                                 from crypto/tls+
         errors                                                       from bufio+
         expvar                                                       from github.com/prometheus/client_golang/prometheus+
-        flag                                                         from tailscale.com/cmd/stund
+        flag                                                         from tailscale.com/cmd/stund+
         fmt                                                          from compress/flate+
         go/token                                                     from google.golang.org/protobuf/internal/strs
         hash                                                         from crypto+

types/lazy/lazy.go

@@ -120,9 +120,9 @@ func (z *SyncValue[T]) PeekErr() (v T, err error, ok bool) {
 	return zero, nil, false
 }
 
-// TB is a subset of testing.TB that we use to set up test helpers.
+// testing_TB is a subset of testing.TB that we use to set up test helpers.
 // It's defined here to avoid pulling in the testing package.
-type TB interface {
+type testing_TB interface {
 	Helper()
 	Cleanup(func())
 }
@@ -132,7 +132,9 @@ type TB interface {
 // subtests complete.
 // It is not safe for concurrent use and must not be called concurrently with
 // any SyncValue methods, including another call to itself.
-func (z *SyncValue[T]) SetForTest(tb TB, val T, err error) {
+//
+// The provided tb should be a [*testing.T] or [*testing.B].
+func (z *SyncValue[T]) SetForTest(tb testing_TB, val T, err error) {
 	tb.Helper()
 
 	oldErr, oldVal := z.err.Load(), z.v

types/logger/logger.go

@@ -24,6 +24,7 @@ import (
 	"go4.org/mem"
 	"tailscale.com/envknob"
 	"tailscale.com/util/ctxkey"
+	"tailscale.com/util/testenv"
 )
 
 // Logf is the basic Tailscale logger type: a printf-like func.
@@ -384,16 +385,10 @@ func (a asJSONResult) Format(s fmt.State, verb rune) {
 	s.Write(v)
 }
 
-// TBLogger is the testing.TB subset needed by TestLogger.
-type TBLogger interface {
-	Helper()
-	Logf(format string, args ...any)
-}
-
 // TestLogger returns a logger that logs to tb.Logf
 // with a prefix to make it easier to distinguish spam
 // from explicit test failures.
-func TestLogger(tb TBLogger) Logf {
+func TestLogger(tb testenv.TB) Logf {
 	return func(format string, args ...any) {
 		tb.Helper()
 		tb.Logf("    ... "+format, args...)

util/syspolicy/handler.go

@@ -4,10 +4,10 @@
 package syspolicy
 
 import (
-	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/rsop"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
+	"tailscale.com/util/testenv"
 )
 
 // TODO(nickkhyl): delete this file once other repos are updated.
@@ -38,15 +38,11 @@ func RegisterHandler(h Handler) {
 	rsop.RegisterStore("DeviceHandler", setting.DeviceScope, WrapHandler(h))
 }
 
-// TB is a subset of testing.TB that we use to set up test helpers.
-// It's defined here to avoid pulling in the testing package.
-type TB = internal.TB
-
 // SetHandlerForTest wraps and sets the specified handler as the device's policy
 // [source.Store] for the duration of tb.
 //
 // Deprecated: using [MustRegisterStoreForTest] should be preferred.
-func SetHandlerForTest(tb TB, h Handler) {
+func SetHandlerForTest(tb testenv.TB, h Handler) {
 	RegisterWellKnownSettingsForTest(tb)
 	MustRegisterStoreForTest(tb, "DeviceHandler-TestOnly", setting.DefaultScope(), WrapHandler(h))
 }

util/syspolicy/internal/internal.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/go-json-experiment/json/jsontext"
 	"tailscale.com/types/lazy"
+	"tailscale.com/util/testenv"
 	"tailscale.com/version"
 )
 
@@ -25,22 +26,10 @@ func OS() string {
 	return OSForTesting.Get(version.OS)
 }
 
-// TB is a subset of testing.TB that we use to set up test helpers.
-// It's defined here to avoid pulling in the testing package.
-type TB interface {
-	Helper()
-	Cleanup(func())
-	Logf(format string, args ...any)
-	Error(args ...any)
-	Errorf(format string, args ...any)
-	Fatal(args ...any)
-	Fatalf(format string, args ...any)
-}
-
 // EqualJSONForTest compares the JSON in j1 and j2 for semantic equality.
 // It returns "", "", true if j1 and j2 are equal. Otherwise, it returns
 // indented versions of j1 and j2 and false.
-func EqualJSONForTest(tb TB, j1, j2 jsontext.Value) (s1, s2 string, equal bool) {
+func EqualJSONForTest(tb testenv.TB, j1, j2 jsontext.Value) (s1, s2 string, equal bool) {
 	tb.Helper()
 	j1 = j1.Clone()
 	j2 = j2.Clone()

util/syspolicy/internal/loggerx/logger.go

@@ -10,7 +10,7 @@ import (
 
 	"tailscale.com/types/lazy"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/syspolicy/internal"
+	"tailscale.com/util/testenv"
 )
 
 const (
@@ -58,7 +58,7 @@ func verbosef(format string, args ...any) {
 
 // SetForTest sets the specified printf and verbosef functions for the duration
 // of tb and its subtests.
-func SetForTest(tb internal.TB, printf, verbosef logger.Logf) {
+func SetForTest(tb testenv.TB, printf, verbosef logger.Logf) {
 	lazyPrintf.SetForTest(tb, printf, nil)
 	lazyVerbosef.SetForTest(tb, verbosef, nil)
 }

util/syspolicy/internal/metrics/metrics.go

@@ -259,7 +259,7 @@ var addMetricTestHook, setMetricTestHook syncs.AtomicValue[metricFn]
 
 // SetHooksForTest sets the specified addMetric and setMetric functions
 // as the metric functions for the duration of tb and all its subtests.
-func SetHooksForTest(tb internal.TB, addMetric, setMetric metricFn) {
+func SetHooksForTest(tb testenv.TB, addMetric, setMetric metricFn) {
 	oldAddMetric := addMetricTestHook.Swap(addMetric)
 	oldSetMetric := setMetricTestHook.Swap(setMetric)
 	tb.Cleanup(func() {

util/syspolicy/internal/metrics/test_handler.go

@@ -9,6 +9,7 @@ import (
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/set"
 	"tailscale.com/util/syspolicy/internal"
+	"tailscale.com/util/testenv"
 )
 
 // TestState represents a metric name and its expected value.
@@ -19,13 +20,13 @@ type TestState struct {
 
 // TestHandler facilitates testing of the code that uses metrics.
 type TestHandler struct {
-	t internal.TB
+	t testenv.TB
 
 	m map[string]int64
 }
 
 // NewTestHandler returns a new TestHandler.
-func NewTestHandler(t internal.TB) *TestHandler {
+func NewTestHandler(t testenv.TB) *TestHandler {
 	return &TestHandler{t, make(map[string]int64)}
 }
 

util/syspolicy/policy_keys.go

@@ -239,7 +239,7 @@ func WellKnownSettingDefinition(k Key) (*setting.Definition, error) {
 
 // RegisterWellKnownSettingsForTest registers all implicit setting definitions
 // for the duration of the test.
-func RegisterWellKnownSettingsForTest(tb TB) {
+func RegisterWellKnownSettingsForTest(tb testenv.TB) {
 	tb.Helper()
 	err := setting.SetDefinitionsForTest(tb, implicitDefinitions...)
 	if err != nil {

util/syspolicy/rsop/resultant_policy.go

@@ -11,9 +11,9 @@ import (
 	"sync/atomic"
 	"time"
 
-	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/setting"
+	"tailscale.com/util/testenv"
 
 	"tailscale.com/util/syspolicy/source"
 )
@@ -449,7 +449,7 @@ func (p *Policy) Close() {
 	}
 }
 
-func setForTest[T any](tb internal.TB, target *T, newValue T) {
+func setForTest[T any](tb testenv.TB, target *T, newValue T) {
 	oldValue := *target
 	tb.Cleanup(func() { *target = oldValue })
 	*target = newValue

util/syspolicy/rsop/store_registration.go

@@ -9,9 +9,9 @@ import (
 	"sync/atomic"
 	"time"
 
-	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
+	"tailscale.com/util/testenv"
 )
 
 // ErrAlreadyConsumed is the error returned when [StoreRegistration.ReplaceStore]
@@ -33,7 +33,7 @@ func RegisterStore(name string, scope setting.PolicyScope, store source.Store) (
 
 // RegisterStoreForTest is like [RegisterStore], but unregisters the store when
 // tb and all its subtests complete.
-func RegisterStoreForTest(tb internal.TB, name string, scope setting.PolicyScope, store source.Store) (*StoreRegistration, error) {
+func RegisterStoreForTest(tb testenv.TB, name string, scope setting.PolicyScope, store source.Store) (*StoreRegistration, error) {
 	setForTest(tb, &policyReloadMinDelay, 10*time.Millisecond)
 	setForTest(tb, &policyReloadMaxDelay, 500*time.Millisecond)
 

util/syspolicy/setting/setting.go

@@ -16,6 +16,7 @@ import (
 
 	"tailscale.com/types/lazy"
 	"tailscale.com/util/syspolicy/internal"
+	"tailscale.com/util/testenv"
 )
 
 // Scope indicates the broadest scope at which a policy setting may apply,
@@ -277,7 +278,7 @@ func DefinitionMapOf(settings []*Definition) (DefinitionMap, error) {
 // for the test duration. It is not concurrency-safe, but unlike [Register],
 // it does not panic and can be called anytime.
 // It returns an error if ds contains two different settings with the same [Key].
-func SetDefinitionsForTest(tb lazy.TB, ds ...*Definition) error {
+func SetDefinitionsForTest(tb testenv.TB, ds ...*Definition) error {
 	m, err := DefinitionMapOf(ds)
 	if err != nil {
 		return err

util/syspolicy/source/test_store.go

@@ -12,8 +12,8 @@ import (
 	"tailscale.com/util/mak"
 	"tailscale.com/util/set"
 	"tailscale.com/util/slicesx"
-	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/setting"
+	"tailscale.com/util/testenv"
 )
 
 var (
@@ -79,7 +79,7 @@ func (r TestExpectedReads) operation() testReadOperation {
 
 // TestStore is a [Store] that can be used in tests.
 type TestStore struct {
-	tb internal.TB
+	tb testenv.TB
 
 	done chan struct{}
 
@@ -98,7 +98,7 @@ type TestStore struct {
 
 // NewTestStore returns a new [TestStore].
 // The tb will be used to report coding errors detected by the [TestStore].
-func NewTestStore(tb internal.TB) *TestStore {
+func NewTestStore(tb testenv.TB) *TestStore {
 	m := make(map[setting.Key]any)
 	store := &TestStore{
 		tb:   tb,
@@ -112,7 +112,7 @@ func NewTestStore(tb internal.TB) *TestStore {
 
 // NewTestStoreOf is a shorthand for [NewTestStore] followed by [TestStore.SetBooleans],
 // [TestStore.SetUInt64s], [TestStore.SetStrings] or [TestStore.SetStringLists].
-func NewTestStoreOf[T TestValueType](tb internal.TB, settings ...TestSetting[T]) *TestStore {
+func NewTestStoreOf[T TestValueType](tb testenv.TB, settings ...TestSetting[T]) *TestStore {
 	store := NewTestStore(tb)
 	switch settings := any(settings).(type) {
 	case []TestSetting[bool]:

util/syspolicy/syspolicy.go

@@ -20,6 +20,7 @@ import (
 	"tailscale.com/util/syspolicy/rsop"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
+	"tailscale.com/util/testenv"
 )
 
 var (
@@ -46,7 +47,7 @@ func RegisterStore(name string, scope setting.PolicyScope, store source.Store) (
 }
 
 // MustRegisterStoreForTest is like [rsop.RegisterStoreForTest], but it fails the test if the store could not be registered.
-func MustRegisterStoreForTest(tb TB, name string, scope setting.PolicyScope, store source.Store) *rsop.StoreRegistration {
+func MustRegisterStoreForTest(tb testenv.TB, name string, scope setting.PolicyScope, store source.Store) *rsop.StoreRegistration {
 	tb.Helper()
 	reg, err := rsop.RegisterStoreForTest(tb, name, scope, store)
 	if err != nil {

util/syspolicy/syspolicy_test.go

@@ -14,6 +14,7 @@ import (
 	"tailscale.com/util/syspolicy/internal/metrics"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
+	"tailscale.com/util/testenv"
 )
 
 var someOtherError = errors.New("error other than not found")
@@ -596,7 +597,7 @@ func TestGetStringArray(t *testing.T) {
 	}
 }
 
-func registerSingleSettingStoreForTest[T source.TestValueType](tb TB, s source.TestSetting[T]) {
+func registerSingleSettingStoreForTest[T source.TestValueType](tb testenv.TB, s source.TestSetting[T]) {
 	policyStore := source.NewTestStoreOf(tb, s)
 	MustRegisterStoreForTest(tb, "TestStore", setting.DeviceScope, policyStore)
 }

util/syspolicy/syspolicy_windows.go

@@ -43,7 +43,7 @@ func init() {
 
 // configureSyspolicy configures syspolicy for use on Windows,
 // either in test or regular builds depending on whether tb has a non-nil value.
-func configureSyspolicy(tb internal.TB) error {
+func configureSyspolicy(tb testenv.TB) error {
 	const localSystemSID = "S-1-5-18"
 	// Always create and register a machine policy store that reads
 	// policy settings from the HKEY_LOCAL_MACHINE registry hive.