treewide: change test structure

peers/e2e_test.go

@@ -42,7 +42,9 @@ backend st_src_global
 		PeerAddr: l.Addr().String(),
 	}
 
-	t.Run("receive update", testutil.WithHAProxy(cfg, func(t *testing.T) {
+	t.Run("receive update", func(t *testing.T) {
+		cfg.Run(t)
+
 		for i := 0; i < 10; i++ {
 			_, err := http.Get("http://127.0.0.1:" + cfg.FrontendPort)
 			if err != nil {
@@ -60,5 +62,5 @@ backend st_src_global
 		case <-tm.C:
 			t.Error("timeout")
 		}
-	}))
+	})
 }

pkg/encoding/actionwriter_test.go

@@ -11,15 +11,15 @@ func TestActionWriter(t *testing.T) {
 	buf := make([]byte, 16386)
 
 	const exampleKey, exampleValue = "key", "value"
-	testutil.WithoutAllocations(func(t *testing.T) {
+	testutil.WithoutAllocations(t, func() {
 		aw := NewActionWriter(buf, 0)
 
 		if err := aw.SetString(VarScopeTransaction, exampleKey, exampleValue); err != nil {
 			t.Error(err)
 		}
 
 		buf = aw.Bytes()
-	})(t)
+	})
 
 	const expectedValue = "010302036b6579080576616c7565"
 	if s := fmt.Sprintf("%x", buf); s != expectedValue {

pkg/encoding/kvscanner.go

@@ -93,7 +93,7 @@ func (k *KVEntry) ValueBool() bool {
 func (k *KVEntry) ValueAddr() netip.Addr {
 	addr, ok := netip.AddrFromSlice(k.byteVal)
 	if !ok {
-		panic("invalid addr decode")
+		panic("invalid addr decode: " + fmt.Sprintf("%x", k.byteVal))
 	}
 	return addr
 }

pkg/encoding/kvwriter_test.go

@@ -11,15 +11,15 @@ func TestKVWriter(t *testing.T) {
 	buf := make([]byte, 16386)
 
 	const exampleKey, exampleValue = "key", "value"
-	testutil.WithoutAllocations(func(t *testing.T) {
+	testutil.WithoutAllocations(t, func() {
 		aw := NewKVWriter(buf, 0)
 
 		if err := aw.SetString(exampleKey, exampleValue); err != nil {
 			t.Error(err)
 		}
 
 		buf = aw.Bytes()
-	})(t)
+	})
 
 	const expectedValue = "036b6579080576616c7565"
 	if s := fmt.Sprintf("%x", buf); s != expectedValue {

pkg/testutil/allocs.go

@@ -6,23 +6,19 @@ import (
 
 const runsPerTest = 10
 
-func WithoutAllocations(fn func(*testing.T)) func(t *testing.T) {
-	return WithNAllocations(0, fn)
+func WithoutAllocations(tb testing.TB, fn func()) {
+	WithNAllocations(tb, 0, fn)
 }
 
-func WithNAllocations(n uint64, fn func(*testing.T)) func(t *testing.T) {
-	return func(t *testing.T) {
-		avg := testing.AllocsPerRun(runsPerTest, func() {
-			fn(t)
-		})
+func WithNAllocations(tb testing.TB, n uint64, fn func()) {
+	avg := testing.AllocsPerRun(runsPerTest, fn)
 
-		// early exit when failed
-		if t.Failed() {
-			return
-		}
+	// early exit when failed
+	if tb.Failed() {
+		return
+	}
 
-		if uint64(avg) != n {
-			t.Errorf("got %v allocs, want %d allocs", avg, n)
-		}
+	if uint64(avg) != n {
+		tb.Errorf("got %v allocs, want %d allocs", avg, n)
 	}
 }

pkg/testutil/allocs_test.go

@@ -5,14 +5,18 @@ import (
 )
 
 func TestAllocTests(t *testing.T) {
-	t.Run("without allocations", WithoutAllocations(func(t *testing.T) {
-		v := make([]byte, 10)
-		copy(v, []byte{1, 2, 3, 4})
-	}))
+	t.Run("without allocations", func(t *testing.T) {
+		WithoutAllocations(t, func() {
+			v := make([]byte, 10)
+			copy(v, []byte{1, 2, 3, 4})
+		})
+	})
 
-	t.Run("with allocation", WithNAllocations(1, func(t *testing.T) {
-		v := make([]byte, 10)
-		v = append(v, 1)
-		_ = v
-	}))
+	t.Run("with allocation", func(t *testing.T) {
+		WithNAllocations(t, 1, func() {
+			v := make([]byte, 10)
+			v = append(v, 1)
+			_ = v
+		})
+	})
 }

pkg/testutil/haproxy.go

@@ -28,6 +28,7 @@ defaults
 frontend test
     mode http
     bind 127.0.0.1:{{ .FrontendPort }}
+    bind unix@{{ .FrontendSocket }} accept-proxy
 
 {{- if .EngineConfigFile }}
     filter spoe engine e2e config {{ .EngineConfigFile }}
@@ -90,93 +91,102 @@ type HAProxyConfig struct {
 	CustomConfig         string
 }
 
-func mustExecuteTemplate(t *testing.T, text string, data any) string {
-	tmpl, err := template.New("").Parse(text)
-	if err != nil {
-		t.Fatal(err)
-	}
+func (cfg HAProxyConfig) Run(tb testing.TB) string {
+	tb.Helper()
 
-	var tmplBuf bytes.Buffer
-	if err := tmpl.Execute(&tmplBuf, data); err != nil {
-		t.Fatal(err)
+	if cfg.EngineConfig == "" {
+		cfg.EngineConfig = haproxyEngineConfig
 	}
 
-	return tmplBuf.String()
-}
-
-func WithHAProxy(cfg HAProxyConfig, f func(t *testing.T)) func(t *testing.T) {
-	return func(t *testing.T) {
-		if cfg.EngineConfig == "" {
-			cfg.EngineConfig = haproxyEngineConfig
-		}
-
-		if cfg.BackendConfig == "" {
-			cfg.BackendConfig = `
+	if cfg.BackendConfig == "" {
+		cfg.BackendConfig = `
 http-request return status 200 content-type "text/plain" string "Hello World!\n"
 `
-		}
+	}
+
+	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("haproxy_%s", cfg.FrontendPort))
+	if err != nil {
+		tb.Fatal(err)
+	}
+	tb.Cleanup(func() {
+		os.RemoveAll(tmpDir)
+	})
+
+	type tmplCfg struct {
+		HAProxyConfig
+
+		StatsSocket      string
+		FrontendSocket   string
+		InstanceID       string
+		LocalPeerAddr    string
+		EngineConfigFile string
+	}
+	var tcfg tmplCfg
+	tcfg.HAProxyConfig = cfg
+	tcfg.InstanceID = fmt.Sprintf("instance_%s", cfg.FrontendPort)
+	tcfg.LocalPeerAddr = fmt.Sprintf("127.0.0.1:%d", TCPPort(tb))
+	tcfg.StatsSocket = fmt.Sprintf("%s/stats.sock", tmpDir)
+	tcfg.FrontendSocket = fmt.Sprintf("%s/frontend.sock", tmpDir)
+
+	if cfg.EngineAddr != "" {
+		engineConfigFile := TempFile(tb, "e2e.cfg", cfg.EngineConfig)
+		tcfg.EngineConfigFile = engineConfigFile
+	}
 
-		type tmplCfg struct {
-			HAProxyConfig
+	haproxyConfig := mustExecuteTemplate(tb, haproxyConfigTemplate, tcfg)
+	haproxyConfigFile := TempFile(tb, "haproxy.cfg", haproxyConfig)
 
-			StatsSocket      string
-			InstanceID       string
-			LocalPeerAddr    string
-			EngineConfigFile string
-		}
-		var tcfg tmplCfg
-		tcfg.HAProxyConfig = cfg
-		tcfg.InstanceID = fmt.Sprintf("instance_%s", cfg.FrontendPort)
-		tcfg.LocalPeerAddr = fmt.Sprintf("127.0.0.1:%d", TCPPort(t))
-		tcfg.StatsSocket = fmt.Sprintf("%s/stats%s.sock", os.TempDir(), tcfg.FrontendPort)
-
-		if cfg.EngineAddr != "" {
-			engineConfigFile := TempFile(t, "e2e.cfg", cfg.EngineConfig)
-			tcfg.EngineConfigFile = engineConfigFile
-			defer os.Remove(engineConfigFile)
+	tb.Cleanup(func() {
+		if tb.Failed() {
+			tb.Logf("HAProxy Config: \n%s", haproxyConfig)
 		}
+	})
 
-		haproxyConfig := mustExecuteTemplate(t, haproxyConfigTemplate, tcfg)
-		haproxyConfigFile := TempFile(t, "haproxy.cfg", haproxyConfig)
-		defer os.Remove(haproxyConfigFile)
+	RunProcess(tb, "haproxy", []string{"-f", haproxyConfigFile, "-L", tcfg.InstanceID})
 
-		defer func() {
-			if t.Failed() {
-				t.Logf("HAProxy Config: \n%s", haproxyConfig)
-			}
-		}()
-
-		WithProcess("haproxy", []string{"-f", haproxyConfigFile, "-L", tcfg.InstanceID}, func(t *testing.T) {
-			waitOrTimeout(t, time.Second*3, func() {
-				for {
-					l, err := net.Dial("unix", tcfg.StatsSocket)
-					if err != nil {
-						continue
-					}
-					l.Close()
-
-					// if we were able to connect, exit and let the test run
-					break
-				}
-			})
-
-			f(t)
-		})(t)
-	}
-}
-
-func waitOrTimeout(t *testing.T, d time.Duration, f func()) {
 	c := make(chan bool)
 	defer close(c)
 
 	go func() {
-		f()
+		for {
+			l, err := net.Dial("unix", tcfg.StatsSocket)
+			if err != nil {
+				continue
+			}
+			l.Close()
+
+			l, err = net.Dial("unix", tcfg.FrontendSocket)
+			if err != nil {
+				continue
+			}
+			l.Close()
+
+			// if we were able to connect, exit and let the test run
+			break
+		}
 		c <- true
 	}()
 
 	select {
-	case <-time.After(d):
-		t.Fatal("timeout while waiting for haproxy")
+	case <-time.After(3 * time.Second):
+		tb.Fatal("timeout while waiting for haproxy")
 	case <-c:
 	}
+
+	return tcfg.FrontendSocket
+}
+
+func mustExecuteTemplate(tb testing.TB, text string, data any) string {
+	tb.Helper()
+	tmpl, err := template.New("").Parse(text)
+	if err != nil {
+		tb.Fatal(err)
+	}
+
+	var tmplBuf bytes.Buffer
+	if err := tmpl.Execute(&tmplBuf, data); err != nil {
+		tb.Fatal(err)
+	}
+
+	return tmplBuf.String()
 }

pkg/testutil/net.go

@@ -5,21 +5,23 @@ import (
 	"testing"
 )
 
-func TCPListener(t *testing.T) net.Listener {
+func TCPListener(tb testing.TB) net.Listener {
+	tb.Helper()
 	l, err := net.Listen("tcp", ":0")
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
-	t.Cleanup(func() {
+	tb.Cleanup(func() {
 		_ = l.Close()
 	})
 	return l
 }
 
-func TCPPort(t *testing.T) int {
+func TCPPort(tb testing.TB) int {
+	tb.Helper()
 	l, err := net.Listen("tcp", ":0")
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	defer l.Close()
 	return l.Addr().(*net.TCPAddr).Port

pkg/testutil/subprocess.go

@@ -7,41 +7,38 @@ import (
 	"testing"
 )
 
-func WithProcess(cmd string, args []string, f func(t *testing.T)) func(t *testing.T) {
+func RunProcess(tb testing.TB, cmd string, args []string) {
+	tb.Helper()
+
 	cmdString := cmd
 	if len(args) != 0 {
 		cmdString += " " + strings.Join(args, " ")
 	}
 
-	return func(t *testing.T) {
-		var stdout, stderr bytes.Buffer
-		c := exec.Command(cmd, args...)
-		c.Stdout = &stdout
-		c.Stderr = &stderr
-
-		defer func() {
-			if c.Process == nil {
-				return
-			}
+	var stdout, stderr bytes.Buffer
+	c := exec.Command(cmd, args...)
+	c.Stdout = &stdout
+	c.Stderr = &stderr
 
-			if err := c.Process.Kill(); err != nil {
-				t.Errorf("while killing: %q: %v", cmdString, err)
-			}
+	tb.Cleanup(func() {
+		if c.Process == nil {
+			return
+		}
 
-			// ignore the exit result
-			_ = c.Wait()
+		if err := c.Process.Kill(); err != nil {
+			tb.Errorf("while killing: %q: %v", cmdString, err)
+		}
 
-			if t.Failed() {
-				t.Logf("Subprocess %q stdout: \n%s", cmdString, stdout.String())
-				t.Logf("Subprocess %q stderr: \n%s", cmdString, stderr.String())
-			}
-		}()
+		// ignore the exit result
+		_ = c.Wait()
 
-		if err := c.Start(); err != nil {
-			t.Fatalf("while running subprocess: %q: %v", cmdString, err)
+		if tb.Failed() {
+			tb.Logf("Subprocess %q stdout: \n%s", cmdString, stdout.String())
+			tb.Logf("Subprocess %q stderr: \n%s", cmdString, stderr.String())
 		}
+	})
 
-		f(t)
-
+	if err := c.Start(); err != nil {
+		tb.Fatalf("while running subprocess: %q: %v", cmdString, err)
 	}
 }