diff --git a/Makefile b/Makefile index 057a916..2782e3c 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: all bootstrap setup check test lint lint-fix fmt fmt-check build clean deps test-coverage test-integration local install release release-snapshot docker hooks +.PHONY: all bootstrap setup check test lint lint-fix fmt fmt-check build clean deps test-coverage local install release release-snapshot docker hooks # Version number VERSION := 1.0.0-rc.1 @@ -27,7 +27,13 @@ setup: check: @script/check -# Run tests only. +# Run tests only. This runs the ENTIRE suite -- there is no separate +# integration target and no build-tagged subset held back. In +# particular internal/vaultik/integration_test.go, which does full +# chunk -> pack -> encrypt -> upload -> restore round-trips, runs here. +# A `test-integration` target used to exist and was removed: no file in +# the repo carried a build tag, so `-tags=integration` selected nothing +# extra and the target was an exact duplicate of this one. test: @script/test @@ -64,15 +70,14 @@ clean: deps: go mod download -# Run tests with coverage. +# Run tests with coverage. -count=1 for the same reason script/test +# uses it: without it an unchanged package is served from Go's test +# result cache, and a coverage profile assembled from cached results +# describes a run that did not happen. test-coverage: - go test -v -coverprofile=coverage.out ./... + go test -v -count=1 -coverprofile=coverage.out ./... go tool cover -html=coverage.out -o coverage.html -# Run integration tests. -test-integration: - go test -v -tags=integration ./... - local: VAULTIK_CONFIG=$(HOME)/etc/vaultik/config.yml ./vaultik snapshot --debug list 2>&1 VAULTIK_CONFIG=$(HOME)/etc/vaultik/config.yml ./vaultik snapshot --debug create 2>&1 diff --git a/README.md b/README.md index 7ebb4db..6e426e9 100644 --- a/README.md +++ b/README.md @@ -600,7 +600,19 @@ them. We provide: `script/bootstrap`, then `script/install-precommit` * `script/projectname` — print the project name (used for the Docker image tag) -* `script/test` — run the test suite (verbose rerun on failure) +* `script/test` — run the test suite (verbose rerun on failure). This + runs *everything*: there is no separate integration target and no + build-tagged subset held back, so the full round-trip tests in + `internal/vaultik/integration_test.go` run on every invocation. It + passes `-count=1`, which disables Go's test result cache. That is + deliberate and costs a few seconds on a repeat run: without it an + unchanged package prints `ok (cached)`, which is + indistinguishable from a package that really ran, so the whole suite + can report a full set of `ok` lines in under half a second having + executed nothing. The `-timeout` is a hang backstop rather than a + performance budget — it applies per test binary to test execution + only, not to compilation — and is set well above the slowest + package's measured runtime; see the comment in the script. * `script/lint` — run `golangci-lint run ./...` at the exact version CI uses, by running the digest-pinned `golangci-lint` image declared by the `Dockerfile` lint stage (requires Docker; it fails loudly rather diff --git a/TODO.md b/TODO.md index 99e478b..ce87f05 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,34 @@ Define remaining scope for a first tagged release and cut v0.1.0. # Completed Steps +- 2026-08-09: Closed the fifth false-green mechanism (issues #93, #69). + `script/test` omitted `-count=1`, so Go's test result cache could + satisfy the gate outright: a second back-to-back `make test` printed + the full set of 14 `ok` lines, every one marked `(cached)`, having + executed no test at all. Since `ok (cached)` is an `ok` line, + the "14 `ok` lines means the suite ran" signal this repo leans on was + forgeable, one level below the Docker layer cache that #85 addressed. + Fixed with `-count=1` unconditionally rather than only in the + container, because the pre-commit hook runs the same script and a + gate honest only in CI is dishonest where people rely on it most; + `test-coverage` got the same flag, and `script/check` inherits it by + calling `script/test`. In the same area, `make test-integration` was + deleted rather than made real: no file in the repo carried a build + tag, so `-tags=integration` selected nothing and the target was an + exact duplicate of `make test`. Tagging a subset was rejected because + the entire suite runs in well under a minute, and a scheme whose + failure mode is "some tests silently stopped running" is a poor trade + for those seconds in a repo with this particular history. The + `-timeout` was raised from 30s after measuring rather than after + assuming: the standing claim that cold-cache compilation is charged + against `-timeout` is **false**, disproved by a containerised run + that spent 46s compiling and still reported per-package durations + within noise of a warm host run. `-timeout` reaches the test binary + as `-test.timeout` and its clock starts inside `testing.M.Run`, after + the build. The real exposure was margin, not compilation. Numbers and + the full verification are recorded once, on the pull request, and are + deliberately not restated here. + - 2026-08-09: Triaged all fifteen stale remote branches (issue #71) and deleted fourteen of them; the full per-branch disposition with evidence is recorded on that issue. Method mattered more than the diff --git a/script/test b/script/test index f1a8a6a..621c542 100755 --- a/script/test +++ b/script/test @@ -6,11 +6,48 @@ set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +# The flags live in one function so the quiet run and the verbose rerun +# below cannot drift apart. A rerun that used different flags would +# diagnose a different program than the one that failed. +# +# -count=1 is the documented way to bypass Go's test result cache, and +# it is not optional here. Without it, a package whose inputs are +# unchanged prints `ok (cached)`, and that line is +# indistinguishable -- to every check this repo performs -- from a +# package that actually ran. The whole suite reports its full set of +# `ok` lines in under half a second having executed nothing. That +# matters beyond the local inner loop: the Dockerfile's `RUN make test` +# is forced to re-execute by CHECK_EPOCH, but a GOCACHE baked into an +# earlier image layer survives into the re-executed step, so the step +# can re-run and still do no work. It is applied unconditionally rather +# than only in the containerised path because the pre-commit hook runs +# this same script; a gate that is honest only in CI is dishonest +# exactly where people lean on it most. +# +# -timeout is a hang backstop, not a performance budget: its job is to +# turn a deadlocked test into a stack dump instead of a wedged CI job, +# so it wants to sit far above the slowest legitimate runtime, not just +# above it. It is per test binary and covers test execution only -- the +# clock starts inside testing.M.Run, after compilation and linking, so +# build time is not charged against it. (Measured: a containerised run +# with an empty GOCACHE reports per-package durations within noise of a +# warm host run. A shell `timeout 30 go test ./...` would include +# compilation, but that is a different mechanism from this flag.) The +# slowest packages are internal/database and internal/vaultik, measured +# between 6.4s and 8.1s under -race, the high end being a cold +# containerised run on a contended host. Against that 8.1s worst case +# 30s left only 3.7x headroom, thin for a loaded or throttled CI +# runner; 120s leaves about 15x while still bounding a hung package -- +# including the verbose rerun below -- to a few minutes. +run_tests() { + go test -race -timeout 120s -count=1 "$@" ./... +} + main() { cd "$ROOT" - go test -race -timeout 30s ./... || { + run_tests || { echo "--- Rerunning with -v for details ---" - go test -race -timeout 30s -v ./... + run_tests -v exit 1 } }