From 02fcd3ae8f7e6532fd8c3006e47c35c1af87de72 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sat, 3 Oct 2026 13:04:34 +0000 Subject: [PATCH] Go tests run with -race and -cover under Go's own timeout (closes #88) backend/script/test runs go test -timeout 30s -race -cover and, if that fails, runs it again with -v and fails. The root script/test drops its one 30-second timeout around both halves: from a cold Go build cache, compiling the tests with -race used it all up. Each half keeps its own limit. The race detector needs a C compiler: the Dockerfile builder stage gains gcc and musl-dev, and script/bootstrap installs gcc, with the C library headers on apt and apk, when gcc is missing; make build still sets CGO_ENABLED=0. New tests: the health check's answer, a valid report's answer, a report file's exact lines, and the flush at the 10 MiB threshold. The handlers TestImport stub is gone. Model: opus-5-5 --- Dockerfile | 5 +- README.md | 12 +- TODO.md | 11 ++ backend/README.md | 5 +- backend/internal/handlers/handlers_test.go | 13 -- backend/internal/handlers/healthcheck_test.go | 120 ++++++++++++++++++ backend/internal/handlers/report_test.go | 57 +++++++++ backend/internal/reportbuf/export_test.go | 4 + backend/internal/reportbuf/reportbuf_test.go | 105 +++++++++++++-- backend/script/test | 12 +- script/bootstrap | 13 +- script/test | 9 +- 12 files changed, 328 insertions(+), 38 deletions(-) delete mode 100644 backend/internal/handlers/handlers_test.go create mode 100644 backend/internal/handlers/healthcheck_test.go diff --git a/Dockerfile b/Dockerfile index 612d8a7..30ddf6b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -20,7 +20,10 @@ RUN make lint # golang:1.25-alpine (2026-02-27) FROM golang:1.25-alpine@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4ced AS builder -RUN apk add --no-cache git make +# gcc and musl-dev are for make test: its race detector needs cgo, which +# Go turns on by itself once a C compiler is present. make build still +# sets CGO_ENABLED=0, so the binary stays static. +RUN apk add --no-cache gcc git make musl-dev WORKDIR /src diff --git a/README.md b/README.md index 0db2a61..e604338 100644 --- a/README.md +++ b/README.md @@ -39,14 +39,16 @@ halves, so the root `make check` fails if either one is broken. We provide: - `script/bootstrap` — install all dependencies (the pinned node via nvm unless one new enough for the frontend's dependencies is installed, yarn via corepack, `yarn install --frozen-lockfile`, the pinned Go unless one at least - as new as `backend/go.mod` asks for is installed, and the Go modules), linking - what it installs itself into `~/.local/bin`, which has to be on `PATH`. It - installs no Go linter and not Docker: `make lint` runs the linter in Docker + as new as `backend/go.mod` asks for is installed, the Go modules, and gcc with + the C library headers unless gcc is installed, for the race detector in + `make test`), linking what it installs itself into `~/.local/bin`, which has + to be on `PATH`. It installs no Go linter and not Docker: `make lint` runs the + linter in Docker - `script/setup` — make a fresh clone ready for development: bootstrap plus the git pre-commit hook - `script/projectname` — print the project name (used for the Docker image tag) -- `script/test` — run `script/frontend-test`, then the backend's Go tests, both - within one 30-second timeout +- `script/test` — run `script/frontend-test`, then the backend's Go tests, each + under its own 30-second timeout - `script/lint` — run `script/frontend-lint`, then golangci-lint in Docker, by building the lint stage of `Dockerfile` without the cache - `script/fmt` — format all files (writes): prettier, then gofmt over `backend/` diff --git a/TODO.md b/TODO.md index a5c802e..536f37f 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,17 @@ latest run passes. # Completed Steps +- 2026-10-03: the Go tests run with the race detector and coverage (issue #88): + `backend/script/test` runs `go test -timeout 30s -race -cover ./...` and, if + that fails, runs it again with `-v` and fails. Go's `-timeout` bounds the + tests, not their compile; the root `script/test` no longer puts one 30-second + timeout around both halves, which a cold Go build cache could use up on + compiling alone. The race detector needs a C compiler: the builder stage of + `Dockerfile` has gcc and musl-dev, and `script/bootstrap` installs gcc, with + the C library headers on apt and apk, when gcc is missing; the binary is still + built with `CGO_ENABLED=0`. New tests cover the health check's answer, a valid + report's answer, a report file's exact contents, and the flush when the buffer + reaches 10 MiB; the handlers' `TestImport` stub is gone - 2026-10-03: each target check times out after 80% of the refresh interval (issue #78), 24 seconds at 30 seconds, where it was capped at 3 seconds. A round started early, after an interval change or when the recovery probe finds diff --git a/backend/README.md b/backend/README.md index 82ca05e..0161cc4 100644 --- a/backend/README.md +++ b/backend/README.md @@ -35,7 +35,10 @@ pattern as the repo root: the targets in `backend/Makefile` are thin shims over stamped in. The version is `VERSION` from the environment; when that is unset or empty, it falls back to `git describe` inside a git checkout, then to `dev` -- `script/test` — run the Go tests under a 30-second timeout +- `script/test` — run the Go tests with the race detector and coverage. Go's + `-timeout 30s` bounds the tests, not their compile. If they fail, they run + again with `-v` for the details, and the script fails. The race detector needs + a C compiler - `script/lint` — check `.golangci.yml` against its pinned sha256, then run golangci-lint. It runs inside the golangci-lint image of the lint stage of the root `Dockerfile`; from a checkout, run `make lint` at the repo root, diff --git a/backend/internal/handlers/handlers_test.go b/backend/internal/handlers/handlers_test.go deleted file mode 100644 index 5128215..0000000 --- a/backend/internal/handlers/handlers_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package handlers_test - -import ( - "testing" - - _ "sneak.berlin/go/netwatch/internal/handlers" -) - -func TestImport(t *testing.T) { - t.Parallel() - // Compilation check — verifies the package parses - // and all imports resolve. -} diff --git a/backend/internal/handlers/healthcheck_test.go b/backend/internal/handlers/healthcheck_test.go new file mode 100644 index 0000000..dfbfa31 --- /dev/null +++ b/backend/internal/handlers/healthcheck_test.go @@ -0,0 +1,120 @@ +package handlers_test + +import ( + "encoding/json" + "maps" + "net/http" + "net/http/httptest" + "slices" + "testing" + "time" + + "sneak.berlin/go/netwatch/internal/globals" + "sneak.berlin/go/netwatch/internal/handlers" + "sneak.berlin/go/netwatch/internal/healthcheck" + "sneak.berlin/go/netwatch/internal/logger" + + "go.uber.org/fx/fxtest" +) + +// newStartedHandlers builds Handlers with a real health check for the +// server named in g, and starts them, which records the time the +// uptime counts from. +func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers { + t.Helper() + + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.Params{Globals: g}) + if err != nil { + t.Fatalf("logger: %v", err) + } + + hc, err := healthcheck.New(lc, + healthcheck.Params{Globals: g, Logger: log}) + if err != nil { + t.Fatalf("health check: %v", err) + } + + h, err := handlers.New(lc, + handlers.Params{Globals: g, Healthcheck: hc, Logger: log}) + if err != nil { + t.Fatalf("handlers: %v", err) + } + + lc.RequireStart() + t.Cleanup(lc.RequireStop) + + return h +} + +// TestHandleHealthCheck checks the health check's answer: 200, a JSON +// content type, and a JSON object with exactly the fields of +// healthcheck.Response, carrying this server's name and version and +// an uptime counted from its start. +func TestHandleHealthCheck(t *testing.T) { + t.Parallel() + + g := &globals.Globals{Appname: "netwatch-server", Version: "v1.2.3"} + h := newStartedHandlers(t, g) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/.well-known/healthcheck", http.NoBody) + + h.HandleHealthCheck().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + contentType := rec.Header().Get("Content-Type") + if contentType != "application/json; charset=utf-8" { + t.Errorf("Content-Type = %q, want %q", + contentType, "application/json; charset=utf-8") + } + + var body map[string]any + + err := json.Unmarshal(rec.Body.Bytes(), &body) + if err != nil { + t.Fatalf("body not a JSON object: %v (%q)", err, rec.Body.String()) + } + + fields := []string{ + "appname", "now", "status", "uptimeHuman", "uptimeSeconds", "version", + } + if got := slices.Sorted(maps.Keys(body)); !slices.Equal(got, fields) { + t.Fatalf("fields = %v, want %v", got, fields) + } + + for field, want := range map[string]string{ + "appname": g.Appname, "status": "ok", "version": g.Version, + } { + if body[field] != want { + t.Errorf("%s = %v, want %q", field, body[field], want) + } + } + + now, _ := body["now"].(string) + + at, err := time.Parse(time.RFC3339Nano, now) + if err != nil || time.Since(at).Abs() > time.Minute { + t.Errorf("now = %q, want the current time in RFC 3339 (%v)", now, err) + } + + // Started just now, so the uptime is well under a minute. + human, _ := body["uptimeHuman"].(string) + + uptime, err := time.ParseDuration(human) + if err != nil || uptime > time.Minute { + t.Errorf("uptimeHuman = %q, want a duration under a minute (%v)", + human, err) + } + + seconds, ok := body["uptimeSeconds"].(float64) + if !ok || seconds < 0 || seconds > time.Minute.Seconds() { + t.Errorf("uptimeSeconds = %v, want a number of seconds under a minute", + body["uptimeSeconds"]) + } +} diff --git a/backend/internal/handlers/report_test.go b/backend/internal/handlers/report_test.go index cd94b5c..c906b52 100644 --- a/backend/internal/handlers/report_test.go +++ b/backend/internal/handlers/report_test.go @@ -46,6 +46,63 @@ func decodeStatus(t *testing.T, body []byte) string { return resp.Status } +// TestHandleReportAcceptsValidReports checks the answer to a valid +// report, one with no hosts and one shaped as the frontend sends them: +// 200 and {"status":"ok"} as JSON. +func TestHandleReportAcceptsValidReports(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + }{ + { + name: "no hosts", + body: `{"clientId":"c1","geo":null,"hosts":[],` + + `"timestamp":"2026-10-03T12:00:00.000Z"}`, + }, + { + name: "a host with a latency and an error sample", + body: `{"clientId":"c1","geo":null,"hosts":[{` + + `"name":"Example","url":"https://example.com/",` + + `"status":"error","history":[` + + `{"t":1790000000000,"latency":42,"error":null},` + + `{"t":1790000003000,"latency":null,"error":"timeout"}]}],` + + `"timestamp":"2026-10-03T12:00:00.000Z"}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandlers(stubAppender{}, io.Discard) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(tt.body), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + contentType := rec.Header().Get("Content-Type") + if contentType != "application/json; charset=utf-8" { + t.Errorf("Content-Type = %q, want %q", + contentType, "application/json; charset=utf-8") + } + + if got := rec.Body.String(); got != "{\"status\":\"ok\"}\n" { + t.Errorf("body = %q, want %q", got, "{\"status\":\"ok\"}\n") + } + }) + } +} + func TestHandleReportStorageFailureIsNon2xx(t *testing.T) { t.Parallel() diff --git a/backend/internal/reportbuf/export_test.go b/backend/internal/reportbuf/export_test.go index efe8133..845feff 100644 --- a/backend/internal/reportbuf/export_test.go +++ b/backend/internal/reportbuf/export_test.go @@ -2,6 +2,10 @@ package reportbuf import "time" +// FlushSizeThreshold exposes the buffer size at which Append starts +// writing a report file to the external tests. +const FlushSizeThreshold = flushSizeThreshold + // Flush writes the buffered reports to a file now, as the periodic // flush does, so tests need not wait a minute for it. func (b *Buffer) Flush() error { diff --git a/backend/internal/reportbuf/reportbuf_test.go b/backend/internal/reportbuf/reportbuf_test.go index 0b56b7e..f9b020e 100644 --- a/backend/internal/reportbuf/reportbuf_test.go +++ b/backend/internal/reportbuf/reportbuf_test.go @@ -334,7 +334,11 @@ func TestTwoFlushesInOneMillisecond(t *testing.T) { } } - files := readReportFiles(t, dir) + files, err := readReportFiles(dir) + if err != nil { + t.Fatalf("read report files: %v", err) + } + if len(files) != flushes { t.Fatalf("%d report files after %d flushes", len(files), flushes) } @@ -347,6 +351,88 @@ func TestTwoFlushesInOneMillisecond(t *testing.T) { } } +// TestReportFileHoldsTheLinesAppended flushes three reports and reads +// their file back: it must decompress to exactly their JSON lines, in +// the order they were appended. +func TestReportFileHoldsTheLinesAppended(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + buf := startBuffer(t) + + for _, id := range []int{1, 2, 3} { + err := buf.Append(map[string]int{"id": id}) + if err != nil { + t.Fatalf("append report %d: %v", id, err) + } + } + + err := buf.Flush() + if err != nil { + t.Fatalf("flush: %v", err) + } + + files, err := readReportFiles(dir) + if err != nil { + t.Fatalf("read report files: %v", err) + } + + want := `{"id":1}` + "\n" + `{"id":2}` + "\n" + `{"id":3}` + "\n" + if len(files) != 1 || files[0] != want { + t.Fatalf("report files = %q, want one holding %q", files, want) + } +} + +// TestFlushAtSizeThreshold appends reports until the buffer holds +// FlushSizeThreshold bytes. The append that gets it there must write +// them all to one report file, with no call to Flush and the periodic +// flush a minute away, and no earlier append may write one. +func TestFlushAtSizeThreshold(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + buf := startBuffer(t) + + // Large reports, so the threshold takes a few hundred appends. + pad := strings.Repeat("a", 64<<10) + + var appended strings.Builder + + for id := 0; appended.Len() < reportbuf.FlushSizeThreshold; id++ { + report := map[string]any{"id": id, "pad": pad} + + err := buf.Append(report) + if err != nil { + t.Fatalf("append report %d: %v", id, err) + } + + line, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal report %d: %v", id, err) + } + + appended.Write(line) + appended.WriteByte('\n') + } + + // Append writes the file in the background, so wait for it. + deadline := time.Now().Add(10 * time.Second) + + for { + files, err := readReportFiles(dir) + if err == nil && len(files) == 1 && files[0] == appended.String() { + return + } + + if time.Now().After(deadline) { + t.Fatalf("%d report files (error: %v), want one holding the "+ + "%d bytes appended", len(files), err, appended.Len()) + } + + time.Sleep(10 * time.Millisecond) + } +} + // reportFilesBytes returns the total size of the report files in dir. func reportFilesBytes(t *testing.T, dir string) int64 { t.Helper() @@ -371,20 +457,19 @@ func reportFilesBytes(t *testing.T, dir string) int64 { } // readReportFiles returns the decompressed contents of each report -// file in dir. -func readReportFiles(t *testing.T, dir string) []string { - t.Helper() - +// file in dir. A file still being written does not decompress, so it +// gives an error. +func readReportFiles(dir string) ([]string, error) { files := os.DirFS(dir) names, err := fs.Glob(files, "reports-*.jsonl.zst") if err != nil { - t.Fatalf("list report files: %v", err) + return nil, fmt.Errorf("list report files: %w", err) } dec, err := zstd.NewReader(nil) if err != nil { - t.Fatalf("create zstd decoder: %v", err) + return nil, fmt.Errorf("create zstd decoder: %w", err) } defer dec.Close() @@ -393,18 +478,18 @@ func readReportFiles(t *testing.T, dir string) []string { for _, name := range names { compressed, readErr := fs.ReadFile(files, name) if readErr != nil { - t.Fatalf("read %s: %v", name, readErr) + return nil, fmt.Errorf("read %s: %w", name, readErr) } data, decErr := dec.DecodeAll(compressed, nil) if decErr != nil { - t.Fatalf("decompress %s: %v", name, decErr) + return nil, fmt.Errorf("decompress %s: %w", name, decErr) } contents = append(contents, string(data)) } - return contents + return contents, nil } func writeBytes(t *testing.T, path string, n int) { diff --git a/backend/script/test b/backend/script/test index cbfc889..a4063e3 100755 --- a/backend/script/test +++ b/backend/script/test @@ -1,12 +1,20 @@ #!/bin/sh -# script/test: run the backend test suite. +# script/test: run the backend test suite with the race detector and +# coverage. Go's own -timeout bounds the tests and not their compile, +# so a cold build cache cannot fail it. The race detector needs cgo, +# and so a C compiler. If the tests fail, they run again with -v for +# the details, and the script fails even if that run passes. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - timeout 30 go test ./... + go test -timeout 30s -race -cover ./... || { + echo "--- Rerunning with -v for details ---" + go test -timeout 30s -race -v ./... + exit 1 + } } main "$@" diff --git a/script/bootstrap b/script/bootstrap index d807ee3..abe1d50 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -64,7 +64,8 @@ detect_pkgmgr() { fi } -# pkg_install +# pkg_install : the apt +# and apk arguments may each list several packages, separated by spaces. pkg_install() { detect_pkgmgr case "$PKGMGR" in @@ -74,10 +75,10 @@ pkg_install() { $SUDO env DEBIAN_FRONTEND=noninteractive apt-get update APT_UPDATED=1 fi - $SUDO env DEBIAN_FRONTEND=noninteractive apt-get install -y "$2" + $SUDO env DEBIAN_FRONTEND=noninteractive apt-get install -y $2 ;; brew) brew install "$3" ;; - apk) apk add --no-cache "$4" ;; + apk) apk add --no-cache $4 ;; esac } @@ -253,6 +254,12 @@ main() { if missing make; then pkg_install gnumake make make make; fi if missing git; then pkg_install git git git git; fi + # The race detector in make test needs cgo, which Go turns on only + # when it finds its C compiler, gcc on Linux. apt and apk ship the C + # library headers apart from gcc. + if missing gcc; then + pkg_install gcc "gcc libc6-dev" gcc "gcc musl-dev" + fi ensure_node ensure_yarn diff --git a/script/test b/script/test index f0ad7e8..d48d3ca 100755 --- a/script/test +++ b/script/test @@ -1,14 +1,17 @@ #!/bin/sh # script/test: run the test suite for the whole repo: the frontend at -# the repo root, then the Go backend in backend/. Both halves together -# get 30 seconds; each also keeps its own limit for the Dockerfiles. +# the repo root, then the Go backend in backend/. Each half has its own +# 30-second limit, and there is none around both: from a cold Go build +# cache, compiling the backend's tests with the race detector can take +# 30 seconds on its own, and Go's -timeout leaves the compile out. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - timeout 30 sh -c 'script/frontend-test && backend/script/test' + script/frontend-test + backend/script/test } main "$@" -- 2.54.0