protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.6k stars 15.48k forks source link

Incorrect conformance tests (RepeatedScalarSelectsLast.{TYPE_UINT64, FIXED64, FIXED32}) #3678

Closed ahamez closed 7 years ago

ahamez commented 7 years ago

Hello,

Using the conformance tool provide with protobuf to test my Elixir implementation, I found it to fail the RepeatedScalarSelectsLast.* tests:

ERROR, test=Required.Proto2.ProtobufInput.RepeatedScalarSelectsLast.UINT64.ProtobufOutput: Output was not equivalent to reference message: deleted: optional_uint64: 0
. request=protobuf_payload: " \271` \377\377\377\377\377\377\377\377\377\001 \000" requested_output_format: PROTOBUF message_type: "protobuf_test_messages.proto2.TestAllTypesProto2", response=protobuf_payload: ""
ERROR, test=Required.Proto2.ProtobufInput.RepeatedScalarSelectsLast.FIXED64.ProtobufOutput: Output was not equivalent to reference message: deleted: optional_fixed64: 0
. request=protobuf_payload: "A90\000\000\000\000\000\000A\377\377\377\377\377\377\377\377A\000\000\000\000\000\000\000\000" requested_output_format: PROTOBUF message_type: "protobuf_test_messages.proto2.TestAllTypesProto2", response=protobuf_payload: ""
ERROR, test=Required.Proto2.ProtobufInput.RepeatedScalarSelectsLast.FIXED32.ProtobufOutput: Output was not equivalent to reference message: deleted: optional_fixed32: 0
. request=protobuf_payload: "=90\000\000=\377\377\377\377=\000\000\000\000" requested_output_format: PROTOBUF message_type: "protobuf_test_messages.proto2.TestAllTypesProto2", response=protobuf_payload: ""

I checked that in all these cases, my library correctly selects the last value, which is 0. As this the default value, an empty message is generated when encoding back the answer payload (isn't it the right behavior, for protobuf2 and protobuf3?).

This only happens with the protobuf2 test message, so I suspect that this failure error is reported because the hasXXX() method will return false, which is then interpreted as a deletion by MessageDifferencer.

Finally, if, for instance, I invert lines 886 and 887 of conformance_test.cc, tests pass because the last value is on the wire (as it's not the default value).

ahamez commented 7 years ago

My bad: in protobuf2, default values are on the wire.