test: Use modern testing features instead of custom helpers

Many of our tests rely on helpers that predate features which are now
available in the standard library.

This patch replaces them with the standard equivalents:

- MustTempDir/RemoveIfOk -> t.ArtifactDir() or t.TempDir() (depending on
  whether it is useful to keep the data on request).
- GetFreePort + waitForServer -> opening the listeners before starting
  the servers. To support this, localrpc.Server gains a Serve method. We
  may re-introduce these in the future if more use cases appear.
- WaitFor and real-time timing checks -> testing/synctest.
- context.Background() -> t.Context().
- Benchmark b.N loops -> b.Loop().
- reflect.DeepEqual -> cmp.Diff, and proto.Equal for protobufs.

.gitignore

@@ -40,3 +40,6 @@ chasquid.test
 # Exclude any .pem files, to prevent accidentally including test keys and
 # certificates.
 *.pem
+
+# Test artifacts, saved when running "go test -artifacts".
+_artifacts/

chasquid_test.go

@@ -21,8 +21,7 @@ func mustMkdir(t *testing.T, path string) string {
 }
 
 func TestFindCertKey(t *testing.T) {
-	tmpDir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	tmpDir := t.ArtifactDir()
 
 	cases := []struct {
 		name  string

internal/aliases/aliases_test.go

@@ -5,11 +5,11 @@ import (
 	"errors"
 	"os"
 	"os/exec"
-	"reflect"
 	"strings"
 	"testing"
 
 	"blitiri.com.ar/go/chasquid/internal/trace"
+	"github.com/google/go-cmp/cmp"
 )
 
 type Cases []struct {
@@ -29,9 +29,8 @@ func (cases Cases) check(t *testing.T, r *Resolver) {
 			t.Errorf("case %q: expected error %v, got %v",
 				c.addr, c.err, err)
 		}
-		if !reflect.DeepEqual(got, c.expect) {
-			t.Errorf("case %q: got %+v, expected %+v",
-				c.addr, got, c.expect)
+		if diff := cmp.Diff(c.expect, got); diff != "" {
+			t.Errorf("case %q: mismatch (-want +got):\n%s", c.addr, diff)
 		}
 	}
 }
@@ -507,7 +506,8 @@ func TestTooMuchRecursionOnCatchAll(t *testing.T) {
 }
 
 func mustWriteFile(t *testing.T, content string) string {
-	f, err := os.CreateTemp("", "aliases_test")
+	t.Helper()
+	f, err := os.CreateTemp(t.TempDir(), "aliases_test")
 	if err != nil {
 		t.Fatalf("failed to get temp file: %v", err)
 	}
@@ -555,7 +555,6 @@ func TestAddFile(t *testing.T) {
 
 	for _, c := range cases {
 		fname := mustWriteFile(t, c.contents)
-		defer os.Remove(fname)
 
 		resolver := NewResolver(allUsersExist)
 		_, err := resolver.AddAliasesFile("dom", fname)
@@ -568,8 +567,8 @@ func TestAddFile(t *testing.T) {
 			t.Errorf("case %q, got error: %v", c.contents, err)
 			continue
 		}
-		if !reflect.DeepEqual(got, c.expected) {
-			t.Errorf("case %q, got %v, expected %v", c.contents, got, c.expected)
+		if diff := cmp.Diff(c.expected, got); diff != "" {
+			t.Errorf("case %q: mismatch (-want +got):\n%s", c.contents, diff)
 		}
 	}
 
@@ -590,7 +589,6 @@ func TestAddFile(t *testing.T) {
 
 	for _, c := range errcases {
 		fname := mustWriteFile(t, c.contents)
-		defer os.Remove(fname)
 
 		resolver := NewResolver(allUsersExist)
 		_, err := resolver.AddAliasesFile("dom", fname)
@@ -637,7 +635,6 @@ y: z`
 
 func TestRichFile(t *testing.T) {
 	fname := mustWriteFile(t, richFileContents)
-	defer os.Remove(fname)
 
 	resolver := NewResolver(allUsersExist)
 	resolver.DropChars = "."
@@ -694,9 +691,6 @@ func TestManyFiles(t *testing.T) {
 		"xd1": mustWriteFile(t, "a: b@xd2"),
 		"xd2": mustWriteFile(t, "b: |cmd"),
 	}
-	for _, fname := range files {
-		defer os.Remove(fname)
-	}
 
 	resolver := NewResolver(allUsersExist)
 	for domain, fname := range files {
@@ -804,8 +798,7 @@ func TestHook(t *testing.T) {
 	if len(rcpts) != 0 {
 		t.Errorf("expected no recipients, got %v", rcpts)
 	}
-	execErr := &exec.ExitError{}
-	if !errors.As(err, &execErr) {
+	if _, ok := errors.AsType[*exec.ExitError](err); !ok {
 		t.Errorf("expected *exec.ExitError, got %T - %v", err, err)
 	}
 }
@@ -846,8 +839,8 @@ func TestParseForward(t *testing.T) {
 		if addr != c.addr {
 			t.Errorf("case %q: got addr %q, expected %q", c.raw, addr, c.addr)
 		}
-		if !reflect.DeepEqual(via, c.via) {
-			t.Errorf("case %q: got via %q, expected %q", c.raw, via, c.via)
+		if diff := cmp.Diff(c.via, via); diff != "" {
+			t.Errorf("case %q: via mismatch (-want +got):\n%s", c.raw, diff)
 		}
 	}
 }

internal/auth/auth_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/dovecot"
@@ -59,6 +60,7 @@ func TestDecodeResponse(t *testing.T) {
 }
 
 func TestAuthenticate(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
 		db := userdb.New("/dev/null")
 		db.AddUser("user", "password")
 		tr := trace.New("test", "TestAuthenticate")
@@ -67,10 +69,6 @@ func TestAuthenticate(t *testing.T) {
 		a := NewAuthenticator()
 		a.Register("domain", WrapNoErrorBackend(db))
 
-	// Shorten the duration to speed up the test. This should still be long
-	// enough for it to fail if we don't sleep intentionally.
-	a.AuthDuration = 20 * time.Millisecond
-
 		// Test the correct case first
 		check(t, a, "user", "domain", "password", true)
 
@@ -96,6 +94,7 @@ func TestAuthenticate(t *testing.T) {
 		for _, c := range cases {
 			check(t, a, c.user, c.domain, c.password, false)
 		}
+	})
 }
 
 func check(t *testing.T, a *Authenticator, user, domain, passwd string, expect bool) {
@@ -173,6 +172,7 @@ func (d *TestBE) Reload() error {
 }
 
 func TestMultipleBackends(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
 		domain1 := NewTestBE()
 		domain2 := NewTestBE()
 		fallback := NewTestBE()
@@ -182,10 +182,6 @@ func TestMultipleBackends(t *testing.T) {
 		a.Register("domain2", domain2)
 		a.Fallback = fallback
 
-	// Shorten the duration to speed up the test. This should still be long
-	// enough for it to fail if we don't sleep intentionally.
-	a.AuthDuration = 20 * time.Millisecond
-
 		domain1.add("user1", "passwd1")
 		domain2.add("user2", "passwd2")
 		fallback.add("user3@fallback", "passwd3")
@@ -212,6 +208,7 @@ func TestMultipleBackends(t *testing.T) {
 		for _, c := range cases {
 			check(t, a, c.user, c.domain, c.password, false)
 		}
+	})
 }
 
 func TestErrors(t *testing.T) {

internal/config/config_test.go

@@ -5,22 +5,21 @@ import (
 	"os"
 	"testing"
 
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 	"blitiri.com.ar/go/log"
 	"github.com/google/go-cmp/cmp"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/testing/protocmp"
 )
 
-func mustCreateConfig(t *testing.T, contents string) (string, string) {
-	tmpDir := testlib.MustTempDir(t)
-	confStr := []byte(contents)
-	err := os.WriteFile(tmpDir+"/chasquid.conf", confStr, 0600)
+func mustCreateConfig(t *testing.T, contents string) string {
+	t.Helper()
+	path := t.ArtifactDir() + "/chasquid.conf"
+	err := os.WriteFile(path, []byte(contents), 0600)
 	if err != nil {
 		t.Fatalf("Failed to write tmp config: %v", err)
 	}
 
-	return tmpDir, tmpDir + "/chasquid.conf"
+	return path
 }
 
 func TestEmptyStruct(t *testing.T) {
@@ -28,8 +27,7 @@ func TestEmptyStruct(t *testing.T) {
 }
 
 func TestEmptyConfig(t *testing.T) {
-	tmpDir, path := mustCreateConfig(t, "")
-	defer testlib.RemoveIfOk(t, tmpDir)
+	path := mustCreateConfig(t, "")
 	c, err := Load(path, "")
 	if err != nil {
 		t.Fatalf("error loading empty config: %v", err)
@@ -61,8 +59,7 @@ func TestFullConfig(t *testing.T) {
 		max_queue_items: 345
 	`
 
-	tmpDir, path := mustCreateConfig(t, confStr)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	path := mustCreateConfig(t, confStr)
 
 	overrideStr := `
 		hostname: "proust"
@@ -118,9 +115,8 @@ func TestErrorLoading(t *testing.T) {
 }
 
 func TestBrokenConfig(t *testing.T) {
-	tmpDir, path := mustCreateConfig(
+	path := mustCreateConfig(
 		t, "<invalid> this is not a valid protobuf")
-	defer testlib.RemoveIfOk(t, tmpDir)
 
 	c, err := Load(path, "")
 	if err == nil {
@@ -129,9 +125,8 @@ func TestBrokenConfig(t *testing.T) {
 }
 
 func TestBrokenOverride(t *testing.T) {
-	tmpDir, path := mustCreateConfig(
+	path := mustCreateConfig(
 		t, `hostname: "test"`)
-	defer testlib.RemoveIfOk(t, tmpDir)
 
 	c, err := Load(path, "broken override")
 	if err == nil {
@@ -140,9 +135,8 @@ func TestBrokenOverride(t *testing.T) {
 }
 
 func TestInvalidGiveUpSendingAfter(t *testing.T) {
-	tmpDir, path := mustCreateConfig(
+	path := mustCreateConfig(
 		t, `give_up_send_after: "10"`)
-	defer testlib.RemoveIfOk(t, tmpDir)
 
 	c, err := Load(path, "")
 	if err == nil {

internal/courier/fakeserver_test.go

@@ -27,7 +27,7 @@ type FakeServer struct {
 func newFakeServer(t *testing.T, responses map[string]string, conns int) *FakeServer {
 	s := &FakeServer{
 		t:         t,
-		tmpDir:    testlib.MustTempDir(t),
+		tmpDir:    t.ArtifactDir(),
 		responses: responses,
 		conns:     conns,
 		wg:        &sync.WaitGroup{},
@@ -36,14 +36,6 @@ func newFakeServer(t *testing.T, responses map[string]string, conns int) *FakeSe
 	return s
 }
 
-func (s *FakeServer) Cleanup() {
-	// Remove our temporary data. Be extra paranoid and make sure the
-	// directory isn't too shallow.
-	if len(s.tmpDir) > 8 {
-		os.RemoveAll(s.tmpDir)
-	}
-}
-
 func (s *FakeServer) initTLS() {
 	var err error
 	s.tlsConfig, err = testlib.GenerateCert(s.tmpDir)
@@ -84,11 +76,7 @@ func (s *FakeServer) start() string {
 
 	s.initTLS()
 
-	s.wg.Add(s.conns)
-
 	accept := func() {
-		defer s.wg.Done()
-
 		c, err := l.Accept()
 		if err != nil {
 			panic(err)
@@ -137,8 +125,8 @@ func (s *FakeServer) start() string {
 		}
 	}
 
-	for i := 0; i < s.conns; i++ {
-		go accept()
+	for range s.conns {
+		s.wg.Go(accept)
 	}
 
 	return s.addr

internal/courier/mda_test.go

@@ -7,13 +7,10 @@ import (
 	"os"
 	"testing"
 	"time"
-
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 )
 
 func TestMDA(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 
 	p := MDA{
 		Binary:  "tee",

internal/courier/smtp_test.go

@@ -9,7 +9,6 @@ import (
 
 	"blitiri.com.ar/go/chasquid/internal/domaininfo"
 	"blitiri.com.ar/go/chasquid/internal/sts"
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 	"blitiri.com.ar/go/chasquid/internal/trace"
 )
 
@@ -27,14 +26,13 @@ func init() {
 	}
 }
 
-func newSMTP(t *testing.T) (*SMTP, string) {
-	dir := testlib.MustTempDir(t)
-	dinfo, err := domaininfo.New(dir)
+func newSMTP(t *testing.T) *SMTP {
+	dinfo, err := domaininfo.New(t.ArtifactDir())
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	return &SMTP{"hello", dinfo, nil}, dir
+	return &SMTP{"hello", dinfo, nil}
 }
 
 func TestSMTP(t *testing.T) {
@@ -52,7 +50,6 @@ func TestSMTP(t *testing.T) {
 		"QUIT":              "250 quit ok\n",
 	}
 	srv := newFakeServer(t, responses, 1)
-	defer srv.Cleanup()
 	host, port := srv.HostPort()
 
 	// Put a non-existing host first, so we check that if the first host
@@ -67,8 +64,7 @@ func TestSMTP(t *testing.T) {
 	}
 	*smtpPort = port
 
-	s, tmpDir := newSMTP(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	s := newSMTP(t)
 	err, _ := s.Deliver("me@me", "to@to", []byte("data"))
 	if err != nil {
 		t.Errorf("deliver failed: %v", err)
@@ -125,14 +121,12 @@ func TestSMTPErrors(t *testing.T) {
 
 	for _, rs := range responses {
 		srv := newFakeServer(t, rs, 1)
-		defer srv.Cleanup()
 		host, port := srv.HostPort()
 
 		testMX["to"] = []*net.MX{{Host: host, Pref: 10}}
 		*smtpPort = port
 
-		s, tmpDir := newSMTP(t)
-		defer testlib.RemoveIfOk(t, tmpDir)
+		s := newSMTP(t)
 		err, _ := s.Deliver("me@me", "to@to", []byte("data"))
 		if err == nil {
 			t.Errorf("deliver not failed in case %q: %v", rs["_welcome"], err)
@@ -146,8 +140,7 @@ func TestSMTPErrors(t *testing.T) {
 func TestNoMXServer(t *testing.T) {
 	testMX["to"] = []*net.MX{}
 
-	s, tmpDir := newSMTP(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	s := newSMTP(t)
 	err, permanent := s.Deliver("me@me", "to@to", []byte("data"))
 	if err == nil {
 		t.Errorf("delivery worked, expected failure")
@@ -258,15 +251,13 @@ var tlsResponses = map[string]string{
 func TestTLS(t *testing.T) {
 	smtpTotalTimeout = 5 * time.Second
 	srv := newFakeServer(t, tlsResponses, 1)
-	defer srv.Cleanup()
 	_, *smtpPort = srv.HostPort()
 
 	testMX["to"] = []*net.MX{
 		{Host: "localhost", Pref: 20},
 	}
 
-	s, tmpDir := newSMTP(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	s := newSMTP(t)
 	err, _ := s.Deliver("me@me", "to@to", []byte("data"))
 	if err != nil {
 		t.Errorf("deliver failed: %v", err)
@@ -286,7 +277,6 @@ func TestTLS(t *testing.T) {
 		"QUIT":              "250 quit ok\n",
 	}
 	srv = newFakeServer(t, responses, 1)
-	defer srv.Cleanup()
 	_, *smtpPort = srv.HostPort()
 
 	err, permanent := s.Deliver("me@me", "to@to", []byte("data"))
@@ -326,15 +316,13 @@ func TestTLSError(t *testing.T) {
 	// after the failed STARTTLS). Note this also checks that we correctly
 	// close the errored connection, instead of leaving it lingering.
 	srv := newFakeServer(t, responses, 2)
-	defer srv.Cleanup()
 	_, *smtpPort = srv.HostPort()
 
 	testMX["to"] = []*net.MX{
 		{Host: "localhost", Pref: 20},
 	}
 
-	s, tmpDir := newSMTP(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	s := newSMTP(t)
 	err, _ := s.Deliver("me@me", "to@to", []byte("data"))
 	if err != nil {
 		t.Errorf("deliver failed: %v", err)
@@ -353,11 +341,9 @@ func TestTLSError(t *testing.T) {
 func TestSTSPolicyEnforcement(t *testing.T) {
 	smtpTotalTimeout = 5 * time.Second
 	srv := newFakeServer(t, tlsResponses, 1)
-	defer srv.Cleanup()
 	_, *smtpPort = srv.HostPort()
 
-	s, tmpDir := newSMTP(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	s := newSMTP(t)
 
 	a := &attempt{
 		courier:  s,
@@ -393,7 +379,6 @@ func TestSTSPolicyEnforcement(t *testing.T) {
 	// be TLS_SECURE which is required by the STS policy.
 	srv = newFakeServer(t, tlsResponses, 1)
 	_, *smtpPort = srv.HostPort()
-	defer srv.Cleanup()
 
 	certRoots = srv.rootCA()
 	defer func() {

internal/dkim/context_test.go

@@ -10,7 +10,7 @@ import (
 func TestTraceNoCtx(t *testing.T) {
 	// Call trace() on a context without a trace function, to check it doesn't
 	// panic.
-	ctx := context.Background()
+	ctx := t.Context()
 	trace(ctx, "test")
 }
 
@@ -19,7 +19,7 @@ func TestTrace(t *testing.T) {
 	traceF := func(f string, a ...any) {
 		s = fmt.Sprintf(f, a...)
 	}
-	ctx := WithTraceFunc(context.Background(), traceF)
+	ctx := WithTraceFunc(t.Context(), traceF)
 	trace(ctx, "test %d", 1)
 	if s != "test 1" {
 		t.Errorf("trace function not called")
@@ -32,7 +32,7 @@ func TestLookupTXTNoCtx(t *testing.T) {
 	// We just check there is a reasonable error.
 	// We don't specifically check that it's NXDOMAIN because if we don't have
 	// internet access, the error may be different.
-	ctx := context.Background()
+	ctx := t.Context()
 	_, err := lookupTXT(ctx, "does.not.exist.example.com")
 	if _, ok := err.(*net.DNSError); !ok {
 		t.Fatalf("expected *net.DNSError, got %T", err)
@@ -45,7 +45,7 @@ func TestLookupTXT(t *testing.T) {
 		called = true
 		return nil, nil
 	}
-	ctx := WithLookupTXTFunc(context.Background(), lookupTXTF)
+	ctx := WithLookupTXTFunc(t.Context(), lookupTXTF)
 	lookupTXT(ctx, "example.com")
 	if !called {
 		t.Errorf("lookupTXT function not called")
@@ -54,7 +54,7 @@ func TestLookupTXT(t *testing.T) {
 
 func TestMaxHeaders(t *testing.T) {
 	// First without an override, check we return the default.
-	ctx := context.Background()
+	ctx := t.Context()
 	if m := maxHeaders(ctx); m != 5 {
 		t.Errorf("expected 5, got %d", m)
 	}

internal/dkim/dns_test.go

@@ -16,7 +16,7 @@ func TestLookupError(t *testing.T) {
 	errLookupF := func(ctx context.Context, name string) ([]string, error) {
 		return nil, testErr
 	}
-	ctx := WithLookupTXTFunc(context.Background(), errLookupF)
+	ctx := WithLookupTXTFunc(t.Context(), errLookupF)
 
 	pks, err := findPublicKeys(ctx, "example.com", "selector")
 	if pks != nil || err != testErr {
@@ -50,7 +50,7 @@ func testLookupTXT(ctx context.Context, name string) ([]string, error) {
 }
 
 func TestSkipBadRecords(t *testing.T) {
-	ctx := WithLookupTXTFunc(context.Background(), testLookupTXT)
+	ctx := WithLookupTXTFunc(t.Context(), testLookupTXT)
 	results["selector._domainkey.example.com"] = []string{
 		"not a tag",
 		"v=DKIM1; p=" + exampleRSAKeyB64,

internal/dkim/file_test.go

@@ -44,7 +44,7 @@ func TestFromPrivateFiles(t *testing.T) {
 }
 
 func testOne(t *testing.T, base string) {
-	ctx := context.Background()
+	ctx := t.Context()
 	ctx = WithTraceFunc(ctx, t.Logf)
 
 	ctx = loadDNS(t, ctx, base+".dns")

internal/dkim/sign_test.go

@@ -1,7 +1,6 @@
 package dkim
 
 import (
-	"context"
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/elliptic"
@@ -33,7 +32,7 @@ Joe.
 `)
 
 func TestSignRSA(t *testing.T) {
-	ctx := context.Background()
+	ctx := t.Context()
 	ctx = WithTraceFunc(ctx, t.Logf)
 
 	// Generate a new key pair.
@@ -84,7 +83,7 @@ func TestSignRSA(t *testing.T) {
 }
 
 func TestSignEd25519(t *testing.T) {
-	ctx := context.Background()
+	ctx := t.Context()
 	ctx = WithTraceFunc(ctx, t.Logf)
 
 	// Generate a new key pair.
@@ -141,7 +140,7 @@ func TestSignBadMessage(t *testing.T) {
 		Domain:   "example.com",
 		Selector: "test",
 	}
-	_, err := s.Sign(context.Background(), "Bad message")
+	_, err := s.Sign(t.Context(), "Bad message")
 	if err == nil {
 		t.Errorf("Sign: wanted error; got nil")
 	}
@@ -159,7 +158,7 @@ func TestSignBadAlgorithm(t *testing.T) {
 	}
 	s.Signer = priv
 
-	_, err = s.Sign(context.Background(), basicMessage)
+	_, err = s.Sign(t.Context(), basicMessage)
 	if !errors.Is(err, errUnsupportedKeyType) {
 		t.Errorf("Sign: wanted unsupported key type; got %v", err)
 	}

internal/dkim/verify_test.go

@@ -20,7 +20,7 @@ func makeLookupTXT(results map[string][]string) lookupTXTFunc {
 }
 
 func TestVerifyRF6376CExample(t *testing.T) {
-	ctx := context.Background()
+	ctx := t.Context()
 	ctx = WithTraceFunc(ctx, t.Logf)
 
 	// Use the public key from the example in RFC 6376 appendix C.
@@ -88,7 +88,7 @@ Joe.
 }
 
 func TestVerifyRFC8463Example(t *testing.T) {
-	ctx := context.Background()
+	ctx := t.Context()
 	ctx = WithTraceFunc(ctx, t.Logf)
 
 	// Use the public keys from the example in RFC 8463 appendix A.2.

internal/domaininfo/domaininfo_test.go

@@ -10,8 +10,7 @@ import (
 )
 
 func TestBasic(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	db, err := New(dir)
 	if err != nil {
 		t.Fatal(err)
@@ -70,8 +69,7 @@ func TestBasic(t *testing.T) {
 }
 
 func TestNewDomain(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	db, err := New(dir)
 	if err != nil {
 		t.Fatal(err)
@@ -101,8 +99,7 @@ func TestNewDomain(t *testing.T) {
 }
 
 func TestProgressions(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	db, err := New(dir)
 	if err != nil {
 		t.Fatal(err)
@@ -146,8 +143,7 @@ func TestErrors(t *testing.T) {
 	}
 
 	// Corrupt/invalid file.
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	db, err := New(dir)
 	if err != nil {
 		t.Fatal(err)
@@ -175,8 +171,7 @@ func TestErrors(t *testing.T) {
 }
 
 func TestDirectoryErrors(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	db, err := New(dir + "/db")
 	if err != nil {
 		t.Fatal(err)

internal/dovecot/dovecot_test.go

@@ -8,8 +8,6 @@ package dovecot
 import (
 	"net"
 	"testing"
-
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 )
 
 func TestUsernameNotSafe(t *testing.T) {
@@ -59,8 +57,7 @@ func TestAutodetect(t *testing.T) {
 	}
 
 	// Create a temporary directory, and two sockets on it.
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.TempDir()
 
 	userdb := dir + "/userdb"
 	client := dir + "/client"

internal/localrpc/client_test.go

@@ -6,22 +6,29 @@ import (
 	"io/fs"
 	"net"
 	"net/textproto"
-	"os"
 	"path/filepath"
 	"testing"
 )
 
+// NewFakeServer starts a fake server listening on the given path, which
+// replies with the given output to every request.
 func NewFakeServer(t *testing.T, path, output string) {
 	t.Helper()
 	lis, err := net.Listen("unix", path)
 	if err != nil {
-		panic(err)
+		t.Fatal(err)
 	}
+	t.Cleanup(func() { lis.Close() })
 
+	go fakeServe(t, lis, output)
+}
+
+func fakeServe(t *testing.T, lis net.Listener, output string) {
 	for {
 		conn, err := lis.Accept()
 		if err != nil {
-			panic(err)
+			// Listener closed at the end of the test.
+			return
 		}
 		t.Logf("FakeServer %v: accepted ", conn)
 
@@ -39,20 +46,14 @@ func NewFakeServer(t *testing.T, path, output string) {
 }
 
 func TestBadServer(t *testing.T) {
-	tmpDir, err := os.MkdirTemp("", "rpc-test-*")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(tmpDir)
-	socketPath := filepath.Join(tmpDir, "rpc.sock")
+	socketPath := filepath.Join(t.TempDir(), "rpc.sock")
 
 	// textproto client expects a numeric code, this should cause ReadCodeLine
 	// to fail with textproto.ProtocolError.
-	go NewFakeServer(t, socketPath, "xxx")
-	waitForServer(t, socketPath)
+	NewFakeServer(t, socketPath, "xxx")
 
 	client := NewClient(socketPath)
-	_, err = client.Call("Echo")
+	_, err := client.Call("Echo")
 	if err == nil {
 		t.Fatal("expected error")
 	}

internal/localrpc/e2e_test.go

@@ -4,9 +4,7 @@ import (
 	"errors"
 	"net"
 	"net/url"
-	"os"
 	"testing"
-	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/trace"
 	"github.com/google/go-cmp/cmp"
@@ -29,7 +27,6 @@ func HolaErr(tr *trace.Trace, input url.Values) (url.Values, error) {
 }
 
 type testServer struct {
-	dir  string
 	sock string
 	*Server
 }
@@ -37,31 +34,25 @@ type testServer struct {
 func newTestServer(t *testing.T) *testServer {
 	t.Helper()
 
-	tmpDir, err := os.MkdirTemp("", "rpc-test-*")
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	tsrv := &testServer{
-		dir:    tmpDir,
-		sock:   tmpDir + "/sock",
+		sock:   t.TempDir() + "/sock",
 		Server: NewServer(),
 	}
 
 	tsrv.Register("Echo", Echo)
 	tsrv.Register("Hola", Hola)
 	tsrv.Register("HolaErr", HolaErr)
-	go tsrv.ListenAndServe(tsrv.sock)
 
-	waitForServer(t, tsrv.sock)
+	lis, err := net.Listen("unix", tsrv.sock)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { lis.Close() })
+	go tsrv.Serve(lis)
+
 	return tsrv
 }
 
-func (tsrv *testServer) Cleanup() {
-	tsrv.Close()
-	os.RemoveAll(tsrv.dir)
-}
-
 func mkV(args ...string) url.Values {
 	v := url.Values{}
 	for i := 0; i < len(args); i += 2 {
@@ -72,7 +63,6 @@ func mkV(args ...string) url.Values {
 
 func TestEndToEnd(t *testing.T) {
 	srv := newTestServer(t)
-	defer srv.Cleanup()
 
 	// Run the client.
 	client := NewClient(srv.sock)
@@ -112,21 +102,6 @@ func TestEndToEnd(t *testing.T) {
 	}
 }
 
-func waitForServer(t *testing.T, path string) {
-	t.Helper()
-	for range 100 {
-		time.Sleep(10 * time.Millisecond)
-		conn, err := net.Dial("unix", path)
-		if conn != nil {
-			conn.Close()
-		}
-		if err == nil {
-			return
-		}
-	}
-	t.Fatal("server didn't start")
-}
-
 // Allow us to compare errors with cmp.Diff by their string content (since the
 // instances/types don't carry across RPC boundaries).
 var transformErrors = cmp.Transformer(

internal/localrpc/localrpc.go

@@ -47,19 +47,24 @@ func (s *Server) Register(name string, handler Handler) {
 
 // ListenAndServe starts the server.
 func (s *Server) ListenAndServe(path string) error {
-	tr := trace.New("LocalRPC.Server", path)
-	defer tr.Finish()
-
 	// Previous instances of the server may have shut down uncleanly, leaving
 	// behind the socket file. Remove it just in case.
 	os.Remove(path)
 
-	var err error
-	s.lis, err = net.Listen("unix", path)
+	lis, err := net.Listen("unix", path)
 	if err != nil {
 		return err
 	}
 
+	return s.Serve(lis)
+}
+
+// Serve connections from the given listener.
+func (s *Server) Serve(lis net.Listener) error {
+	tr := trace.New("LocalRPC.Server", lis.Addr().String())
+	defer tr.Finish()
+
+	s.lis = lis
 	tr.Printf("Listening")
 	for {
 		conn, err := s.lis.Accept()

internal/nettrace/bench_test.go

@@ -39,7 +39,7 @@ func BenchmarkTrace_10000(b *testing.B) {
 }
 
 func BenchmarkNewAndFinish(b *testing.B) {
-	for i := 0; i < b.N; i++ {
+	for b.Loop() {
 		tr := New("bench", "test")
 		tr.Finish()
 	}
@@ -48,8 +48,7 @@ func BenchmarkNewAndFinish(b *testing.B) {
 func BenchmarkPrintf(b *testing.B) {
 	tr := New("bench", "test")
 	defer tr.Finish()
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
+	for b.Loop() {
 		// Keep this without any formatting, so we measure our code instead of
 		// the performance of fmt.Sprintf.
 		tr.Printf("this is printf")

internal/nettrace/context_test.go

@@ -1,7 +1,6 @@
 package nettrace
 
 import (
-	"context"
 	"testing"
 )
 
@@ -10,7 +9,7 @@ func TestContext(t *testing.T) {
 	defer tr.Finish()
 
 	// Attach the trace to a new context.
-	ctx := NewContext(context.Background(), tr)
+	ctx := NewContext(t.Context(), tr)
 
 	// Get the trace back from the context.
 	{
@@ -46,7 +45,7 @@ func TestContext(t *testing.T) {
 	// FromContextOrNew needs to create a new one.
 	{
 		tr5, ctx5 := FromContextOrNew(
-			context.Background(), "TestContext", "tr5")
+			t.Context(), "TestContext", "tr5")
 		if tr, _ := FromContext(ctx5); tr != tr5 {
 			t.Errorf("Context with trace returned the wrong trace: %v != %v",
 				tr, tr5)
@@ -57,7 +56,7 @@ func TestContext(t *testing.T) {
 	// Child from a context that has no trace attached.
 	{
 		tr6 := ChildFromContext(
-			context.Background(), "TestContext", "child")
+			t.Context(), "TestContext", "child")
 		tr6.Finish()
 		if p := tr6.(*trace).Parent; p != nil {
 			t.Errorf("Expected orphan trace, it has a parent: %v", p)

internal/normalize/normalize_test.go

@@ -197,8 +197,7 @@ func BenchmarkToCRLF(b *testing.B) {
 	}
 	buf := bb.Bytes()
 
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
+	for b.Loop() {
 		ToCRLF(buf)
 	}
 }
@@ -211,8 +210,7 @@ func BenchmarkStringToCRLF(b *testing.B) {
 	}
 	s := sb.String()
 
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
+	for b.Loop() {
 		StringToCRLF(s)
 	}
 }

internal/protoio/protoio_test.go

@@ -5,20 +5,18 @@ import (
 	"testing"
 
 	"blitiri.com.ar/go/chasquid/internal/protoio/testpb"
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 )
 
 func TestBin(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	fname := t.ArtifactDir() + "/f"
 	pb := &testpb.M{Content: "hola"}
 
-	if err := WriteMessage("f", pb, 0600); err != nil {
+	if err := WriteMessage(fname, pb, 0600); err != nil {
 		t.Error(err)
 	}
 
 	pb2 := &testpb.M{}
-	if err := ReadMessage("f", pb2); err != nil {
+	if err := ReadMessage(fname, pb2); err != nil {
 		t.Error(err)
 	}
 	if pb.Content != pb2.Content {
@@ -27,16 +25,15 @@ func TestBin(t *testing.T) {
 }
 
 func TestText(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	fname := t.ArtifactDir() + "/f"
 	pb := &testpb.M{Content: "hola"}
 
-	if err := WriteTextMessage("f", pb, 0600); err != nil {
+	if err := WriteTextMessage(fname, pb, 0600); err != nil {
 		t.Error(err)
 	}
 
 	pb2 := &testpb.M{}
-	if err := ReadTextMessage("f", pb2); err != nil {
+	if err := ReadTextMessage(fname, pb2); err != nil {
 		t.Error(err)
 	}
 	if pb.Content != pb2.Content {
@@ -45,8 +42,7 @@ func TestText(t *testing.T) {
 }
 
 func TestStore(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	st, err := NewStore(dir + "/store")
 	if err != nil {
 		t.Fatalf("failed to create store: %v", err)
@@ -98,8 +94,6 @@ func mustCreate(t *testing.T, fname string) {
 }
 
 func TestFileErrors(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
 	pb := &testpb.M{Content: "hola"}
 
 	if err := WriteMessage("/proc/doesnotexist", pb, 0600); err == nil {
@@ -125,18 +119,17 @@ func TestFileErrors(t *testing.T) {
 }
 
 func TestMarshalErrors(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 
 	// The marshaller enforces that strings are well-formed utf8. So to create
 	// a marshalling error, we use a non-utf8 string.
 	pb := &testpb.M{Content: "\xc3\x28"}
 
-	if err := WriteMessage("f", pb, 0600); err == nil {
+	if err := WriteMessage(dir+"/f", pb, 0600); err == nil {
 		t.Errorf("write worked, expected error")
 	}
 
-	if err := WriteTextMessage("ft", pb, 0600); err == nil {
+	if err := WriteTextMessage(dir+"/ft", pb, 0600); err == nil {
 		t.Errorf("text write worked, expected error")
 	}
 }

internal/queue/queue_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/aliases"
@@ -18,8 +19,8 @@ func allUsersExist(tr *trace.Trace, user, domain string) (bool, error) {
 }
 
 func TestBasic(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	synctest.Test(t, func(t *testing.T) {
+		dir := t.ArtifactDir()
 		localC := testlib.NewTestCourier()
 		remoteC := testlib.NewTestCourier()
 		q, _ := New(dir, set.NewString("loco"),
@@ -42,8 +43,9 @@ func TestBasic(t *testing.T) {
 		localC.Wait()
 		remoteC.Wait()
 
-	// Make sure the delivered items leave the queue.
-	testlib.WaitFor(func() bool { return q.Len() == 0 }, 2*time.Second)
+		// Wait for the send loop to finish, and make sure the delivered items
+		// leave the queue.
+		synctest.Wait()
 		if q.Len() != 0 {
 			t.Fatalf("%d items not removed from the queue after delivery", q.Len())
 		}
@@ -68,13 +70,14 @@ func TestBasic(t *testing.T) {
 				t.Errorf("wrong request for %q: %v", c.expectedTo, req)
 			}
 		}
+	})
 }
 
 func TestDSNOnTimeout(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
 		localC := testlib.NewTestCourier()
 		remoteC := testlib.NewTestCourier()
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+		dir := t.ArtifactDir()
 		q, _ := New(dir, set.NewString("loco"),
 			aliases.NewResolver(allUsersExist),
 			localC, remoteC)
@@ -113,13 +116,18 @@ func TestDSNOnTimeout(t *testing.T) {
 			!strings.Contains(string(req.Data), "X-Failed-Recipients: to@to,") {
 			t.Errorf("wrong DSN: %q", string(req.Data))
 		}
+
+		// Wait for the send loops to finish, so they don't race with the test
+		// directory cleanup.
+		synctest.Wait()
+	})
 }
 
 func TestAliases(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
 		localC := testlib.NewTestCourier()
 		remoteC := testlib.NewTestCourier()
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+		dir := t.ArtifactDir()
 		q, _ := New(dir, set.NewString("loco"),
 			aliases.NewResolver(allUsersExist),
 			localC, remoteC)
@@ -186,11 +194,16 @@ func TestAliases(t *testing.T) {
 				t.Errorf("wrong request for %q: %v", c.expectedTo, *req)
 			}
 		}
+
+		// Wait for the send loops to finish, so they don't race with the test
+		// directory cleanup.
+		synctest.Wait()
+	})
 }
 
 func TestFullQueue(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	synctest.Test(t, func(t *testing.T) {
+		dir := t.ArtifactDir()
 		q, _ := New(dir, set.NewString(),
 			aliases.NewResolver(allUsersExist),
 			testlib.DumbCourier, testlib.DumbCourier)
@@ -231,11 +244,15 @@ func TestFullQueue(t *testing.T) {
 			t.Errorf("Put: %v", err)
 		}
 		q.Remove(id)
+
+		// Wait for the send loops to finish, so they don't race with the test
+		// directory cleanup.
+		synctest.Wait()
+	})
 }
 
 func TestPipes(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 	q, _ := New(dir, set.NewString("loco"),
 		aliases.NewResolver(allUsersExist),
 		testlib.DumbCourier, testlib.DumbCourier)
@@ -290,8 +307,8 @@ func TestNextDelay(t *testing.T) {
 }
 
 func TestSerialization(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	synctest.Test(t, func(t *testing.T) {
+		dir := t.ArtifactDir()
 
 		// Save an item in the queue directory.
 		item := &Item{
@@ -328,6 +345,11 @@ func TestSerialization(t *testing.T) {
 		if req.From != "from@loco" || req.To != "to@to" {
 			t.Errorf("wrong email: %v", req)
 		}
+
+		// Wait for the send loops to finish, so they don't race with the test
+		// directory cleanup.
+		synctest.Wait()
+	})
 }
 
 func mkR(a string, t Recipient_Type, s Recipient_Status, m, o string) *Recipient {

internal/safeio/safeio_test.go

@@ -42,8 +42,8 @@ func testWriteFile(fname string, data []byte, perm os.FileMode, ops ...FileOp) e
 }
 
 func TestWriteFile(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
+	t.Chdir(dir)
 
 	// Write a new file.
 	content := []byte("content 1")
@@ -65,8 +65,8 @@ func TestWriteFile(t *testing.T) {
 }
 
 func TestWriteFileWithOp(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
+	t.Chdir(dir)
 
 	var opFile string
 	op := func(f string) error {
@@ -88,8 +88,8 @@ func TestWriteFileWithOp(t *testing.T) {
 }
 
 func TestWriteFileWithFailingOp(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
+	t.Chdir(dir)
 
 	var opFile string
 	opOK := func(f string) error {
@@ -166,8 +166,8 @@ func (f *testFile) Close() error {
 var _ osFile = &testFile{}
 
 func TestErrors(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
+	t.Chdir(dir)
 
 	oldCreateTemp := createTemp
 	defer func() { createTemp = oldCreateTemp }()
@@ -248,8 +248,8 @@ func checkNotExists(t *testing.T, fname string) {
 
 func TestFsyncFileOp(t *testing.T) {
 	// Check it as an operation on a normal file.
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
+	t.Chdir(dir)
 
 	content := []byte("content 1")
 	err := testWriteFile("file1", content, 0660, FsyncFileOp)

internal/smtpsrv/conn_test.go

@@ -2,11 +2,9 @@ package smtpsrv
 
 import (
 	"net"
-	"os"
 	"testing"
 
 	"blitiri.com.ar/go/chasquid/internal/domaininfo"
-	"blitiri.com.ar/go/chasquid/internal/testlib"
 	"blitiri.com.ar/go/chasquid/internal/trace"
 	"blitiri.com.ar/go/spf"
 )
@@ -14,15 +12,7 @@ import (
 func TestSecLevel(t *testing.T) {
 	// We can't simulate this externally because of the SPF record
 	// requirement, so do a narrow test on Conn.secLevelCheck.
-	// Create the directory by hand because we don't want to automatically
-	// chdir into it (it affects the fuzzing infrastructure).
-	dir, err := os.MkdirTemp("", "testlib_")
-	if err != nil {
-		t.Fatalf("Failed to create temp dir: %v\n", dir)
-	}
-	defer testlib.RemoveIfOk(t, dir)
-
-	dinfo, err := domaininfo.New(dir)
+	dinfo, err := domaininfo.New(t.ArtifactDir())
 	if err != nil {
 		t.Fatalf("Failed to create domain info: %v", err)
 	}

internal/smtpsrv/server_test.go

@@ -12,7 +12,6 @@ import (
 	"os"
 	"strings"
 	"testing"
-	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/aliases"
 	"blitiri.com.ar/go/chasquid/internal/auth"
@@ -491,8 +490,7 @@ func TestAddDKIMSigner(t *testing.T) {
 		t.Errorf("AddDKIMSigner: expected not exist, got %v", err)
 	}
 
-	tmpDir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, tmpDir)
+	tmpDir := t.ArtifactDir()
 
 	// Invalid PEM file.
 	kf1 := tmpDir + "/key1-bad_pem.pem"
@@ -555,8 +553,7 @@ func BenchmarkManyEmails(b *testing.B) {
 	c := mustDial(b, ModeSMTP, false)
 	defer c.Close()
 
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
+	for b.Loop() {
 		sendEmail(b, c)
 	}
 }
@@ -576,25 +573,6 @@ func BenchmarkManyEmailsParallel(b *testing.B) {
 // === Test environment ===
 //
 
-// waitForServer waits 5 seconds for the server to start, and returns an error
-// if it fails to do so.
-// It does this by repeatedly connecting to the address until it either
-// replies or times out. Note we do not do any validation of the reply.
-func waitForServer(addr string) error {
-	start := time.Now()
-	for time.Since(start) < 10*time.Second {
-		conn, err := net.Dial("tcp", addr)
-		if err == nil {
-			conn.Close()
-			return nil
-		}
-
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	return fmt.Errorf("not reachable")
-}
-
 type brokenAuthBE struct{}
 
 func (b brokenAuthBE) Authenticate(user, password string) (bool, error) {
@@ -615,11 +593,7 @@ func realMain(m *testing.M) int {
 	flag.Parse()
 
 	// Create a 1MiB string, which the large message tests use.
-	buf := make([]byte, 1024*1024)
-	for i := range buf {
-		buf[i] = 'a'
-	}
-	str1MiB = string(buf)
+	str1MiB = strings.Repeat("a", 1024*1024)
 
 	// Set up the mail log to stdout, which is captured by the test runner,
 	// so we have better debugging information on failures.
@@ -647,17 +621,29 @@ func realMain(m *testing.M) int {
 			return 1
 		}
 
-		smtpAddr = testlib.GetFreePort()
-		submissionAddr = testlib.GetFreePort()
-		submissionTLSAddr = testlib.GetFreePort()
-
 		s := NewServer()
 		s.Hostname = "localhost"
 		s.MaxDataSize = int64(maxDataSizeMiB) * 1024 * 1024
 		s.AddCerts(tmpDir+"/cert.pem", tmpDir+"/key.pem")
-		s.AddAddr(smtpAddr, ModeSMTP)
-		s.AddAddr(submissionAddr, ModeSubmission)
-		s.AddAddr(submissionTLSAddr, ModeSubmissionTLS)
+
+		// Open the listeners ourselves, so the ports are already bound
+		// (and we know their addresses) by the time the tests run.
+		for _, l := range []struct {
+			addr *string
+			mode SocketMode
+		}{
+			{&smtpAddr, ModeSMTP},
+			{&submissionAddr, ModeSubmission},
+			{&submissionTLSAddr, ModeSubmissionTLS},
+		} {
+			ln, err := net.Listen("tcp", "localhost:0")
+			if err != nil {
+				fmt.Printf("Failed to listen: %v\n", err)
+				return 1
+			}
+			*l.addr = ln.Addr().String()
+			s.AddListeners([]net.Listener{ln}, l.mode)
+		}
 
 		s.InitQueue(tmpDir+"/queue", localC, remoteC)
 
@@ -687,9 +673,6 @@ func realMain(m *testing.M) int {
 		go s.ListenAndServe()
 	}
 
-	waitForServer(smtpAddr)
-	waitForServer(submissionAddr)
-	waitForServer(submissionTLSAddr)
 	return m.Run()
 }
 

internal/sts/sts_test.go

@@ -72,7 +72,7 @@ func TestMain(m *testing.M) {
 	httpServer := httptest.NewServer(http.HandlerFunc(testHTTPHandler))
 
 	fakeURLForTesting = httpServer.URL
-	os.Exit(m.Run())
+	m.Run()
 }
 
 func TestParsePolicy(t *testing.T) {
@@ -196,21 +196,21 @@ func TestFetch(t *testing.T) {
 	// defined in TestMain above. See httpGet for more details.
 
 	// Normal fetch, all valid.
-	p, err := Fetch(context.Background(), "domain.com")
+	p, err := Fetch(t.Context(), "domain.com")
 	if err != nil {
 		t.Errorf("failed to fetch policy: %v", err)
 	}
 	t.Logf("domain.com: %+v", p)
 
 	// Domain without a policy (HTTP get fails).
-	p, err = Fetch(context.Background(), "policy404")
+	p, err = Fetch(t.Context(), "policy404")
 	if err == nil {
 		t.Errorf("fetched unknown policy: %v", p)
 	}
 	t.Logf("policy404: got error as expected: %v", err)
 
 	// Domain with an invalid policy (unknown version).
-	p, err = Fetch(context.Background(), "version99")
+	p, err = Fetch(t.Context(), "version99")
 	if err != ErrUnknownVersion {
 		t.Errorf("expected error %v, got %v (and policy: %v)",
 			ErrUnknownVersion, err, p)
@@ -218,7 +218,7 @@ func TestFetch(t *testing.T) {
 	t.Logf("version99: got expected error: %v", err)
 
 	// Error fetching TXT record for this domain.
-	p, err = Fetch(context.Background(), "domErr")
+	p, err = Fetch(t.Context(), "domErr")
 	if err != errTest {
 		t.Errorf("expected error %v, got %v (and policy: %v)",
 			errTest, err, p)
@@ -235,7 +235,7 @@ func TestPolicyTooBig(t *testing.T) {
 	raw += `"mxlast"], "max_age": 100}`
 	policyForDomain["toobig"] = raw
 
-	_, err := Fetch(context.Background(), "toobig")
+	_, err := Fetch(t.Context(), "toobig")
 	if err == nil {
 		t.Errorf("fetch worked, but should have failed")
 	}
@@ -251,8 +251,7 @@ func expvarMustEq(t *testing.T, name string, v *expvar.Int, expected int64) {
 }
 
 func TestCacheBasics(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 
 	c, err := NewCache(dir)
 	if err != nil {
@@ -266,7 +265,7 @@ func TestCacheBasics(t *testing.T) {
 	cacheFetches.Set(0)
 	cacheHits.Set(0)
 
-	ctx := context.Background()
+	ctx := t.Context()
 
 	// Fetch domain.com, check we get a reasonable policy, and that it's a
 	// cache miss.
@@ -314,15 +313,14 @@ func TestCacheBasics(t *testing.T) {
 
 // Test how the cache behaves when the files are corrupt.
 func TestCacheBadData(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 
 	c, err := NewCache(dir)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	ctx := context.Background()
+	ctx := t.Context()
 
 	cacheUnmarshalErrors.Set(0)
 	cacheInvalid.Set(0)
@@ -391,6 +389,7 @@ func (c *PolicyCache) mustFetch(ctx context.Context, t *testing.T, d string) *Po
 }
 
 func mustRewriteAndChtime(t *testing.T, fname, content string) {
+	t.Helper()
 	testlib.Rewrite(t, fname, content)
 
 	// Advance the expiration time to the future, so the rewritten policy is
@@ -403,15 +402,14 @@ func mustRewriteAndChtime(t *testing.T, fname, content string) {
 }
 
 func TestCacheRefresh(t *testing.T) {
-	dir := testlib.MustTempDir(t)
-	defer testlib.RemoveIfOk(t, dir)
+	dir := t.ArtifactDir()
 
 	c, err := NewCache(dir)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	ctx := context.Background()
+	ctx := t.Context()
 
 	txtResults["_mta-sts.refresh-test"] = []string{"v=STSv1; id=blah;"}
 	policyForDomain["refresh-test"] = `
@@ -439,7 +437,7 @@ func TestCacheRefresh(t *testing.T) {
 
 	// Launch background refreshes, and wait for one to complete.
 	cacheRefreshCycles.Set(0)
-	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	ctx, cancel := context.WithTimeout(t.Context(), 1*time.Second)
 	defer cancel()
 	go c.PeriodicallyRefresh(ctx)
 	for cacheRefreshCycles.Value() == 0 {
@@ -453,7 +451,7 @@ func TestCacheRefresh(t *testing.T) {
 }
 
 func TestCacheSlashSafe(t *testing.T) {
-	dir := testlib.MustTempDir(t)
+	dir := t.ArtifactDir()
 	c, err := NewCache(dir)
 	if err != nil {
 		t.Fatal(err)
@@ -519,7 +517,7 @@ func TestHTTPGet(t *testing.T) {
 		}))
 	defer srv1.Close()
 
-	ctx := context.Background()
+	ctx := t.Context()
 	raw, err := httpGet(ctx, srv1.URL)
 	if err != nil {
 		t.Errorf("GET failed: got %q, %v", raw, err)

internal/testlib/testlib.go

@@ -11,78 +11,18 @@ import (
 	"math/big"
 	"net"
 	"os"
-	"strings"
 	"sync"
 	"testing"
 	"time"
 )
 
-// MustTempDir creates a temporary directory, or dies trying.
-func MustTempDir(t *testing.T) string {
-	dir, err := os.MkdirTemp("", "testlib_")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = os.Chdir(dir)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	t.Logf("test directory: %q", dir)
-	return dir
-}
-
-// RemoveIfOk removes the given directory, but only if we have not failed. We
-// want to keep the failed directories for debugging.
-func RemoveIfOk(t *testing.T, dir string) {
-	// Safeguard, to make sure we only remove test directories.
-	// This should help prevent accidental deletions.
-	if !strings.Contains(dir, "testlib_") {
-		panic("invalid/dangerous directory")
-	}
-
-	if !t.Failed() {
-		os.RemoveAll(dir)
-	}
-}
-
-// Rewrite a file with the given contents.
-func Rewrite(t *testing.T, path, contents string) error {
-	// Safeguard, to make sure we only mess with test files.
-	if !strings.Contains(path, "testlib_") {
-		panic("invalid/dangerous path")
-	}
-
+// Rewrite a file with the given contents, or fail the test.
+func Rewrite(t *testing.T, path, contents string) {
+	t.Helper()
 	err := os.WriteFile(path, []byte(contents), 0600)
 	if err != nil {
-		t.Errorf("failed to rewrite file: %v", err)
+		t.Fatalf("failed to rewrite file: %v", err)
 	}
-
-	return err
-}
-
-// GetFreePort returns a free TCP port. This is hacky and not race-free, but
-// it works well enough for testing purposes.
-func GetFreePort() string {
-	l, err := net.Listen("tcp", "localhost:0")
-	if err != nil {
-		panic(err)
-	}
-	defer l.Close()
-	return l.Addr().String()
-}
-
-// WaitFor f to return true (returns true), or d to pass (returns false).
-func WaitFor(f func() bool, d time.Duration) bool {
-	start := time.Now()
-	for time.Since(start) < d {
-		if f() {
-			return true
-		}
-		time.Sleep(20 * time.Millisecond)
-	}
-	return false
 }
 
 type deliverRequest struct {
@@ -97,27 +37,27 @@ type TestCourier struct {
 	wg       sync.WaitGroup
 	Requests []*deliverRequest
 	ReqFor   map[string]*deliverRequest
-	sync.Mutex
+	mu       sync.Mutex
 }
 
 // Deliver the given mail (saving it in tc.Requests).
 func (tc *TestCourier) Deliver(from string, to string, data []byte) (error, bool) {
 	defer tc.wg.Done()
 	dr := &deliverRequest{from, to, data, nil}
-	tc.Lock()
+	tc.mu.Lock()
 	tc.Requests = append(tc.Requests, dr)
 	tc.ReqFor[to] = dr
-	tc.Unlock()
+	tc.mu.Unlock()
 	return nil, false
 }
 
 func (tc *TestCourier) Forward(from string, to string, data []byte, servers []string) (error, bool) {
 	defer tc.wg.Done()
 	dr := &deliverRequest{from, to, data, servers}
-	tc.Lock()
+	tc.mu.Lock()
 	tc.Requests = append(tc.Requests, dr)
 	tc.ReqFor[to] = dr
-	tc.Unlock()
+	tc.mu.Unlock()
 	return nil, false
 }
 

internal/testlib/testlib_test.go

@@ -1,107 +1,26 @@
 package testlib
 
 import (
-	"errors"
-	"io/fs"
 	"os"
 	"testing"
-	"time"
 )
 
-func TestBasic(t *testing.T) {
-	dir := MustTempDir(t)
-	if err := os.WriteFile(dir+"/file", nil, 0660); err != nil {
-		t.Fatalf("could not create file in %s: %v", dir, err)
-	}
-
-	wd, err := os.Getwd()
-	if err != nil {
-		t.Fatalf("could not get working directory: %v", err)
-	}
-	if wd != dir {
-		t.Errorf("MustTempDir did not change directory")
-		t.Errorf("  expected %q, got %q", dir, wd)
-	}
-
-	RemoveIfOk(t, dir)
-	if _, err := os.Stat(dir); !errors.Is(err, fs.ErrNotExist) {
-		t.Fatalf("%s existed, should have been deleted: %v", dir, err)
-	}
-}
-
-func TestRemoveCheck(t *testing.T) {
-	defer func() {
-		if r := recover(); r != nil {
-			t.Logf("recovered: %v", r)
-		} else {
-			t.Fatalf("check did not panic as expected")
-		}
-	}()
-
-	RemoveIfOk(t, "/tmp/something")
-}
-
-func TestLeaveDirOnError(t *testing.T) {
-	myt := &testing.T{}
-	dir := MustTempDir(myt)
-	myt.Errorf("something bad happened")
-
-	RemoveIfOk(myt, dir)
-	if _, err := os.Stat(dir); errors.Is(err, fs.ErrNotExist) {
-		t.Fatalf("%s was removed, should have been kept", dir)
-	}
-
-	// Remove the directory for real this time.
-	RemoveIfOk(t, dir)
-}
-
-func TestRewriteSafeguard(t *testing.T) {
-	myt := &testing.T{}
-	defer func() {
-		if r := recover(); r != nil {
-			t.Logf("recovered: %v", r)
-		} else {
-			t.Fatalf("check did not panic as expected")
-		}
-	}()
-
-	Rewrite(myt, "/something", "test")
-}
-
 func TestRewrite(t *testing.T) {
-	dir := MustTempDir(t)
-	defer RemoveIfOk(t, dir)
+	fname := t.TempDir() + "/file"
+	Rewrite(t, fname, "hola")
+	Rewrite(t, fname, "chau")
 
-	myt := &testing.T{}
-	Rewrite(myt, dir+"/file", "hola")
-	if myt.Failed() {
-		t.Errorf("basic rewrite failed")
+	data, err := os.ReadFile(fname)
+	if err != nil {
+		t.Fatalf("failed to read file: %v", err)
 	}
-}
-
-func TestGetFreePort(t *testing.T) {
-	p := GetFreePort()
-	if p == "" {
-		t.Errorf("failed to get free port")
-	}
-}
-
-func TestWaitFor(t *testing.T) {
-	ok := WaitFor(func() bool { return true }, 20*time.Millisecond)
-	if !ok {
-		t.Errorf("WaitFor(true) timed out")
-	}
-
-	ok = WaitFor(func() bool { return false }, 20*time.Millisecond)
-	if ok {
-		t.Errorf("WaitFor(false) worked")
+	if string(data) != "chau" {
+		t.Errorf("file content: got %q, expected %q", data, "chau")
 	}
 }
 
 func TestGenerateCert(t *testing.T) {
-	dir := MustTempDir(t)
-	defer os.RemoveAll(dir)
-	conf, err := GenerateCert(dir)
+	conf, err := GenerateCert(t.TempDir())
 	if err != nil {
 		t.Errorf("GenerateCert returned error: %v", err)
 	}

internal/userdb/userdb_test.go

@@ -1,34 +1,21 @@
 package userdb
 
 import (
-	"fmt"
 	"os"
-	"reflect"
-	"strings"
 	"testing"
+
+	"google.golang.org/protobuf/proto"
 )
 
-// Remove the file if the test was successful. Used in defer statements, to
-// leave files around for inspection when the tests failed.
-func removeIfSuccessful(t *testing.T, fname string) {
-	// Safeguard, to make sure we only remove test files.
-	// This should help prevent accidental deletions.
-	if !strings.Contains(fname, "userdb_test") {
-		panic("invalid/dangerous directory")
-	}
-
-	if !t.Failed() {
-		os.Remove(fname)
-	}
-}
-
 // Create a database with the given content on a temporary filename. Return
 // the filename, or an error if there were errors creating it.
 func mustCreateDB(t *testing.T, content string) string {
-	f, err := os.CreateTemp("", "userdb_test")
+	t.Helper()
+	f, err := os.CreateTemp(t.ArtifactDir(), "userdb_test")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer f.Close()
 
 	if _, err := f.WriteString(content); err != nil {
 		t.Fatal(err)
@@ -39,22 +26,7 @@ func mustCreateDB(t *testing.T, content string) string {
 }
 
 func dbEquals(a, b *DB) bool {
-	if a.db == nil || b.db == nil {
-		return a.db == nil && b.db == nil
-	}
-
-	if len(a.db.Users) != len(b.db.Users) {
-		return false
-	}
-
-	for k, av := range a.db.Users {
-		bv, ok := b.db.Users[k]
-		if !ok || !reflect.DeepEqual(av, bv) {
-			return false
-		}
-	}
-
-	return true
+	return proto.Equal(a.db, b.db)
 }
 
 var emptyDB = &DB{
@@ -80,7 +52,6 @@ func TestEmptyLoad(t *testing.T) {
 
 func testOneLoad(t *testing.T, desc, content string, fatal bool, fatalErr error) {
 	fname := mustCreateDB(t, content)
-	defer removeIfSuccessful(t, fname)
 	db, err := Load(fname)
 	if fatal {
 		if err == nil {
@@ -109,7 +80,6 @@ func mustLoad(t *testing.T, fname string) *DB {
 
 func TestWrite(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	if err := db.Write(); err != nil {
@@ -170,8 +140,7 @@ func TestWrite(t *testing.T) {
 }
 
 func TestNew(t *testing.T) {
-	fname := fmt.Sprintf("%s/userdb_test-%d", os.TempDir(), os.Getpid())
-	defer os.Remove(fname)
+	fname := t.TempDir() + "/userdb"
 	db1 := New(fname)
 	db1.AddUser("user", "passwd")
 	db1.Write()
@@ -188,7 +157,6 @@ func TestNew(t *testing.T) {
 
 func TestInvalidUsername(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	// Names that are invalid.
@@ -227,7 +195,6 @@ func plainPassword(p string) *Password {
 // debugging, but it should be functional for that purpose.
 func TestPlainScheme(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	db.db.Users["user"] = plainPassword("pass word")
@@ -248,7 +215,6 @@ func TestPlainScheme(t *testing.T) {
 // Test the denied scheme.
 func TestDeniedScheme(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	db.db.Users["user"] = &Password{Scheme: &Password_Denied{}}
@@ -266,7 +232,6 @@ func TestDeniedScheme(t *testing.T) {
 func TestReload(t *testing.T) {
 	content := "users:< key: 'u1' value:< plain:< password: 'pass' >>>"
 	fname := mustCreateDB(t, content)
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	// Add a valid line to the file.
@@ -306,7 +271,6 @@ func TestReload(t *testing.T) {
 
 func TestRemoveUser(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	if ok := db.RemoveUser("unknown"); ok {
@@ -332,7 +296,6 @@ func TestRemoveUser(t *testing.T) {
 
 func TestExists(t *testing.T) {
 	fname := mustCreateDB(t, "")
-	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
 	if db.Exists("unknown") {