From bd208661c6e3a173cc808c054f2f77bf8c19f861 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sat, 5 Sep 2026 19:52:55 -0500 Subject: [PATCH] build: run every workspace module's tests, not just the root's go test ./... only ever covers the module it runs from, so the scanner's 17 test files were never reached by make test. Everything written for it this week was dormant: the mock hold, the manifest corpus, the blob integrity suite, the concurrency scenarios. The same shape as the billing tests a few commits back, which is why the module list is derived from go.work rather than written out, so adding a module to the workspace is enough to get it tested. Wiring it up immediately failed, which is the argument for having done it. TestSBOMIsStableAcrossScanDirectories asks the two SPDX keys that still vary between encodings to keep varying, so that whoever makes the digest stable is told to tighten the test rather than discovering the slack later. But creationInfo.created has one-second granularity, so two scans inside the same second produce an identical value and the assertion becomes a coin flip on how fast the machine is. It passed under -race, which is slow enough to straddle a second, and failed under -cover, which is not. Only documentNamespace can carry that signal, since its UUID is redrawn on every encode. creationInfo stays in the residue set, because the digest comparison must still exclude it; it is simply no longer asked to prove anything. Stable across repeated runs under -cover. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01U1Km3N3uUmeGaj7VbaM8PF --- Makefile | 31 ++++++++++++++++++++++++++--- scanner/internal/scan/syft_test.go | 32 ++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 6530353..51ef59b 100644 --- a/Makefile +++ b/Makefile @@ -96,9 +96,32 @@ build-trixie: generate ## Build all production binaries (appview, hold, credenti ##@ Test Targets -test: test-billing ## Run all tests +# Every module in the workspace except the root. `go test ./...` only ever +# covers the module it is run from, so a workspace module needs its own +# invocation or its tests never run at all. That is how the scanner's suite sat +# dark: 17 test files, none of them reached by `make test`. +# +# Derived from go.work rather than listed by hand, so adding a module to the +# workspace is enough to get it tested. Falls back to the known set if the +# go tool cannot answer, so a broken workspace fails loudly in the test run +# rather than silently testing less. +GO_SUBMODULE_DIRS := $(shell go list -m -f '{{.Dir}}' 2>/dev/null | tail -n +2) +ifeq ($(strip $(GO_SUBMODULE_DIRS)),) +GO_SUBMODULE_DIRS := scanner cmd/credential-helper/atcr cmd/credential-helper/seamark deploy/upcloud +endif + +# $(call test-submodules,) +define test-submodules + @for dir in $(GO_SUBMODULE_DIRS); do \ + echo "→ Testing module $$(basename $$dir)..."; \ + (cd $$dir && go test $(1) ./...) || exit 1; \ + done +endef + +test: test-billing ## Run all tests (every workspace module) @echo "→ Running tests..." go test -cover ./... + $(call test-submodules,-cover) # pkg/billing is behind the `billing` build tag, so `go test ./...` never # compiles it, let alone runs it. Its tests covered the money path and had @@ -107,15 +130,17 @@ test-billing: ## Run the billing-tagged tests (skipped by plain `go test ./...`) @echo "→ Running billing-tagged tests..." go test -tags billing -cover ./pkg/billing/... -test-race: ## Run tests with race detector +test-race: ## Run tests with race detector (every workspace module) @echo "→ Running tests with race detector..." go test -race ./... @echo "→ Running billing-tagged tests with race detector..." go test -race -tags billing ./pkg/billing/... + $(call test-submodules,-race) -test-verbose: ## Run tests with verbose output +test-verbose: ## Run tests with verbose output (every workspace module) @echo "→ Running tests with verbose output..." go test -v ./... + $(call test-submodules,-v) integration-test: ## Run in-process smoke test (no docker, fake PDS + gofakes3 + hold + appview) @echo "→ Running integration smoke test..." diff --git a/scanner/internal/scan/syft_test.go b/scanner/internal/scan/syft_test.go index b4cf159..d25744d 100644 --- a/scanner/internal/scan/syft_test.go +++ b/scanner/internal/scan/syft_test.go @@ -158,14 +158,25 @@ func writeTestLayout(t *testing.T, files map[string][]byte) string { // So sbomDigest is still not stable across a rescan, even though nothing // scan-local reaches the document any more. That is a separate defect with a // separate cause, and this set is what keeps the two apart: if a third key -// starts moving, the assertions below fail rather than shrugging, and if these -// two stop moving the test says so and asks to be tightened to a plain digest -// comparison. +// starts moving, the assertions below fail rather than shrugging. var scanLocalResidue = map[string]string{ "creationInfo": "creationInfo.created is time.Now() inside Syft's SPDX encoder", "documentNamespace": "documentNamespace embeds a random UUID inside Syft's SPDX encoder", } +// alwaysVaries is the residue key that is a sound signal of the underlying +// defect. If these two keys ever stop moving, sbomDigest can become stable and +// this test should be tightened to a plain digest comparison — but only +// documentNamespace can say so, because its UUID is redrawn on every encode. +// +// creationInfo cannot: "created" has one-second granularity, so two scans that +// land inside the same second produce an identical value. Asserting that it +// varies makes the test a coin flip on how fast the machine is, which is +// exactly how it failed the first time the module was wired into `make test` +// (passing under -race, which is slow enough to straddle a second, and failing +// under -cover, which is not). +const alwaysVaries = "documentNamespace" + // TestSBOMIsStableAcrossScanDirectories is the requirement, as far as this // package can carry it: the same image scanned twice from two different // scratch directories must produce the same SBOM, apart from the two encoder @@ -224,13 +235,14 @@ func TestSBOMIsStableAcrossScanDirectories(t *testing.T) { t.Errorf("SBOM documentName is %s, want the manifest digest %q", got, testManifestDigest) } - // If the residue is ever fixed upstream or worked around here, this test - // should become a plain digest equality check. - for key, why := range scanLocalResidue { - if fieldOf(t, jsonA, key) == fieldOf(t, jsonB, key) { - t.Errorf("%q no longer varies between scans (%s); sbomDigest may now be stable, "+ - "so drop it from scanLocalResidue and assert digest equality directly", key, why) - } + // 5. If the residue is ever fixed upstream or worked around here, this test + // should become a plain digest equality check. Only documentNamespace is + // asked, for the reason on alwaysVaries: a matching creationInfo means + // the clock did not tick, not that the encoder became deterministic. + if fieldOf(t, jsonA, alwaysVaries) == fieldOf(t, jsonB, alwaysVaries) { + t.Errorf("%q no longer varies between scans (%s); sbomDigest may now be stable, "+ + "so drop scanLocalResidue and assert digest equality directly", + alwaysVaries, scanLocalResidue[alwaysVaries]) } }