What
Four test helpers silently t.Skip() when a runtime capability they depend on isn't available, instead of failing:
requireBwrap (internal/sandboxrun/integration_linux_test.go:21) — skips if bwrap is missing or can't create a user namespace.
requireUnixSocket (internal/facade/facade_test.go:22) — skips if AF_UNIX listen/dial isn't permitted.
requireLoopback (internal/facade/route_mutation_test.go:18) — skips if loopback TCP listen/dial isn't permitted.
requireLoopbackDial (internal/cli/controlinfo_test.go:15) — same, for the control-plane tests.
There is no CI-side check that a minimum number of tests actually ran (vs. skipped), so a broken/incomplete environment produces a green CI run with zero real coverage from the gated tests.
Why
This already happened for real: internal/sandboxrun's 15 requireBwrap-gated integration tests — the ones that launch a genuine bwrap sandbox rather than just asserting on the Grants struct — silently skipped in ci.yml's ubuntu-latest job for an unknown period, because bubblewrap was never installed there (only e2e.yml had the install step). This was caught and fixed in #87 by installing bubblewrap + the AppArmor userns allow-rule in ci.yml too, confirmed empirically by comparing package-level test timing with bwrap present vs. removed from PATH.
That fix adds provisioning, not a tripwire. Nothing stops the same failure mode from recurring: a future GitHub-hosted runner image tightening AppArmor defaults again, an apt package/binary rename, or a self-hosted/containerized runner added later without the same provisioning step would all silently zero out coverage again, caught only if someone happens to notice — as happened this time.
The remaining three helpers (requireUnixSocket, requireLoopback, requireLoopbackDial) gate the facade and control-plane transport tests and have the identical silent-skip shape; they just haven't bitten us yet because GitHub-hosted runners are unsandboxed VMs where loopback TCP and AF_UNIX always work. That stops being a safe assumption the moment tests run inside a container or a more restrictive sandbox.
How
Differentiate CI vs. local execution instead of skipping unconditionally. There's already a precedent for this in the codebase (internal/e2e/classify.go:188,231, gated on os.Getenv("GITHUB_ACTIONS") == "true"):
- Locally (no
GITHUB_ACTIONS env var): keep t.Skip() — a developer without bwrap installed, or working in a network-restricted sandbox, shouldn't be blocked.
- In CI (
GITHUB_ACTIONS=true): t.Fatalf instead — a missing capability there is an infrastructure regression, not something to quietly tolerate.
Apply to all four helpers:
requireBwrap — internal/sandboxrun/integration_linux_test.go
requireUnixSocket — internal/facade/facade_test.go
requireLoopback — internal/facade/route_mutation_test.go
requireLoopbackDial — internal/cli/controlinfo_test.go
Worth doing before or alongside #88 (Unix-socket-only facade/control-plane transport): if TCP tests get deleted as part of that change, the replacement Unix-only tests shouldn't inherit the same blind spot.
Verification
- Locally: temporarily rename
bwrap off PATH (or otherwise break one precondition) and confirm the affected package still passes via skip — no behavior change for local dev.
- In CI: same setup with
GITHUB_ACTIONS=true forced, confirm the affected test now fails loudly instead of silently passing.
- Optionally, a lightweight guard test that asserts these four packages report a nonzero executed-test count in CI, independent of the individual
require* helpers, as a second line of defense.
What
Four test helpers silently
t.Skip()when a runtime capability they depend on isn't available, instead of failing:requireBwrap(internal/sandboxrun/integration_linux_test.go:21) — skips ifbwrapis missing or can't create a user namespace.requireUnixSocket(internal/facade/facade_test.go:22) — skips if AF_UNIX listen/dial isn't permitted.requireLoopback(internal/facade/route_mutation_test.go:18) — skips if loopback TCP listen/dial isn't permitted.requireLoopbackDial(internal/cli/controlinfo_test.go:15) — same, for the control-plane tests.There is no CI-side check that a minimum number of tests actually ran (vs. skipped), so a broken/incomplete environment produces a green CI run with zero real coverage from the gated tests.
Why
This already happened for real:
internal/sandboxrun's 15requireBwrap-gated integration tests — the ones that launch a genuine bwrap sandbox rather than just asserting on theGrantsstruct — silently skipped inci.yml'subuntu-latestjob for an unknown period, because bubblewrap was never installed there (onlye2e.ymlhad the install step). This was caught and fixed in #87 by installing bubblewrap + the AppArmor userns allow-rule inci.ymltoo, confirmed empirically by comparing package-level test timing with bwrap present vs. removed fromPATH.That fix adds provisioning, not a tripwire. Nothing stops the same failure mode from recurring: a future GitHub-hosted runner image tightening AppArmor defaults again, an apt package/binary rename, or a self-hosted/containerized runner added later without the same provisioning step would all silently zero out coverage again, caught only if someone happens to notice — as happened this time.
The remaining three helpers (
requireUnixSocket,requireLoopback,requireLoopbackDial) gate the facade and control-plane transport tests and have the identical silent-skip shape; they just haven't bitten us yet because GitHub-hosted runners are unsandboxed VMs where loopback TCP and AF_UNIX always work. That stops being a safe assumption the moment tests run inside a container or a more restrictive sandbox.How
Differentiate CI vs. local execution instead of skipping unconditionally. There's already a precedent for this in the codebase (
internal/e2e/classify.go:188,231, gated onos.Getenv("GITHUB_ACTIONS") == "true"):GITHUB_ACTIONSenv var): keept.Skip()— a developer without bwrap installed, or working in a network-restricted sandbox, shouldn't be blocked.GITHUB_ACTIONS=true):t.Fatalfinstead — a missing capability there is an infrastructure regression, not something to quietly tolerate.Apply to all four helpers:
requireBwrap—internal/sandboxrun/integration_linux_test.gorequireUnixSocket—internal/facade/facade_test.gorequireLoopback—internal/facade/route_mutation_test.gorequireLoopbackDial—internal/cli/controlinfo_test.goWorth doing before or alongside #88 (Unix-socket-only facade/control-plane transport): if TCP tests get deleted as part of that change, the replacement Unix-only tests shouldn't inherit the same blind spot.
Verification
bwrapoffPATH(or otherwise break one precondition) and confirm the affected package still passes via skip — no behavior change for local dev.GITHUB_ACTIONS=trueforced, confirm the affected test now fails loudly instead of silently passing.require*helpers, as a second line of defense.