Printing a protobuf message changes its equality

[veqryn]

What version of protobuf and what language are you using?
Version: v1.4.2

What did you do?

func TestTimestampWKT(t *testing.T) {
	now := time.Now()

	pbt1, err := ptypes.TimestampProto(now)
	if err != nil {
		panic(err)
	}

	pbt2, err := ptypes.TimestampProto(now)
	if err != nil {
		panic(err)
	}

	if !reflect.DeepEqual(pbt1, pbt2) {
		t.Fatalf("Should be equal: %#v, %#v", pbt1, pbt2)
	}

	fmt.Println(pbt1)

	if !reflect.DeepEqual(pbt1, pbt2) {
		t.Fatalf("Should still be equal: %#v, %#v", pbt1, pbt2)
	}
}

What did you expect to see?
Test should pass

What did you see instead?
Test fails with:

=== RUN   TestTimestampWKT
seconds:1594889122 nanos:308953600
    TestTimestampWKT: endpoint_pixel_counts_test.go:139: Should still be equal: &timestamppb.Timestamp{state:impl.MessageState{NoUnkeyedLiterals:pragma.NoUnkeyedLiterals{}, DoNotCompare:pragma.DoNotCompare{}, DoNotCopy:pragma.DoNotCopy{}, atomicMessageInfo:(*impl.MessageInfo)(0xc0000908c0)}, sizeCache:0, unknownFields:[]uint8(nil), Seconds:1594889122, Nanos:308953600}, &timestamppb.Timestamp{state:impl.MessageState{NoUnkeyedLiterals:pragma.NoUnkeyedLiterals{}, DoNotCompare:pragma.DoNotCompare{}, DoNotCopy:pragma.DoNotCopy{}, atomicMessageInfo:(*impl.MessageInfo)(nil)}, sizeCache:0, unknownFields:[]uint8(nil), Seconds:1594889122, Nanos:308953600}
--- FAIL: TestTimestampWKT (0.00s)
FAIL

Anything else we should know about your project / environment?
This bug came up when I tried upgrading to the newest package version (from v1.3.5).
We have a test that uses gomock, and it takes in a protobuff with WKT timestamps in it.
Previously this test passed, but now it fails because we are printing the protobuf at some point.
This really should still pass.

[neild]

Read operations on messages can modify internal cached state in the message, as you are seeing here. For this and other reasons, reflect.DeepEqual does not work well on messages.

Instead, use the cmp package with the protocmp.Transform() option:

if !cmp.Equal(pbt1, pbt2, protocmp.Transform()) {
  ....
}

[neild]

...or in the simple case of just comparing two messages (not slices of messages, or some other more complex data structure including a message), just use proto.Equal.

The cmp package is useful in tests, because it can compare arbitrary data structures and cmp.Diff will produce a nice report of the diffs between two values.

[veqryn]

Makes sense, but it is a little harder when using test helpers like go mock:

This is the actual place it failed in our code:

	mockSnowflake.EXPECT().GetAggregationCounts(
		gomock.Any(), // ctx
		expectedParams, // Includes protobuf messages. Fails on golang/protobuf v1.4.2, passes on v1.3.5
	).Return(
		data,
		nil,
	)

I've solved it with this matcher:

func EqualsProto(x interface{}) gomock.Matcher {
	return equalsProtobufMatcher{x}
}

type equalsProtobufMatcher struct {
	x interface{}
}

func (e equalsProtobufMatcher) Matches(x interface{}) bool {
	return cmp.Equal(e.x, x, protocmp.Transform())
}

func (e equalsProtobufMatcher) String() string {
	return fmt.Sprintf("is equal to %v", e.x)
}

Then my test code looks like:

	mockSnowflake.EXPECT().GetAggregationCounts(
		gomock.Any(), // ctx
		EqualsProto(expectedParams),
	).Return(
		data,
		nil,
	)

[neild]

For future reference, I've added a FAQ entry for DeepEqual on messages:

https://developers.google.com/protocol-buffers/docs/reference/go/faq#deepequal