Skip to content

Add tests locking down ChunkID hex formatting - #380

Merged
folbricht merged 1 commit into
masterfrom
lock-chunkid-formatting
Aug 4, 2026
Merged

Add tests locking down ChunkID hex formatting#380
folbricht merged 1 commit into
masterfrom
lock-chunkid-formatting

Conversation

@folbricht

Copy link
Copy Markdown
Owner

ChunkID.String() and MarshalJSON() were on pointer receivers between #273 and #379, which meant a ChunkID value passed to %s/%v/%q was rendered by fmt's byte-array fallback, and a non-addressable ChunkID was marshalled as an array of numbers. #379 moved both back to value receivers but didn't add a test, so the same regression could slip back in unnoticed.

This adds types_test.go covering:

  • formatting a ChunkID value (and pointer) with %s, %v and %q, plus ChunkMissing.Error() which formats the ID by value
  • json.Marshal of a ChunkID in a map value — map values aren't addressable, so a pointer-receiver MarshalJSON is skipped there and the ID comes out as an array of numbers
  • the []ChunkAdditionalInfo round trip used by inspect-chunks
  • compile-time assertions that ChunkID (not just *ChunkID) implements fmt.Stringer and json.Marshaler, so a receiver change fails the build rather than silently changing output

Verified the tests catch the regression: reverting both methods to pointer receivers fails TestChunkIDFormatting (raw bytes instead of hex) and TestChunkIDMarshalJSON (number array instead of a hex string).

ChunkID.String() and MarshalJSON() were briefly moved to pointer
receivers in #273, which meant a ChunkID value was rendered by fmt's
byte-array fallback and marshalled as an array of numbers. #379 moved
them back to value receivers but added no test, so the same regression
could slip in again unnoticed.

Cover both the formatting and the JSON encoding of a ChunkID value,
including the non-addressable map-value case that skips a
pointer-receiver MarshalJSON, plus compile-time assertions that the
value type implements fmt.Stringer and json.Marshaler.
@folbricht
folbricht merged commit 86db41d into master Aug 4, 2026
3 checks passed
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.

1 participant