Decode a real descriptor set in the tests, on engine 3.4.19 - #12
Merged
Merged
Conversation
The decode tests write their payloads by hand, which keeps them legible and keeps them small. Nothing covered a payload with the depth, repetition and nesting a real one has. descriptor.proto's own FileDescriptorSet is the obvious candidate: it is what protoc -o and protogen emit, so it is a likely thing to drop into the decode view, and it exercises groups, enums, extensions ranges and deep nesting in realistic proportions. Both halves come from protobuf-net.Reflection's embedded copy, so there is no binary fixture and no snapshot to drift - the test tracks whatever descriptor.proto the referenced package carries. The schema is two lines. SchemaIndex walks every file in the set, imports included, so importing descriptor.proto names all 34 of its messages without restating any of them. protobuf-net is a new test-only reference: Reflection brings just protobuf-net.Core, which has no concrete TypeModel to serialize with.
3.4.19 exposes FileDescriptorSet.Serializer, so the test can build its payload with the model protobuf-net.Reflection already carries rather than pulling in protobuf-net purely to reach RuntimeTypeModel.Default. The extra package reference goes with it. This moves the engine itself, not just the test - every Generate on the site runs five releases further forward. Nothing in this repo covers codegen, so I checked it by hand rather than by assertion: the C# and VB output for proto2 (groups, extensions, packed, defaults), proto3 (maps, oneofs, optional, well-known imports, streaming services) and edition 2023 (explicit presence, expanded repeated) is byte-identical between 3.3.21 and 3.4.19. Trimmed publish is still warning-clean, so the IL2104 audit in docs/trimming.md holds.
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.
The decode tests write their payloads by hand, which keeps them legible and keeps them small. Nothing covered a payload with the depth, repetition and nesting a real one has.
descriptor.proto's own
FileDescriptorSetis the obvious candidate. It is whatprotoc -oandprotogenemit, so it is a likely thing for someone to drop into the decode view with a.binthey cannot identify, and it exercises groups, enums, extension ranges and deep nesting in realistic proportions — 13 KB, 1823 nodes.Both halves come from protobuf-net.Reflection's embedded copy, so there is no binary fixture and no snapshot to drift: the test tracks whatever descriptor.proto the referenced package carries.
The schema is two lines.
SchemaIndexwalks every file in the set, imports included, so importing descriptor.proto names all 34 of its messages without restating any of them.Three tests:
FileDescriptorSetwith no ties, when nothing says what the payload isfilereads asrepeated FileDescriptorProto, and inside it,FileDescriptorSet.field[0]reads back asLABEL_REPEATED,TYPE_MESSAGE,.google.protobuf.FileDescriptorProtoOne assertion is a
DoesNotContain(..., e => e.IsError)rather than a null check: the stub schema imports something it never uses, and protobuf-net says so. The warning is correct, so the test allows warnings and not errors.Engine 3.3.21 → 3.4.19
The payload needs a
TypeModel. protobuf-net.Reflection brings only protobuf-net.Core, which has no concrete one, so the first pass took a test-onlyprotobuf-netreference to reachRuntimeTypeModel.Default. 3.4.19 exposesFileDescriptorSet.Serializerinstead, so that reference is gone and the payload is built with the model the package already carries.That moves the engine itself, not just the test — every Generate on the site runs five releases further forward. Nothing in this repo covers codegen, so I checked it by hand rather than by assertion: C# and VB output is byte-identical between 3.3.21 and 3.4.19 across proto2 (groups, extensions, packed, defaults), proto3 (maps, oneofs,
optional, well-known imports, streaming services) and edition 2023 (explicit presence, expanded repeated). The trimmed publish is still warning-clean, so the IL2104 audit indocs/trimming.mdholds.36 tests pass.