diff --git a/cmd/tapir/shutdown_test.go b/cmd/tapir/shutdown_test.go index 610a782..b70a328 100644 --- a/cmd/tapir/shutdown_test.go +++ b/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") } } diff --git a/internal/uploads/intake_defaults_test.go b/internal/uploads/intake_defaults_test.go new file mode 100644 index 0000000..c51d2c9 --- /dev/null +++ b/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 +} diff --git a/test/lintgate/lintgate_test.go b/test/lintgate/lintgate_test.go index 560e2ca..a971f61 100644 --- a/test/lintgate/lintgate_test.go +++ b/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) {