test: pin the production receive limits, split the vacuous drain test, harden the lint-gate test (review of #55)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

cmd/tapir/shutdown_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"sync"
 	"syscall"
 	"testing"
 	"time"
@@ -32,9 +33,20 @@ func TestShutdownContext_IsCancelledBySIGTERMAndSIGINT(t *testing.T) {
 }
 
 // serveHTTP must not return while a request is in flight or the worker is still recording
-// its job: ListenAndServe returns ErrServerClosed the moment Shutdown starts, so returning
-// from it let the process exit before either finished (#50).
-func TestServeHTTP_ReturnsOnlyAfterTheInFlightRequestAndTheWorkerHaveFinished(t *testing.T) {
+// its job: Serve returns http.ErrServerClosed as soon as Shutdown STARTS, and returning
+// from it let the process exit before either finished (#50). Each half is tested with the
+// other already satisfied, so neither can hold the call open on the other's behalf.
+
+func pendingFor(t *testing.T, returned <-chan error, why string) {
+	t.Helper()
+	select {
+	case err := <-returned:
+		t.Fatalf("serveHTTP returned (%v) %s", err, why)
+	case <-time.After(150 * time.Millisecond):
+	}
+}
+
+func TestServeHTTP_DrainsAnInFlightRequestBeforeReturning(t *testing.T) {
 	release := make(chan struct{})
 	entered := make(chan struct{})
 	srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
@@ -44,8 +56,12 @@ func TestServeHTTP_ReturnsOnlyAfterTheInFlightRequestAndTheWorkerHaveFinished(t
 	}))
 	srv.Start()
 	defer srv.Close()
+	var once sync.Once
+	releaseHandler := func() { once.Do(func() { close(release) }) }
+	defer releaseHandler() // runs before srv.Close: a failing test must not hang on the blocked handler
 	ctx, cancel := context.WithCancel(context.Background())
 	workerDone := make(chan struct{})
+	close(workerDone) // the worker is already done: only the request can hold serveHTTP open
 	returned := make(chan error, 1)
 	go func() { returned <- serveHTTP(ctx, srv.Config, srv.Listener, nil, workerDone, 5*time.Second) }()
 
@@ -53,17 +69,30 @@ func TestServeHTTP_ReturnsOnlyAfterTheInFlightRequestAndTheWorkerHaveFinished(t
 	<-entered
 	cancel()
 
-	assertPending := func(why string) {
-		t.Helper()
-		select {
-		case err := <-returned:
-			t.Fatalf("serveHTTP returned (%v) %s", err, why)
-		case <-time.After(150 * time.Millisecond):
+	pendingFor(t, returned, "while a request was still in flight")
+	releaseHandler()
+	select {
+	case err := <-returned:
+		if err != nil {
+			t.Fatalf("serveHTTP: %v", err)
 		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("serveHTTP did not return once the request finished")
 	}
-	assertPending("while a request was still in flight")
-	close(release)
-	assertPending("before the worker finished")
+}
+
+func TestServeHTTP_WaitsForTheWorkerBeforeReturning(t *testing.T) {
+	srv := httptest.NewUnstartedServer(http.NotFoundHandler())
+	srv.Start()
+	defer srv.Close()
+	ctx, cancel := context.WithCancel(context.Background())
+	workerDone := make(chan struct{})
+	returned := make(chan error, 1)
+	go func() { returned <- serveHTTP(ctx, srv.Config, srv.Listener, nil, workerDone, 5*time.Second) }() // no request in flight
+
+	cancel()
+
+	pendingFor(t, returned, "before the worker finished")
 	close(workerDone)
 	select {
 	case err := <-returned:
@@ -71,7 +100,7 @@ func TestServeHTTP_ReturnsOnlyAfterTheInFlightRequestAndTheWorkerHaveFinished(t
 			t.Fatalf("serveHTTP: %v", err)
 		}
 	case <-time.After(2 * time.Second):
-		t.Fatal("serveHTTP did not return once the request and the worker had finished")
+		t.Fatal("serveHTTP did not return once the worker finished")
 	}
 }
 

internal/uploads/intake_defaults_test.go

@@ -0,0 +1,31 @@
+package uploads_test
+
+// The limits production runs on are the DEFAULTS: nothing outside tests sets
+// MaxReceives or MaxReceivesPerUser. Pinned here with the literal numbers, so changing a
+// default is a visible decision (#47 review). With 4 receives, 20 queued and 1 running at
+// the 200 MiB cap, the spool needs ~5 GiB of the 6 Gi emptyDir (infra); raising any of
+// them or TAPIR_UPLOAD_MAX_BYTES must revisit that arithmetic.
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	"git.d-ma.be/mathias/tapir/internal/uploads"
+)
+
+func TestNewIntake_DefaultsAllowTwoReceivesPerUserAndFourOverall(t *testing.T) {
+	r := newRig(t)
+	in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
+	startSlowReceive(t, r, in, "a", 1)
+	startSlowReceive(t, r, in, "a", 2)
+
+	_, err := in.Accept(context.Background(), "a", "t", strings.NewReader("x"))
+	requireBusy(t, err) // a third body from one user
+
+	startSlowReceive(t, r, in, "b", 3)
+	startSlowReceive(t, r, in, "b", 4)
+	_, err = in.Accept(context.Background(), "c", "t", strings.NewReader("x"))
+	requireBusy(t, err) // a fifth body overall
+}

test/lintgate/lintgate_test.go

@@ -53,12 +53,17 @@ func hasSkipNotice(out string) bool {
 
 // runLint runs `task lint` in dir with the given PATH and returns its combined output and exit code.
 func runLint(t *testing.T, dir, path string) (string, int) {
+	t.Helper()
+	return runTask(t, dir, path, "lint")
+}
+
+func runTask(t *testing.T, dir, path string, args ...string) (string, int) {
 	t.Helper()
 	taskBin, err := exec.LookPath("task")
 	if err != nil {
 		t.Skip("task not installed")
 	}
-	cmd := exec.Command(taskBin, "lint")
+	cmd := exec.Command(taskBin, args...)
 	cmd.Dir = dir
 	cmd.Env = append(os.Environ(), "PATH="+path)
 	var out bytes.Buffer
@@ -87,6 +92,20 @@ func TestTaskLint_ExitsNonZeroWhenGolangciLintReportsAFinding(t *testing.T) {
 	if hasSkipNotice(out) {
 		t.Fatalf("task lint blamed a missing binary for a lint failure; output:\n%s", out)
 	}
+	if !strings.Contains(out, "errcheck") {
+		t.Fatalf("a non-zero exit without the finding means the linter crashed, not that it found something; output:\n%s", out)
+	}
+}
+
+// The lint step only gates anything if `task check` runs it.
+func TestTaskCheck_RunsTheLintStep(t *testing.T) {
+	out, code := runTask(t, scratchModule(t), os.Getenv("PATH"), "--dry", "check")
+	if code != 0 {
+		t.Fatalf("task --dry check exited %d; output:\n%s", code, out)
+	}
+	if !strings.Contains(out, "task: [lint]") {
+		t.Fatalf("task check no longer runs the lint step; output:\n%s", out)
+	}
 }
 
 func TestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsent(t *testing.T) {