all: update tests to use tstest.MemLogger

And give MemLogger a mutex, as one caller had, which does match the logf
contract better.

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

cmd/tailscale/cli/cli_test.go

@@ -17,6 +17,7 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/tstest"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/preftype"
 )
@@ -607,10 +608,7 @@ func TestPrefsFromUpArgs(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var warnBuf bytes.Buffer
+			var warnBuf tstest.MemLogger
-			warnf := func(format string, a ...interface{}) {
-				fmt.Fprintf(&warnBuf, format, a...)
-			}
 			goos := tt.goos
 			if goos == "" {
 				goos = "linux"
@@ -619,7 +617,7 @@ func TestPrefsFromUpArgs(t *testing.T) {
 			if st == nil {
 				st = new(ipnstate.Status)
 			}
-			got, err := prefsFromUpArgs(tt.args, warnf, st, goos)
+			got, err := prefsFromUpArgs(tt.args, warnBuf.Logf, st, goos)
 			gotErr := fmt.Sprint(err)
 			if tt.wantErr != "" {
 				if tt.wantErr != gotErr {

ipn/ipnlocal/peerapi_test.go

@@ -20,16 +20,13 @@ import (
 	"testing"
 
 	"tailscale.com/tailcfg"
+	"tailscale.com/tstest"
 )
 
 type peerAPITestEnv struct {
 	ph     *peerAPIHandler
 	rr     *httptest.ResponseRecorder
-	logBuf bytes.Buffer
+	logBuf tstest.MemLogger
-}
-
-func (e *peerAPITestEnv) logf(format string, a ...interface{}) {
-	fmt.Fprintf(&e.logBuf, format, a...)
 }
 
 type check func(*testing.T, *peerAPITestEnv)
@@ -403,7 +400,7 @@ func TestHandlePeerAPI(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			var e peerAPITestEnv
 			lb := &LocalBackend{
-				logf:           e.logf,
+				logf:           e.logBuf.Logf,
 				capFileSharing: tt.capSharing,
 			}
 			e.ph = &peerAPIHandler{

net/dns/manager_linux_test.go

@@ -5,14 +5,13 @@
 package dns
 
 import (
-	"bytes"
 	"errors"
-	"fmt"
 	"io/fs"
 	"os"
 	"strings"
 	"testing"
 
+	"tailscale.com/tstest"
 	"tailscale.com/util/cmpver"
 )
 
@@ -146,12 +145,8 @@ func TestLinuxDNSMode(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var logBuf bytes.Buffer
+			var logBuf tstest.MemLogger
-			logf := func(format string, a ...interface{}) {
+			got, err := dnsMode(logBuf.Logf, tt.env)
-				fmt.Fprintf(&logBuf, format, a...)
-				logBuf.WriteByte('\n')
-			}
-			got, err := dnsMode(logf, tt.env)
 			if err != nil {
 				t.Fatal(err)
 			}

net/portmapper/upnp_test.go

@@ -5,7 +5,6 @@
 package portmapper
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -17,6 +16,7 @@ import (
 	"testing"
 
 	"inet.af/netaddr"
+	"tailscale.com/tstest"
 )
 
 // Google Wifi
@@ -97,12 +97,8 @@ func TestGetUPnPClient(t *testing.T) {
 			}))
 			defer ts.Close()
 			gw, _ := netaddr.FromStdIP(ts.Listener.Addr().(*net.TCPAddr).IP)
-			var logBuf bytes.Buffer
+			var logBuf tstest.MemLogger
-			logf := func(format string, a ...interface{}) {
+			c, err := getUPnPClient(context.Background(), logBuf.Logf, gw, uPnPDiscoResponse{
-				fmt.Fprintf(&logBuf, format, a...)
-				logBuf.WriteByte('\n')
-			}
-			c, err := getUPnPClient(context.Background(), logf, gw, uPnPDiscoResponse{
 				Location: ts.URL + "/rootDesc.xml",
 			})
 			if err != nil {

tstest/log.go

@@ -127,10 +127,13 @@ func (lt *LogLineTracker) Close() {
 // MemLogger is a bytes.Buffer with a Logf method for tests that want
 // to log to a buffer.
 type MemLogger struct {
+	sync.Mutex
 	bytes.Buffer
 }
 
 func (ml *MemLogger) Logf(format string, args ...interface{}) {
+	ml.Lock()
+	defer ml.Unlock()
 	fmt.Fprintf(&ml.Buffer, format, args...)
 	if !mem.HasSuffix(mem.B(ml.Buffer.Bytes()), mem.S("\n")) {
 		ml.Buffer.WriteByte('\n')

wgengine/magicsock/magicsock_test.go

@@ -1315,26 +1315,6 @@ func BenchmarkReceiveFrom_Native(b *testing.B) {
 	}
 }
 
-type bufLog struct {
-	sync.Mutex
-	buf bytes.Buffer
-}
-
-func (b *bufLog) Logf(format string, args ...interface{}) {
-	b.Lock()
-	defer b.Unlock()
-	fmt.Fprintf(&b.buf, format, args...)
-	if !bytes.HasPrefix(b.buf.Bytes(), []byte("\n")) {
-		b.buf.WriteByte('\n')
-	}
-}
-
-func (b *bufLog) String() string {
-	b.Lock()
-	defer b.Unlock()
-	return b.buf.String()
-}
-
 // Test that a netmap update where node changes its node key but
 // doesn't change its disco key doesn't result in a broken state.
 //
@@ -1342,7 +1322,7 @@ func (b *bufLog) String() string {
 func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 	conn := newTestConn(t)
 	t.Cleanup(func() { conn.Close() })
-	var buf bufLog
+	var buf tstest.MemLogger
 	conn.logf = buf.Logf
 
 	conn.SetPrivateKey(wgkey.Private{0: 1})
@@ -1401,7 +1381,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 func TestRebindStress(t *testing.T) {
 	conn := newTestConn(t)
 
-	var buf bufLog
+	var buf tstest.MemLogger
 	conn.logf = buf.Logf
 
 	closed := false