Skip to content

test(pass): guard the Payload carriers checkEncodingKeys walks - #365

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
test/pass-payload-carrier-guard
Open

test(pass): guard the Payload carriers checkEncodingKeys walks#365
OmarAlJarrah wants to merge 1 commit into
mainfrom
test/pass-payload-carrier-guard

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

checkEncodingKeys resolves Content.Encoding keys against the model each content is typed by. To reach every content it names the fields that carry an ir.Payload by hand — Operation.Request, Response.Payload, Message.Payload — and encodingCarriers() on the test side repeats the same three. Nothing in a Payload's Go type says who owns one, so both lists are hand-written, and nothing held either: a fourth carrier added to the IR would be walked by neither, its encoding keys would go unresolved, and an emitter rendering multipart parts from them would silently render no part for a key that misses — with the whole suite green.

This adds the drift guard the rest of the verification layer already has (indexCarrierFields/integerFields, walkChecks(), nofloat_test.go).

TestEncodingCarriers_NameEveryPayloadFieldInTheIR walks the IR's static type graph from ir.Document, collecting every struct field whose type is an ir.Payload or a pointer, slice, array or map of one, and diffs that set against encodingCarriers(). The walk visits each distinct reflect.Type once, so recursive shapes terminate, and both it and the field-type walk carry an explicit depth cap. It does not descend into structs when deciding whether a field is a carrier: a field that merely reaches a Payload further down is the spine leading to one — Document.Services reaches every one of today's — and naming the spine would name most of the IR.

The sealed TypeDef sum is reached only through an interface, which a walk over the static type graph cannot descend into, so each concrete kind is walked from its own root as well, seeded from the TypeKind constants the ir sources declare rather than from a list here. ir/nofloat_test.go walks the same two halves for the same reason.

It holds both lists, in two steps. The new test holds encodingCarriers() against the IR; TestValidate_EncodingKeyAddressesNoProperty already holds checkEncodingKeys against encodingCarriers(), by requiring a diagnostic from every entry. So a carrier added to the IR reddens the new test, and adding it to the list reddens the existing one until checkEncodingKeys walks it too. The carriers are renamed from prose ("request") to the ir fields they are ("Operation.Request") so the two sets are comparable; that is also what the subtests are now named.

No production behaviour changes — the only non-test edit is checkEncodingKeys's doc comment, which said the list had to be maintained by hand and now says what guards it.

Test plan

Full gate green: gofmt, go vet ./..., golangci-lint run (0 issues), go build ./..., ./scripts/check-coverage.sh (all 4942 statements covered).

A guard that has never been watched failing is not a guard, so both halves of the walk were driven by planting the defect each exists to catch and watching it go red.

Planting Probe []*Payload on ir.Channel (reachable from ir.Document, and behind a slice of pointers, so it also exercises the "contains" handling):

--- FAIL: TestEncodingCarriers_NameEveryPayloadFieldInTheIR (0.00s)
    validate_carriers_test.go:64:
        	Error:      	Should be empty, but was   []string{
        	            	- 	"Channel.Probe",
        	            	  	"Message.Payload",
        	            	  	"Operation.Request",
        	            	  	"Response.Payload",
        	            	  }
        	Messages:   	encodingCarriers must name every ir field that carries an ir.Payload, once each (-declared +listed); a new one also has to be walked by checkEncodingKeys

Planting Probe map[string]Payload on ir.Model, which is reachable only through the sealed sum (and behind a map value):

--- FAIL: TestEncodingCarriers_NameEveryPayloadFieldInTheIR (0.00s)
    validate_carriers_test.go:64:
        	Error:      	Should be empty, but was   []string{
        	            	  	"Message.Payload",
        	            	- 	"Model.Probe",
        	            	  	"Operation.Request",
        	            	  	"Response.Payload",
        	            	  }

That second half is load-bearing rather than decorative: with the Model.Probe plant still in place, dropping the concrete-kind seeding from the walk turns the test green again, so the planted carrier escapes entirely.

The chain to the production list was checked the same way. With the guard as committed, deleting the Message.Payload loop from checkEncodingKeys reddens the existing behavioural test:

--- FAIL: TestValidate_EncodingKeyAddressesNoProperty/Message.Payload (0.00s)
        	Error:      	"[]" should have 1 item(s), but has 0
        	Messages:   	exactly the planted key must address nothing

All three plants were removed before committing; the diff touches only pass/.

Closes #303

checkEncodingKeys resolves Content.Encoding keys against the model each
content is typed by, and reaches every content by naming the fields that
carry an ir.Payload — Operation.Request, Response.Payload,
Message.Payload. The test side repeats the same three in
encodingCarriers(). Nothing in a Payload's Go type says who owns one, so
both lists are hand-written, and nothing held either: a fourth carrier
added to the IR would be walked by neither, its encoding keys resolved
against nothing, with the whole suite green.

Add a reflection-driven drift guard. It walks the IR's static type graph
from ir.Document — visiting each distinct reflect.Type once, so recursive
shapes terminate — collecting every struct field whose type is an
ir.Payload or a pointer, slice, array or map of one, and diffs that
against encodingCarriers(). The sealed TypeDef sum is reached only
through an interface, which a walk over the static type graph cannot
descend into, so each concrete kind is walked from its own root as well,
seeded from the kinds the ir sources declare rather than from a list.

The guard holds both lists, in two steps: it holds encodingCarriers
against the IR, and TestValidate_EncodingKeyAddressesNoProperty already
holds checkEncodingKeys against encodingCarriers by requiring a
diagnostic from every entry. So a carrier added to the IR reddens the new
test, and adding it to the list reddens the existing one until
checkEncodingKeys walks it too. The carriers are renamed to the ir fields
they are so the two sets are comparable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pass: the Payload carriers checkEncodingKeys walks are hand-listed with no drift guard

1 participant