Run the serialisation test suite - #29
Merged
Merged
Conversation
test/tests/serialisation-tests/ has never been exercised. The Makefile's
test/tests/*.json glob doesn't descend into it, and both harnesses assumed
every case has a "raw" element, so pointing them at those files died with
KeyError: 'raw'.
Gate each phase on what the suite supplies, as its README defines it --
parse iff the case has "raw", serialise iff it has "expected" -- rather
than on must_fail, which means "parsing must fail" on a parse case and
"serialising must fail" on a serialisation case. No parse case lacks "raw"
and none carries both must_fail and "expected", so the gating is equivalent
for everything that ran before.
Also corrects an eager default: test.get("canonical", test["raw"]) evaluated
test["raw"] even when canonical was present, which stayed harmless only
while every case had raw.
544 cases added to each harness. 2861 of 2861 pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
test/tests/serialisation-tests/has never been run. Two things kept it out:the Makefile's
test/tests/*.jsonglob doesn't descend into the subdirectory,and both harnesses assumed every case has a
rawelement, so aiming them atthose files died with
KeyError: 'raw'.What changed
Makefile — added
SER_TESTS, passed it totest.pyandtest_compat_full.py, and added it to the submodule-init rule so a freshcheckout still bootstraps.
Both harnesses — the phase gating was keyed on
must_fail, which conflatestwo meanings: on a parse case it means parsing must fail, on a serialisation
case it means serialising must fail. It's now keyed on what the suite
supplies, as the suite README defines it — parse iff the case has
raw,serialise iff it has
expected— andtest_serialiseunderstands thatmust_failwithoutrawmeansser()is required to raise. I checked theshapes before relying on that: no parse case is missing
raw, and none carriesboth
must_failandexpected, so the gating is equivalent for everythingthat ran before.
One latent bug turned up on the way.
test.get("canonical", test["raw"])evaluates its default eagerly, so it blew up on the five non-
must_failserialisation cases even though
canonicalwas present. Now an explicitconditional.
Verification
make testexits 0 with 2861 of 2861 in each harness — 544 cases added apiece(378 key-generated, 124 token-generated, 33 string-generated, 9 number).
Tests that can't fail aren't tests, so I checked both directions by breaking
the library on purpose and confirming the new cases catch it:
serialisation-tests/number.jsonto6 of 9, flagging
0.0025→0.003and9.9995→9.999ser_keydropskey-generated.jsonto 202 of 378Both reverted;
http_sf/is untouched by this PR.make lintandmake typecheckexit 0.Note that five assertions in the parse suites fail but count as passes, because
the suite marks them
can_fail— base64bad paddingand the syntactic maxand min dates at ±999,999,999,999,999. That's pre-existing and unrelated;
nothing in
serialisation-tests/iscan_fail.Context
This came out of reading httpwg/structured-field-tests#114, which adds decimal
rounding cases that distinguish decimal from binary rounding. If that PR lands,
serialisation-tests/number.jsongoes from 9 cases to 13 and http-sf passesall of them, since the harness reads the JSON with
parse_float=Decimal. Itwould have gone on passing silently either way until now.
Separately, and not addressed here:
ser_decimalconverts afloatargumentwith
Decimal(input_decimal), which captures the exact binary value. Thatfails all four of the proposed cases and two of the five existing ones. The
test suite doesn't exercise it because the harness feeds
Decimal, notfloat.Written by Claude Code (Opus 5) in an interactive session, at my direction and
with me reading each step's output. It started as a question about
httpwg/structured-field-tests#114 and turned up the fact that these tests were
never being run.
🤖 Generated with Claude Code