♻️ refactor(forge): extract the conformance suite into an importable package with an explicit observation surface (E10-S01) - #98
Merged
Conversation
…package with an explicit observation surface (E10-S01)
…the coverage-floor headroom (D-165)
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
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.
Implements E10-S01 (
[autonomous], depends on S00 which merged ase2e7691). Decision row: D-165.The problem
internal/forge/conformancewas 1,155 lines across four_test.gofiles. Go cannotimport
_test.go, so the only way to conformance-test a second adapter was to COPYthe cases — which guarantees drift, and is why D-084's
github-deferredrows wereunflippable.
What landed
Case bodies now live in ordinary Go. An adapter supplies a
Factoryand callsRunSuite; nothing is copied.suite.go—Factory,Case,Cases(),RunSuiteobserve.go— theObserver/Fixturesurfaces (REQ-E10-S01-04)portcount.go— port-level write countingreplay.go,fixtures.go,cases_replay.go,ids.go— moved case + fixture bodiescatalog.go— strict-decoding catalog loader (REQ-E10-S01-03)tb.go— theTBseam that makes "can this case fail?" answerableThe five exported test names are unchanged. The DoD forbids renaming a case, and
exitgate_test.go'sl1CatalogTestspins these exact names — renaming them wouldhave silently unhooked the E7 exit gate.
What the extraction found
The two SHA-guard cases had never run against GitLab, despite their catalog rows
saying
forge: gitlab. The httptest harness served discussions and notes only. It nowserves MR/branch/approve/merge, so they do.
A conditional assertion that could not fail. The "summary updated in place" check
was guarded behind
if ff, ok := f.(*fake.Forge); ok— false on GitLab, so on thatbackend it proved nothing while still reporting PASS. It is now unconditional.
Merge ATTEMPTS were never distinguished from merges PERFORMED. Both SHA-guard cases
asserted only
len(f.Merges) == 0, which is equally true of a backend that skipped thepre-check and let the atomic CAS refuse. Split, the two cases now disagree — 0 attempts
when the pre-check fails closed before any mutation, 1 when the CAS is reached and
refuses — which is the guarantee each was supposed to prove.
Counting is done by a port-level decorator, not per adapter: GitLab's
MergeCASre-reads heads and refuses client-side, so a moved head produces ZERO merge HTTP
requests. A harness counting PUTs would report 0 attempts where the port was called once.
Three gates keep it honest
A conformance suite that has stopped proving its property still reports PASS, so:
TestCatalogMatchesExecutedCases— the catalog is compared against OBSERVEDEXECUTION (
RunSuitereturns what it dispatched), not against a name list. A gatekeyed on test-function names stays green for a case unhooked from the runner — a
predicate over TEXT standing in for a STRUCTURAL property, the species behind D-164.
TestEveryCaseCanFail— every case must go red against a sabotaged backend, onevery adapter, with a positive control that it still passes against a conforming one.
TestEveryObservationIsLoadBearing— corrupts each value a case reads, ONE AT ATIME, and requires each to flip the verdict.
Gate 3 found two real weaknesses in code that had already passed my own review:
a
NotesUpdated() > 0check no corruption could flip (now an exact count), and ablind spot in my own mutation operator (a uniform offset is invisible to a before/after
delta — corruption is now applied to the first read only).
Scope held deliberately
Factoryreturnsforge.Forge, notforge.RunPort.RunPortis E10-S02'sdeliverable and S02 is LGTM-gated; declaring it in this autonomous lane would
smuggle an LGTM-gated core-contract change past
backlog.md:557. S02 depends on S01precisely so the port change lands against an executable suite.
Cases take pins from
Fixture.Pins()rather than literals, because GitLab synthesisesits merge-result digest — a literal digest is why those cases were fake-only.
Collapsing that digest is E10-S03.
Verification
task checkEXIT=0 (all 21 stages), coverage 91.1% (floor 91%)catalogued cases now run on both adapters
reds; merge attempts collapsed onto performed → REQ-04 proof reds on both backends;
summary re-created instead of updated → rerun-idempotence reds