Adds a payload-schema drift test for the doc - #81
Merged
Conversation
wire_format_spec_test.exs guards only the type index. Nothing tied docs/wire-format.md's per-table field and presence schemas to what StatifierUI.Trace.Manifest actually builds, so the sui-o5c verify walk had to find the document's data-table defects by hand. The new test parses the six tables of the `session.start` section out of the markdown - the payload table, states, transitions, contents, data, and the location object - and checks each against three built messages: a chart written to reach every conditional field, a bare one written to reach none, and the first projected. A field the producer emits with no doc row fails, a doc row nothing emits fails, and the presence column is enforced both ways: an `always` row must be on every sample row, a conditional row must be observed present and absent. A guard test pins the parsed table names so a restructured section fails loudly rather than skipping every check. It found one disagreement immediately. `data.value_location` was documented as present only when the compiler recorded a value span, but Statifier.Compiler returns a location from every arm of build_data_value/2, and the document's own prose below the table already described the fallback. The table cell was stale; it now says always. Refs: sui-o4e
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.
Why
docs/wire-format.mdis the format's normative home: when it and animplementation disagree, the document is what conformance means. But only its
type index was mechanically guarded (
wire_format_spec_test.exs). Nothingtied its per-table field, type, and presence schemas to what
StatifierUI.Trace.Manifestactually builds, so drift in those tables couldonly be found by reading them. The
sui-o5cverify walk found two such prosedefects by hand, which is what filed this bead.
What
A sibling test,
test/statifier_ui/trace/wire_format_payload_schema_test.exs,parses the six markdown tables of the
session.startsection - the top-levelpayload table,
states,transitions,contents,data, and the locationobject - and checks each against three built messages: a chart written to
reach every conditional field, a bare one written to reach none, and the
first of those projected (so the
projectionheader has somewhere to beobserved present).
It parses the document rather than restating it, the way the type-index test
already does, so the document stays the single place a field is written down.
Three properties are checked per table:
alwaysmustbe present on every sample row, and a row marked
present only when ...oromitted ...must be observed both present and absent, so a conditionalrow that has quietly become unconditional fails rather than passing
vacuously.
A fourth guard test pins the set of table names the parser resolved. Without
it, a restructured section that parsed to nothing would make every other test
here iterate an empty schema and pass.
The one disagreement it found
data.value_locationwas documented as present only when the compilerrecorded a span for the element's value. That is stale: every arm of
Statifier.Compiler'sbuild_data_value/2returns aLocation.t()- thewritten value's span for an
expr- orsrc-written element, the<data>element's own span otherwise - and the document's own prose immediately below
the table already described exactly that fallback. The table cell contradicted
its own section.
Resolved on the document side, which owns this contract: the presence cell now
reads
always, with the fallback named. No producer was changed - the codewas right and the table was stale.
Notes
mix qualitygreen and attested on the rebased HEAD (909 tests, 92.9%coverage). The two
○lines are the repo's permanent Gettext and Sobelowskips.
and reverted: an extra producer field with no doc row, a phantom doc row, an
alwayscell flipped to conditional (ondata.d_indexand onstates.children), a conditional cell flipped toalways(
transitions.cond_location), and a renamed table label that makes theparser find nothing. All six failed the suite.
session.startpayload tables only. The ninetrace.*and teneffect.*schemas are the same shape of gap and would bea follow-up bead, not a silent extension of this one.
published wire-format contract promises a consumer, even though no emitted
bytes moved.
Closes sui-o4e