fix(conformance): close four false-green gaps (#70/#71/#72/#73)#91
Merged
Conversation
…postgres probe Three contract clauses the suite claimed to verify were never exercised; each let a genuinely-broken plugin pass conformance. #73 — Fault_DatadirPermission never ran (all five plugins set SkipDatadirPermission=true). A naive flag-flip is a false-red: the suite forces backend=docker, whose Up only bind-mounts the datadir and lets the root daemon create the mount source, so a host-side 0o000 never surfaces as an Up error. The fault now forces the host-process backend (Config.DatadirFaultBackend, default "nix"), whose Up mkdirs the datadir synchronously before exec'ing the engine — a 0o000 parent is then a deterministic, cross-platform Up failure, and because the mkdir precedes the backend exec it fires even without that binary installed. The datadir is nested two levels (faultDir/data/db) so both prep shapes fail: mysql/redis/es mkdir the datadir itself, postgres mkdirs only its parent. isBackendBinaryMissing turns the rare fall-through (mkdir succeeds, backend binary absent) into a Skip, never a spurious pass. #70 — up-or-reuse was never exercised. The idempotent loop Downs between iterations, so its second Up is a restart, not a reuse. A dedicated UpReuse phase now calls Up again while the service is up, requires nil, and re-checks readiness so a reuse that tears the engine down is caught. #72 — postgres had no NativeProbe, leaving contract clause 5 unenforced for one engine. Added conformance.PostgresProbe, a stdlib-only SSLRequest handshake requiring the server's 'S'/'N' reply (postgres sends no greeting, so an SSLRequest is the minimal pre-auth round-trip). Also corrected three now-false docs the above touched: the "kind->probe lookup" NativeProbe fallback (never existed), the IdempotentCount "catches Up-on-existing-state" claim, and CONTRACT.md clauses 3 and 11. Verified on real containers: mysql, postgres, redis and elasticsearch conformance all pass, including UpReuse and Fault_DatadirPermission (each surfaces a real "mkdir datadir: permission denied"); postgres's EnvVars phase round-trips the new SSLRequest probe against postgres:17.
extractDialableAddrs silently dropped any *_URL whose value had no explicit port, leaving AssertReachable's v0.2.6 guard blind to it — a URL that resolves to a scheme default while the engine listens on a bough-allocated port is exactly the unreachable-advertise class the guard exists to catch. A port-less URL now falls back to its scheme's default port (schemeDefaultPort); an unknown scheme with no port is left unverified rather than dialed at a guess.
ikeikeikeike
added a commit
that referenced
this pull request
Jul 6, 2026
The local main ref this branch forked from was stale — PR #91/#92/#93 were merged via `gh pr merge` (GitHub API) directly against the remote, per this session's standing rule to never touch the shared checkout's local main HEAD while a parallel session might be using it, but the local main ref was never fetched afterward. This branch was created from that stale local ref, so it never had PR #91's newly added "UpReuse" conformance phase (Up called a second time while the service is already running, must be a no-op) locally to test against.
ikeikeikeike
added a commit
that referenced
this pull request
Jul 6, 2026
Merging origin/main pulled in PR #91's new conformance UpReuse phase (Up called a second time while the service is already running, must be a no-op) -- this branch predates that merge locally, since #91/#92/ #93 were merged via `gh pr merge` directly against the remote without ever fetching the result back into this session's shared checkout, so this branch forked from a stale local main ref. That phase caught a real bug: Up() checked dockerutil.IsPortFree unconditionally, so a second Up() against an already-running container -- which legitimately holds the port -- was rejected as a "foreign" conflict instead of being reused. dockerutil.UpOrReuse now runs first (same order the mysql/postgres/redis/elasticsearch docker.go plugins already use): if a container named bough-compose-<port> is already running, Up reuses it and skips the port-conflict check entirely; IsPortFree is only consulted on a genuine fresh start. Sidecar state is now written unconditionally (reuse or fresh) since Down/EnvVars/ReadyCheck must work correctly even from a freshly-spawned plugin process that did not itself create the container. Not reproducible on this macOS dev machine: Docker Desktop's proxy layer does not visibly bind host ports the way native Linux does, so IsPortFree(port) returned true even with the container's own port already published, masking the bug locally. Confirmed fixed via the local conformance suite (UpReuse phase passing) after merging in the newer conformance package; the original failure was only caught by the ubuntu-24.04 CI runner.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #70, #71, #72, #73.
Four false-green gaps in the conformance suite — clauses it claimed to verify but never exercised:
Fault_DatadirPermissionwas skipped by every plugin. A naive flag-flip is a false-red (the suite forcesbackend=docker, which only bind-mounts the datadir). It now forces the host-process backend, whoseUpmkdirs the datadir synchronously; the datadir is nested two levels so postgres's parent-only mkdir fails too, and it skips (not passes) if the backend binary is absent.Downs between iterations, so its secondUpis a restart. A dedicatedUpReusephase callsUpagain while the service is up, then re-checks readiness.PostgresProbe(SSLRequest handshake), so contract clause 5 is enforced for every engine.*_URLvalues are now dialed via their scheme default instead of being silently skipped.Also corrects the now-false
NativeProbe/IdempotentCountdocs and CONTRACT.md clauses 3 & 11.Verified: all four engines' docker conformance pass (
UpReuse+Fault_DatadirPermissionsurface a realmkdir datadir: permission denied; postgres round-trips the new probe againstpostgres:17); fast-lane unit + mock self-tests green;golangci-lint0 issues.