Tests are failing with more recent versions of google/protobuf

[vincentbernat]

Hey!

When regenerating *.pb.go files with a more recent version, many tests are failing:

--- FAIL: Test_AddPathCapability (0.00s)
    capability_test.go:187:
                Error Trace:    capability_test.go:187
                Error:          Not equal:
                                expected: &gobgpapi.AddPathCapability{Tuples:[]*gobgpapi.AddPathCapabilityTuple{(*gobgpapi.AddPathCapabilityTuple)(0xc0002b5f20)}, XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:10}
                                actual  : &gobgpapi.AddPathCapability{Tuples:[]*gobgpapi.AddPathCapabilityTuple{(*gobgpapi.AddPathCapabilityTuple)(0xc0002fa0c0)}, XXX_NoUnkeyedLiteral:struct {}{}, XXX_unrecognized:[]uint8(nil), XXX_sizecache:0}

                                Diff:
                Test:           Test_AddPathCapability

This is due to the addition of XXX_sizecache. Authors suggest to use proto.Equal() instead of reflect.DeepEqual(). However, as the test is done through assert.Equal(), I am unsure of what the right solution is.

Testify has an issue and a PR to use go-cmp instead, but I don't think it would change anything since the structures don't come with an Equal() method. We could either have a wrapper around assert.Equal() to use proto.Equal() if possible first. Or just use a dedicated helper when we compare proto messages. The first approach seems less error-prone.

[fujita]

Thanks for the report. What's the version of protobuf?

Due to the following golang bug, I can't update the dependency for the newer protobuf but I'll try after it's fixed.
golang/go#30833

[vincentbernat]

This is with 1.2.0.

[vincentbernat]

Hey! Any news on that?

[fujita]

I tend to simply update the test code use proto.Equal() like

@@ -64,7 +65,9 @@ func Test_AsPathAttribute(t *testing.T) {
 
        output := NewAsPathAttributeFromNative(n.(*bgp.PathAttributeAsPath))
        assert.Equal(2, len(output.Segments))
-       assert.Equal(input.Segments, output.Segments)
+       for i := 0; i < len(input.Segments); i++ {
+               assert.True(proto.Equal(input.Segments[i], output.Segments[i]))
+       }
 }

[vincentbernat]

A long term solution would be for testify to use something like go-cmp. This is tracked here: stretchr/testify#535. In the meantime, I will provide a PR with your proposed change if you are OK with it.

[fujita]

Great, thanks a lot! btw, what's the advantage of using the latest protobuf/grpc?

[vincentbernat]

None, but that's what in Debian and to package gobgpd into Debian, I have to use the versions in Debian (by policy).