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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1Km3N3uUmeGaj7VbaM8PF
This commit is contained in:
Evan Jarrett
2026-09-05 19:52:55 -05:00
co-authored by Claude Opus 5
parent 29fe24aa41
commit bd208661c6
2 changed files with 50 additions and 13 deletions
+28 -3
View File
@@ -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,<go test flags>)
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..."
+22 -10
View File
@@ -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])
}
}