Refuse an absurd payload for every structure, not just the newest - #135
Merged
Conversation
TestPersistenceHostilePayloads exercised the field-level guards of 18 of the 33 structures. The drift ran backwards from the sweeps fixed earlier: the structures added most recently had the most guards tested, and fifteen of the oldest -- HyperLogLog, DDSketch, ThetaSketch, CountMinSketch, QuotientFilter, BinaryFuseFilter, BloomierFilter and the rest -- had none at all. Their readers already had the guards. Nothing exercised them, so a guard that had stopped firing would have been found by a user with an edited file rather than here. Eighteen tests now reach them, each asserting the message its own guard produces, which is what keeps a payload refused for the right reason. TestEveryStructureRefusesSomeAbsurdPayload is the completeness check. It cannot be one loop the way the persistence sweeps are, because what counts as absurd differs per structure, so it is a map from each structure to the test carrying it -- with both halves checked. The roster is StructureId, and each named test has to exist and be a test, so an entry pointing at something renamed or un-attributed fails rather than silently vouching for nothing. Also removes an orphaned doc comment that had drifted off PokeUInt64 and stacked a second <summary> onto PokeByte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JH2NRDbhF5bwAsTAP7znb9
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JH2NRDbhF5bwAsTAP7znb9
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.
TestPersistenceHostilePayloadsexercised the field-level guards of 18 of the 33 structures. The drift ran the opposite way from the sweeps fixed in #132 and #133: the structures added most recently were the best covered, and fifteen of the oldest had no field-level test at all —HyperLogLog,DDSketch,ThetaSketch,CountMinSketch,QuotientFilter,BinaryFuseFilter,BloomierFilter,CountSketch,SimHashSignature,InvertibleBloomLookupTable,HyperLogLogPlus, and the four older Bloom variants.Their readers already had the guards. Nothing exercised them, so a guard that had quietly stopped firing would have been found by a user with an edited file rather than here.
What this covers
Eighteen new tests, each poking one field, repairing the CRC, and asserting the message its own guard produces — the message is the landing assertion, which is what keeps a payload from being refused for the wrong reason. That mattered: three of my fragments (
"between","hash function","at least one") were loose enough to match an unrelated refusal, and I tightened them and re-ran to confirm the tests still landed where intended.The offsets come from FORMAT.md, which #133 made complete and testable.
A map, not a loop
TestEveryStructureRefusesSomeAbsurdPayloadis the completeness check. It can't be one loop the way the persistence sweeps are, because what counts as absurd differs per structure. So it's a map from each structure to the test carrying it, with both halves checked: the roster comes fromStructureId, and each named test must exist and carry[TestMethod].I first wrote that map with a
DeclaredElsewhereexemption list claiming fifteen structures were covered in their own test files. That was false — they were covered in this same file all along, and my exemptions would have quietly excused them from the roster. The map is now all 33 entries with no exemptions, generated by parsing the file rather than transcribed.Verification
HyperLogLog's reader guard → its test failsBinaryFuseFilter's fingerprint-width guard → its test failsThetaSketch's trailing-value tolerance to every value → its test failsStructureId→ the completeness check fails by name[TestMethod]from a test the map vouches for → fails naming the testMy first attempt at the
HyperLogLogprobe reported SURVIVED, which I chased before trusting it: the surgery had disabled the constructor's power-of-two check rather than the reader's, so the test was never going to notice. Retargeted at theInvalidDataExceptionsite, it kills. A probe that lands in the wrong place is indistinguishable from a test that doesn't work, until you look.Also removes an orphaned doc comment that had drifted off
PokeUInt64and stacked a second<summary>ontoPokeByte.Clean
dotnet build -c Release -warnaserror, exit 0 unpiped. 925 tests passing, up 19.