Make the test gate unfakeable and stop test-integration lying (closes #93) #98
21
Makefile
21
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 number
|
||||||
VERSION := 1.0.0-rc.1
|
VERSION := 1.0.0-rc.1
|
||||||
@@ -27,7 +27,13 @@ setup:
|
|||||||
check:
|
check:
|
||||||
@script/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:
|
test:
|
||||||
@script/test
|
@script/test
|
||||||
|
|
||||||
@@ -64,15 +70,14 @@ clean:
|
|||||||
deps:
|
deps:
|
||||||
go mod download
|
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:
|
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
|
go tool cover -html=coverage.out -o coverage.html
|
||||||
|
|
||||||
# Run integration tests.
|
|
||||||
test-integration:
|
|
||||||
go test -v -tags=integration ./...
|
|
||||||
|
|
||||||
local:
|
local:
|
||||||
VAULTIK_CONFIG=$(HOME)/etc/vaultik/config.yml ./vaultik snapshot --debug list 2>&1
|
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
|
VAULTIK_CONFIG=$(HOME)/etc/vaultik/config.yml ./vaultik snapshot --debug create 2>&1
|
||||||
|
|||||||
19
README.md
19
README.md
@@ -600,7 +600,24 @@ them. We provide:
|
|||||||
`script/bootstrap`, then `script/install-precommit`
|
`script/bootstrap`, then `script/install-precommit`
|
||||||
* `script/projectname` — print the project name (used for the Docker
|
* `script/projectname` — print the project name (used for the Docker
|
||||||
image tag)
|
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 it is not free: on this repo's suite it costs about 11
|
||||||
|
seconds on every repeat run (measured, back to back: 0.4s cached
|
||||||
|
versus 11.6s with `-count=1`). That is the price of the run meaning
|
||||||
|
anything, because without it an unchanged package prints
|
||||||
|
`ok <pkg> (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. Its 120s value deliberately
|
||||||
|
diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in
|
||||||
|
the comment in the script, and issue #101 proposes amending the policy
|
||||||
|
text.
|
||||||
* `script/lint` — run `golangci-lint run ./...` at the exact version CI
|
* `script/lint` — run `golangci-lint run ./...` at the exact version CI
|
||||||
uses, by running the digest-pinned `golangci-lint` image declared by
|
uses, by running the digest-pinned `golangci-lint` image declared by
|
||||||
the `Dockerfile` lint stage (requires Docker; it fails loudly rather
|
the `Dockerfile` lint stage (requires Docker; it fails loudly rather
|
||||||
|
|||||||
33
TODO.md
33
TODO.md
@@ -18,6 +18,39 @@ Define remaining scope for a first tagged release and cut v0.1.0.
|
|||||||
|
|
||||||
# Completed Steps
|
# 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 <pkg> (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. The 120s
|
||||||
|
landed on is a **deliberate, documented divergence** from
|
||||||
|
`REPO_POLICIES.md:192`, which mandates 30s, and from that file's
|
||||||
|
canonical recipe at `:212-214`; the divergence is recorded in
|
||||||
|
`script/test`'s comment because `REPO_POLICIES.md` is org-canonical
|
||||||
|
and not editable here, and issue #101 proposes amending the policy
|
||||||
|
text upstream. 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
|
- 2026-08-09: Triaged all fifteen stale remote branches (issue #71) and
|
||||||
deleted fourteen of them; the full per-branch disposition with
|
deleted fourteen of them; the full per-branch disposition with
|
||||||
evidence is recorded on that issue. Method mattered more than the
|
evidence is recorded on that issue. Method mattered more than the
|
||||||
|
|||||||
55
script/test
55
script/test
@@ -6,11 +6,62 @@ set -eu
|
|||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
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 <pkg> (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 120s value DELIBERATELY DIVERGES from REPO_POLICIES.md:192, which
|
||||||
|
# mandates "Add a 30-second timeout", and from that file's canonical Go
|
||||||
|
# recipe at :212-214, which uses -timeout 30s. REPO_POLICIES.md is
|
||||||
|
# org-canonical and cannot be amended from this repo, so the divergence
|
||||||
|
# is recorded here instead, and issue #101 proposes amending the policy
|
||||||
|
# text upstream. Do not revert this to 30s without reading #101 first.
|
||||||
|
#
|
||||||
|
# Why it diverges: the slowest packages are internal/database and
|
||||||
|
# internal/vaultik, observed under -race at about 6.4s warm, 8.1s in a
|
||||||
|
# cold containerised run on a contended host, and 10.2s in an
|
||||||
|
# independent cold run on this same host. The worst case is not tightly
|
||||||
|
# characterised -- each fresh measurement has come in above the last --
|
||||||
|
# which is itself an argument for generous headroom. Against the 10.2s
|
||||||
|
# observation, 30s is only 2.9x: not a safety margin but a flake
|
||||||
|
# waiting for a slow day, whose failure mode is a timeout that looks
|
||||||
|
# like a real defect. 120s leaves about 12x while still bounding a hung
|
||||||
|
# package -- including the verbose rerun below -- to a few minutes. The
|
||||||
|
# cost of that choice, also recorded on #101: because of the rerun, a
|
||||||
|
# hung package pays the timeout twice.
|
||||||
|
run_tests() {
|
||||||
|
go test -race -timeout 120s -count=1 "$@" ./...
|
||||||
|
}
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
go test -race -timeout 30s ./... || {
|
run_tests || {
|
||||||
echo "--- Rerunning with -v for details ---"
|
echo "--- Rerunning with -v for details ---"
|
||||||
go test -race -timeout 30s -v ./...
|
run_tests -v
|
||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user