test(pass): guard the Payload carriers checkEncodingKeys walks - #365
Open
OmarAlJarrah wants to merge 1 commit into
Open
test(pass): guard the Payload carriers checkEncodingKeys walks#365OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
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.
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.
Summary
checkEncodingKeysresolvesContent.Encodingkeys against the model each content is typed by. To reach every content it names the fields that carry anir.Payloadby hand —Operation.Request,Response.Payload,Message.Payload— andencodingCarriers()on the test side repeats the same three. Nothing in aPayload'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_NameEveryPayloadFieldInTheIRwalks the IR's static type graph fromir.Document, collecting every struct field whose type is anir.Payloador a pointer, slice, array or map of one, and diffs that set againstencodingCarriers(). The walk visits each distinctreflect.Typeonce, 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.Servicesreaches every one of today's — and naming the spine would name most of the IR.The sealed
TypeDefsum 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 theTypeKindconstants the ir sources declare rather than from a list here.ir/nofloat_test.gowalks the same two halves for the same reason.It holds both lists, in two steps. The new test holds
encodingCarriers()against the IR;TestValidate_EncodingKeyAddressesNoPropertyalready holdscheckEncodingKeysagainstencodingCarriers(), 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 untilcheckEncodingKeyswalks 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 []*Payloadonir.Channel(reachable fromir.Document, and behind a slice of pointers, so it also exercises the "contains" handling):Planting
Probe map[string]Payloadonir.Model, which is reachable only through the sealed sum (and behind a map value):That second half is load-bearing rather than decorative: with the
Model.Probeplant 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.Payloadloop fromcheckEncodingKeysreddens the existing behavioural test:All three plants were removed before committing; the diff touches only
pass/.Closes #303