net/portmapper: fix test flakes from logging after test done

Fixes tailscale#15794

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

Author: bradfitz

net/portmapper/igd_test.go

@@ -18,8 +18,10 @@ import (
 	"tailscale.com/net/netaddr"
 	"tailscale.com/net/netmon"
 	"tailscale.com/syncs"
+	"tailscale.com/tstest"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/eventbus"
+	"tailscale.com/util/testenv"
 )
 
 // TestIGD is an IGD (Internet Gateway Device) for testing. It supports fake
@@ -64,7 +66,8 @@ type igdCounters struct {
 	invalidPCPMapPkt int32
 }
 
-func NewTestIGD(logf logger.Logf, t TestIGDOptions) (*TestIGD, error) {
+func NewTestIGD(tb testenv.TB, t TestIGDOptions) (*TestIGD, error) {
+	logf := tstest.WhileTestRunningLogger(tb)
 	d := &TestIGD{
 		doPMP:  t.PMP,
 		doPCP:  t.PCP,
@@ -265,7 +268,7 @@ func (d *TestIGD) handlePCPQuery(pkt []byte, src netip.AddrPort) {
 func newTestClient(t *testing.T, igd *TestIGD, bus *eventbus.Bus) *Client {
 	var c *Client
 	c = NewClient(Config{
-		Logf:         t.Logf,
+		Logf:         tstest.WhileTestRunningLogger(t),
 		NetMon:       netmon.NewStatic(),
 		ControlKnobs: new(controlknobs.Knobs),
 		EventBus:     bus,

net/portmapper/portmapper_test.go

@@ -61,7 +61,7 @@ func TestClientProbeThenMap(t *testing.T) {
 }
 
 func TestProbeIntegration(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{PMP: true, PCP: true, UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{PMP: true, PCP: true, UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -95,7 +95,7 @@ func TestProbeIntegration(t *testing.T) {
 }
 
 func TestPCPIntegration(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{PMP: false, PCP: true, UPnP: false})
+	igd, err := NewTestIGD(t, TestIGDOptions{PMP: false, PCP: true, UPnP: false})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -137,7 +137,7 @@ func TestGetUPnPErrorsMetric(t *testing.T) {
 }
 
 func TestUpdateEvent(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{PCP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{PCP: true})
 	if err != nil {
 		t.Fatalf("Create test gateway: %v", err)
 	}

net/portmapper/select_test.go

@@ -28,7 +28,7 @@ func TestSelectBestService(t *testing.T) {
 	}
 
 	// Run a fake IGD server to respond to UPnP requests.
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}

net/portmapper/upnp_test.go

@@ -533,7 +533,7 @@ func TestGetUPnPClient(t *testing.T) {
 }
 
 func TestGetUPnPPortMapping(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -672,7 +672,7 @@ func TestGetUPnPPortMapping_LeaseDuration(t *testing.T) {
 				"DeletePortMapping":    "", // Do nothing for test
 			}
 
-			igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+			igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -722,7 +722,7 @@ func TestGetUPnPPortMapping_LeaseDuration(t *testing.T) {
 //
 // See https://github.com/tailscale/tailscale/issues/10911
 func TestGetUPnPPortMapping_NoValidServices(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -753,7 +753,7 @@ func TestGetUPnPPortMapping_NoValidServices(t *testing.T) {
 
 // Tests the legacy behaviour with the pre-UPnP standard portmapping service.
 func TestGetUPnPPortMapping_Legacy(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -796,7 +796,7 @@ func TestGetUPnPPortMapping_Legacy(t *testing.T) {
 }
 
 func TestGetUPnPPortMappingNoResponses(t *testing.T) {
-	igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -912,7 +912,7 @@ func TestGetUPnPPortMapping_Invalid(t *testing.T) {
 		"127.0.0.1",
 	} {
 		t.Run(responseAddr, func(t *testing.T) {
-			igd, err := NewTestIGD(t.Logf, TestIGDOptions{UPnP: true})
+			igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 			if err != nil {
 				t.Fatal(err)
 			}

tstest/log.go

@@ -13,6 +13,7 @@ import (
 
 	"go4.org/mem"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/testenv"
 )
 
 type testLogWriter struct {
@@ -149,7 +150,7 @@ func (ml *MemLogger) String() string {
 
 // WhileTestRunningLogger returns a logger.Logf that logs to t.Logf until the
 // test finishes, at which point it no longer logs anything.
-func WhileTestRunningLogger(t testing.TB) logger.Logf {
+func WhileTestRunningLogger(t testenv.TB) logger.Logf {
 	var (
 		mu   sync.RWMutex
 		done bool