test: testing standards + first-batch coverage climb (16% → 17.7%) + integration suite repair

.github/workflows/dco.yml

@@ -57,7 +57,18 @@ jobs:
         run: |
           set -euo pipefail
           # List commits introduced by this PR (BASE..HEAD).
-          commits=$(git log --format='%H' "${BASE_SHA}..${HEAD_SHA}")
+          #
+          # --no-merges skips merge commits (commits with 2+ parents).
+          # Merge commits are integration points without authored
+          # content — their diff is computed automatically from the
+          # parents — so DCO does not apply to them. The Probot DCO
+          # app and the Linux-kernel checkpatch tooling both skip
+          # merge commits for the same reason. This also means
+          # GitHub's "Update branch" button (which generates an
+          # unsigned-off merge commit) no longer breaks the check.
+          # Authored commits on both sides of the merge are still
+          # verified because they appear individually in BASE..HEAD.
+          commits=$(git log --no-merges --format='%H' "${BASE_SHA}..${HEAD_SHA}")
           if [ -z "$commits" ]; then
             echo "No new commits in this PR — nothing to check."
             exit 0

docs/CONTRIBUTING.md

@@ -13,7 +13,8 @@ change in front of maintainers and what we look for in a clean PR.
 3. **Sign your commits.** SSH or GPG, your choice. `main` enforces signed
    commits; unsigned PRs are blocked at merge.
 4. **Write tests** for behavioural changes. The bar is "is this change
-   regressable?" — if yes, add a test.
+   regressable?" — if yes, add a test. Follow the conventions in
+   [`TESTING.md`](TESTING.md) (gomega + table-driven + sub-tests).
 5. **Don't suppress findings.** Lint, SAST, vuln-scan, and Scorecard
    warnings are signals, not noise. If a finding is a real false
    positive, document why in the PR description, not via
@@ -43,13 +44,22 @@ go build ./...
 # Unit tests
 go test ./...
 
+# Unit + integration tests
+go test -tags integration ./...
+
+# Coverage (uses the welder task with entry-point + mock exclusions)
+welder run coverage
+
 # Fuzz tests (HMAC cache parse path)
 go test -run='^$' -fuzz=FuzzVerifyAndExtract -fuzztime=30s ./pkg/security/
 
 # Lint
 golangci-lint run    # if installed locally
 ```
 
+See [`TESTING.md`](TESTING.md) for the full set of test invocations,
+fixture conventions, and the coverage policy.
+
 CI runs golangci-lint, `go vet`, `staticcheck`, Semgrep, CodeQL, and
 fuzz on every PR — set up local tooling to catch most of these before
 push to save round-trips.

docs/TESTING.md

@@ -0,0 +1,330 @@
+# Testing standards
+
+This document codifies the testing conventions for the
+`simple-container-com/api` repo so contributors can write tests that
+fit the existing style, and so the project can grow toward the
+OpenSSF Best Practices statement-coverage targets (Silver: ≥ 80 %,
+Gold: ≥ 90 %).
+
+It satisfies the OpenSSF Best Practices criteria `test_invocation`,
+`test_continuous_integration`, `tests_documented_added`,
+`test_policy_mandatory`, and is the contract that
+[`docs/CONTRIBUTING.md`](CONTRIBUTING.md) points at when it requires
+new tests for every code change.
+
+## Current state — to be updated each pass
+
+| Metric | Value as of 2026-05-19 |
+|---|---|
+| Total test files | 87 (`*_test.go`) |
+| Test files using **gomega** | 67 (77 %) |
+| Test files using **testify** | 3 (3 %; mock-only) |
+| Test files using **plain `testing`** | 19 (22 %; mostly fuzz + small utilities) |
+| Table-driven tests | 43 files |
+| Sub-tests via `t.Run` | 66 files |
+| Integration tests (`*_integration_test.go`) | 5 packages |
+| Mocks generated by `mockery v2.53.4` | `pkg/api/git/mocks/`, `pkg/clouds/pulumi/mocks/` |
+| **Overall statement coverage** | **~16 %** |
+| Coverage on `pkg/security/...` | 42 – 66 % per sub-pkg |
+
+These numbers are the baseline. Every PR should hold or improve them.
+
+## Test framework — choose gomega
+
+Use [`github.com/onsi/gomega`](https://onsi.github.io/gomega/) with the
+standard Go `testing` runner. Do **not** introduce Ginkgo (BDD-style
+`Describe` / `Context` blocks) — the existing tests use gomega's
+matchers directly inside `func TestX(t *testing.T)` functions, and
+mixing styles fragments the codebase.
+
+### Canonical shape
+
+```go
+package mypkg
+
+import (
+    "testing"
+
+    . "github.com/onsi/gomega"
+)
+
+func TestThing(t *testing.T) {
+    RegisterTestingT(t)            // bind gomega to *testing.T
+
+    got, err := Thing("input")
+    Expect(err).ToNot(HaveOccurred())
+    Expect(got).To(Equal("output"))
+}
+```
+
+### Sub-tests
+
+```go
+func TestThing(t *testing.T) {
+    t.Run("happy path", func(t *testing.T) {
+        RegisterTestingT(t)
+        // ...
+    })
+    t.Run("rejects empty input", func(t *testing.T) {
+        RegisterTestingT(t)
+        // ...
+    })
+}
+```
+
+`RegisterTestingT` **must** be called inside each sub-test — it binds
+gomega's failure handler to the current `t`, and the parent binding
+does not propagate.
+
+### Table-driven tests
+
+For input → output coverage, write a table. The repo already has 43
+examples; a representative one is
+[`pkg/clouds/pulumi/kubernetes/simple_container_parentenv_test.go`](../pkg/clouds/pulumi/kubernetes/simple_container_parentenv_test.go).
+
+```go
+func TestThing(t *testing.T) {
+    cases := []struct {
+        name string
+        in   string
+        want string
+    }{
+        {"happy", "foo", "FOO"},
+        {"empty input", "", ""},
+        {"unicode", "ümlaut", "ÜMLAUT"},
+    }
+    for _, tc := range cases {
+        t.Run(tc.name, func(t *testing.T) {
+            RegisterTestingT(t)
+            got := Thing(tc.in)
+            Expect(got).To(Equal(tc.want))
+        })
+    }
+}
+```
+
+Use human-readable `name` values — they show up in failure messages
+and in CI run output.
+
+### Preferred matchers (rank order, by repo usage)
+
+| Matcher | When to use |
+|---|---|
+| `Equal(x)` | exact value comparison (works for primitives, slices, maps, structs) |
+| `BeNil()` / `BeTrue()` / `BeFalse()` | nilness / boolean truth |
+| `HaveOccurred()` / `ToNot(HaveOccurred())` | error / no-error assertions |
+| `Succeed()` | `Expect(f()).To(Succeed())` reads better than `ToNot(HaveOccurred())` when `f()` returns only an error |
+| `ContainSubstring(s)` | string-contains assertions on log lines / error messages |
+| `HaveLen(n)` | exact length |
+| `ContainElement(x)` | slice/array membership |
+| `HaveKey(k)` / `HaveKeyWithValue(k, v)` | map assertions |
+| `BeEquivalentTo(x)` | type-agnostic compare (rarely needed; prefer `Equal` after type-asserting) |
+
+Avoid `MatchError` for one-shot string checks — use
+`ContainSubstring` against `err.Error()` instead, which is what the
+existing tests do.
+
+## Mocks — generate with mockery
+
+Mocks live under `<pkg>/mocks/` and are produced by
+[`mockery`](https://github.com/vektra/mockery) v2.53.4. They use
+`testify/mock` under the hood; **inside test code itself, keep
+assertions in gomega style** so the codebase stays consistent.
+
+### Regenerating
+
+Add a `go:generate` directive next to the interface (this is the
+convention we want to migrate to — existing mocks were generated
+without one):
+
+```go
+//go:generate mockery --name Repo --output ./mocks --filename git_mock.go --structname GitRepoMock
+type Repo interface {
+    AddFileToGit(path string) error
+    // ...
+}
+```
+
+Run `go generate ./...` to refresh. **Never hand-edit a generated
+mock file.** Files with `// Code generated by mockery` headers are
+overwritten on regen.
+
+### Using a mock in a test
+
+```go
+import (
+    git_mocks "github.com/simple-container-com/api/pkg/api/git/mocks"
+    "github.com/stretchr/testify/mock"
+    . "github.com/onsi/gomega"
+)
+
+func TestThingWithGit(t *testing.T) {
+    RegisterTestingT(t)
+
+    m := new(git_mocks.GitRepoMock)
+    m.On("AddFileToGit", "path/to/file").Return(nil).Once()
+
+    err := DoThing(m, "path/to/file")
+    Expect(err).ToNot(HaveOccurred())
+    m.AssertExpectations(t)
+}
+```
+
+`mock.AnythingOfType("string")` and `mock.Anything` are acceptable
+when an argument is irrelevant to the test's intent.
+
+## Unit vs integration tests
+
+| Category | File naming | Build tag | Default `go test` runs it? |
+|---|---|---|---|
+| **Unit** | `<name>_test.go`, package `<pkg>` | none | Yes |
+| **Black-box unit** | `<name>_test.go`, package `<pkg>_test` | none | Yes |
+| **Fuzz** | `<name>_fuzz_test.go`, `testing.F` | none | Yes (short-fuzz on CI) |
+| **Integration** | `<name>_integration_test.go`, package `<pkg>_test` | `//go:build integration` | **No** — requires `-tags integration` |
+
+### Migrating existing integration tests
+
+The current `*_integration_test.go` files do **not** declare a build
+tag, so they run on every `go test ./...` invocation. New
+integration tests **must** include the build tag at the top of the
+file:
+
+```go
+//go:build integration
+
+package security_test
+```
+
+Existing integration files will be moved under the build tag in a
+future tidy-up PR; the change is mechanical (add 2 lines at the top)
+and does not affect their semantics.
+
+### What counts as integration
+
+- Anything that requires the host filesystem outside `t.TempDir()`
+- Anything that shells out to `docker`, `git`, `cosign`, `syft`, etc.
+- Anything that hits a network endpoint
+- Anything that takes more than ~5 seconds on a fast laptop
+
+If you can't put a test under build-tagged integration without losing
+real coverage, the test belongs in a unit-level seam — split the code
+under test so the I/O surface is mockable.
+
+## Test invocation
+
+| Goal | Command |
+|---|---|
+| Run the unit-test suite | `go test ./...` |
+| Run unit + integration | `go test -tags integration ./...` |
+| Run a single package | `go test ./pkg/security/...` |
+| Run with race detector | `go test -race ./...` |
+| Run with coverage (text summary) | `go test -coverprofile=/tmp/cover.out ./... && go tool cover -func=/tmp/cover.out` |
+| Run with coverage (HTML report) | `go test -coverprofile=/tmp/cover.out ./... && go tool cover -html=/tmp/cover.out` |
+| Run fuzz target for 30 s | `go test -fuzz=FuzzVerifyAndExtract -fuzztime=30s ./pkg/security/...` |
+
+The canonical CI invocation is `welder run test`, defined in
+[`welder.yaml`](../welder.yaml) under the `test` task.
+
+### Coverage-task aggregate suite
+
+The `welder run coverage` task runs the full suite with a coverage
+profile and prints both the aggregate percentage and the
+package-level breakdown. Failing the aggregate threshold (currently
+**no-regression** vs the previous run; will tighten to a hard floor
+once coverage climbs) blocks the build.
+
+## When tests are required
+
+`docs/CONTRIBUTING.md` is the authoritative policy. The short
+version:
+
+- **Behaviour change**: tests required, must exercise the new path.
+- **Bug fix**: regression test required — the test should fail on
+  `main` and pass on the PR.
+- **Refactor without behaviour change**: tests not required if the
+  existing suite covers the touched code; if it doesn't, that's the
+  test gap to file.
+- **Security-sensitive paths** (`pkg/security/`, `push.yaml`, `sc.sh`,
+  SLSA / cosign / Sigstore chain): tests required + the
+  threat-model note documented in `docs/CONTRIBUTING.md`.
+- **Pure dependency bump**: tests not required (existing suite is
+  the regression net); the `govulncheck` + `Trivy` gates are the
+  validation surface.
+
+PRs without required tests are blocked at maintainer review.
+
+## Naming conventions
+
+| What | Convention | Example |
+|---|---|---|
+| Test function | `Test<Type>_<Method>_<Scenario>` or `Test<Function>` with sub-tests | `TestCache_Get_HitAfterSet` |
+| Sub-test name | human-readable string with spaces | `t.Run("rejects expired entries", ...)` |
+| Table case `name` field | same | `{name: "rejects expired entries", ...}` |
+| Test fixture file | `testdata/<scenario>.<ext>` | `testdata/expired-entry.json` |
+| Mock variable | `m<Type>` or `<type>Mock` | `mGit` or `gitMock` |
+
+## Fixtures
+
+Use the Go-conventional `testdata/` directory (Go's build toolchain
+ignores `testdata/` automatically). Place fixture files next to the
+test that uses them:
+
+```
+pkg/security/
+├── cache.go
+├── cache_test.go
+└── testdata/
+    └── corrupted-entry.json
+```
+
+Load with `os.ReadFile` rooted at the test's package directory:
+
+```go
+b, err := os.ReadFile(filepath.Join("testdata", "corrupted-entry.json"))
+Expect(err).ToNot(HaveOccurred())
+```
+
+For corpus-style fuzz inputs, follow Go's fuzz testdata convention
+(`testdata/fuzz/<TestName>/`).
+
+## Coverage targets
+
+| Target | Threshold | Status |
+|---|---|---|
+| **Per-PR no-regression** | Aggregate must not decrease vs `main` | Enforced by `welder run coverage` (planned CI gate) |
+| **Silver badge** (`test_statement_coverage80`) | ≥ 80 % aggregate | Open — current ~16 % |
+| **Gold badge** (`test_statement_coverage90`) | ≥ 90 % aggregate | Open — follows Silver |
+
+### Files excluded from the coverage measurement
+
+These contribute 0 % by design and would otherwise hide real gains:
+
+- `cmd/*/main.go` — entry points; coverage requires e2e runs and
+  doesn't measure behavioural correctness.
+- `**/mocks/*.go` — auto-generated.
+- `pkg/api/tests/*.go` — reference-application fixtures consumed by
+  integration tests.
+
+The coverage task applies these exclusions before computing the
+aggregate.
+
+## Continuous integration
+
+Tests run on every PR via `welder run test` invoked from the
+`build-staging.yml` workflow. The `Go Fuzz` workflow
+(`.github/workflows/fuzz.yml`) runs the `testing.F` targets for 30
+seconds per target on each PR commit, and 10 minutes per target on
+the Monday cron.
+
+A coverage workflow that posts a delta comment on each PR will be
+added once these standards land.
+
+## Related documents
+
+- [`CONTRIBUTING.md`](CONTRIBUTING.md) — when tests are required
+- [`SECURITY.md`](SECURITY.md) — threat model the security-sensitive
+  test layer is written against
+- [`DEPENDENCIES.md`](DEPENDENCIES.md) — pre-release SCA gate that
+  complements the test suite
+- [`ARCHITECTURE.md`](ARCHITECTURE.md) — actors + trust boundaries
+  the integration tests exercise