Skip to content

Commit

Permalink
Use custom gomega matcher for protobuf equality (#4966)
Browse files Browse the repository at this point in the history 
The standard gomega Equal matcher is not reliable when used with
protobuf messages since they can be logically equal while containing
different implementation field values. This change implements a custom
ProtoEqual matcher that uses proto.Equal for reliable equality checks.

A failed match produces output similar to the following:

```
[FAILED] Expected
        ChaincodeMessage{
                type:  TRANSACTION
                payload:  "\n\x04arg1\n\x04arg2"
                txid:  "tx-id"
                proposal:  {
                        proposal_bytes:  "signed-proposal-bytes"
                        signature:  "signature"
                }
                channel_id:  "channel-id"
        }
  to equal
        SignedProposal{
                proposal_bytes:  "signed-proposal-bytes"
                signature:  "signature"
        }
```

Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>
  • Loading branch information
@bestbeforetoday
bestbeforetoday authored Aug 30, 2024
1 parent bbf2300 commit 08df37e
Show file tree
Hide file tree
Showing 21 changed files with 311 additions and 239 deletions.
3 changes: 2 additions & 1 deletion common/deliver/acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
cb "github.com/hyperledger/fabric-protos-go/common"
"github.com/hyperledger/fabric/common/deliver"
"github.com/hyperledger/fabric/common/deliver/mock"
. "github.com/hyperledger/fabric/internal/test"
"github.com/hyperledger/fabric/protoutil"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -47,7 +48,7 @@ var _ = Describe("SessionAccessControl", func() {

Expect(fakePolicyChecker.CheckPolicyCallCount()).To(Equal(1))
env, cid := fakePolicyChecker.CheckPolicyArgsForCall(0)
Expect(env).To(Equal(envelope))
Expect(env).To(ProtoEqual(envelope))
Expect(cid).To(Equal("chain-id"))
})

Expand Down
32 changes: 16 additions & 16 deletions common/deliver/deliver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"io"
"time"

"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes/timestamp"
"github.com/hyperledger/fabric-lib-go/common/metrics/disabled"
"github.com/hyperledger/fabric-lib-go/common/metrics/metricsfakes"
Expand All @@ -25,6 +24,7 @@ import (
"github.com/hyperledger/fabric/common/deliver/mock"
"github.com/hyperledger/fabric/common/ledger/blockledger"
"github.com/hyperledger/fabric/common/util"
. "github.com/hyperledger/fabric/internal/test"
"github.com/hyperledger/fabric/protoutil"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -295,7 +295,7 @@ var _ = Describe("Deliver", func() {
Expect(fakeInspector.InspectCallCount()).To(Equal(1))
ctx, header := fakeInspector.InspectArgsForCall(0)
Expect(ctx).To(Equal(context.Background()))
Expect(proto.Equal(header, channelHeader)).To(BeTrue())
Expect(header).To(ProtoEqual(channelHeader))
})

Context("when channel header validation fails", func() {
Expand Down Expand Up @@ -335,7 +335,7 @@ var _ = Describe("Deliver", func() {

Expect(fakePolicyChecker.CheckPolicyCallCount()).To(BeNumerically(">=", 1))
e, cid := fakePolicyChecker.CheckPolicyArgsForCall(0)
Expect(proto.Equal(e, envelope)).To(BeTrue())
Expect(e).To(ProtoEqual(envelope))
Expect(cid).To(Equal("chain-id"))
})

Expand All @@ -345,7 +345,7 @@ var _ = Describe("Deliver", func() {

Expect(fakeBlockReader.IteratorCallCount()).To(Equal(1))
startPosition := fakeBlockReader.IteratorArgsForCall(0)
Expect(proto.Equal(startPosition, seekInfo.Start)).To(BeTrue())
Expect(startPosition).To(ProtoEqual(seekInfo.Start))
})

Context("when multiple blocks are requested", func() {
Expand Down Expand Up @@ -427,12 +427,12 @@ var _ = Describe("Deliver", func() {

Expect(fakeBlockReader.IteratorCallCount()).To(Equal(1))
start := fakeBlockReader.IteratorArgsForCall(0)
Expect(start).To(Equal(&ab.SeekPosition{}))
Expect(start).To(ProtoEqual(&ab.SeekPosition{}))
Expect(fakeBlockIterator.NextCallCount()).To(Equal(1))

Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(1))
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(0)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: 100},
}))
})
Expand All @@ -457,13 +457,13 @@ var _ = Describe("Deliver", func() {

Expect(fakeBlockReader.IteratorCallCount()).To(Equal(1))
start := fakeBlockReader.IteratorArgsForCall(0)
Expect(start).To(Equal(&ab.SeekPosition{}))
Expect(start).To(ProtoEqual(&ab.SeekPosition{}))

Expect(fakeBlockIterator.NextCallCount()).To(Equal(2))
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(2))
for i := 0; i < fakeResponseSender.SendBlockResponseCallCount(); i++ {
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(i)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: uint64(i + 1)},
}))
}
Expand Down Expand Up @@ -494,7 +494,7 @@ var _ = Describe("Deliver", func() {
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(1))
for i := 0; i < fakeResponseSender.SendBlockResponseCallCount(); i++ {
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(i)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: uint64(i)},
}))
}
Expand Down Expand Up @@ -528,13 +528,13 @@ var _ = Describe("Deliver", func() {

Expect(fakeBlockReader.IteratorCallCount()).To(Equal(1))
start := fakeBlockReader.IteratorArgsForCall(0)
Expect(start).To(Equal(&ab.SeekPosition{}))
Expect(start).To(ProtoEqual(&ab.SeekPosition{}))

Expect(fakeBlockIterator.NextCallCount()).To(Equal(2))
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(2))
for i := 0; i < fakeResponseSender.SendBlockResponseCallCount(); i++ {
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(i)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: uint64(i + 1)},
Data: nil,
Metadata: &cb.BlockMetadata{Metadata: [][]byte{{3}, {4}}},
Expand Down Expand Up @@ -584,13 +584,13 @@ var _ = Describe("Deliver", func() {
Expect(err).NotTo(HaveOccurred())
Expect(fakeBlockReader.IteratorCallCount()).To(Equal(1))
start := fakeBlockReader.IteratorArgsForCall(0)
Expect(start).To(Equal(&ab.SeekPosition{}))
Expect(start).To(ProtoEqual(&ab.SeekPosition{}))
Expect(fakeBlockIterator.NextCallCount()).To(Equal(3))
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(3))
for i := 0; i < fakeResponseSender.SendBlockResponseCallCount(); i++ {
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(i)
if i+1 == 1 || i+1 == 3 {
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: uint64(i + 1)},
Data: nil,
Metadata: &cb.BlockMetadata{Metadata: [][]byte{{3}, {4}}},
Expand All @@ -607,7 +607,7 @@ var _ = Describe("Deliver", func() {
Data: &cb.BlockData{Data: [][]byte{protoutil.MarshalOrPanic(envelope)}},
Metadata: &cb.BlockMetadata{Metadata: [][]byte{{3}, {4}}},
}
Expect(b).To(Equal(blk))
Expect(b).To(ProtoEqual(blk))
Expect(b.Data.Data).NotTo(BeNil())
}
}
Expand Down Expand Up @@ -702,7 +702,7 @@ var _ = Describe("Deliver", func() {
Expect(fakeResponseSender.DataTypeCallCount()).To(Equal(1))
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(1))
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(0)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: 100},
}))
})
Expand Down Expand Up @@ -769,7 +769,7 @@ var _ = Describe("Deliver", func() {
Expect(err).NotTo(HaveOccurred())
Expect(fakeResponseSender.SendBlockResponseCallCount()).To(Equal(1))
b, _, _, _ := fakeResponseSender.SendBlockResponseArgsForCall(0)
Expect(b).To(Equal(&cb.Block{
Expect(b).To(ProtoEqual(&cb.Block{
Header: &cb.BlockHeader{Number: 100},
}))
})
Expand Down
12 changes: 6 additions & 6 deletions common/deliverclient/blocksprovider/deliverer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"sync"
"time"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric-lib-go/bccsp"
"github.com/hyperledger/fabric-lib-go/bccsp/sw"
"github.com/hyperledger/fabric-lib-go/common/flogging"
Expand All @@ -26,6 +25,7 @@ import (
"github.com/hyperledger/fabric/core/config/configtest"
"github.com/hyperledger/fabric/internal/configtxgen/encoder"
"github.com/hyperledger/fabric/internal/configtxgen/genesisconfig"
. "github.com/hyperledger/fabric/internal/test"
"github.com/hyperledger/fabric/protoutil"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -575,11 +575,11 @@ var _ = Describe("CFT-Deliverer", func() {
It("checks the validity of the block", func() {
Eventually(fakeUpdatableBlockVerifier.VerifyBlockCallCount, eventuallyTO).Should(Equal(1))
block := fakeUpdatableBlockVerifier.VerifyBlockArgsForCall(0)
Expect(proto.Equal(block, &common.Block{
Expect(block).To(ProtoEqual(&common.Block{
Header: &common.BlockHeader{
Number: 8,
},
})).To(BeTrue())
}))
})

When("the block is invalid", func() {
Expand Down Expand Up @@ -679,19 +679,19 @@ var _ = Describe("CFT-Deliverer", func() {
It("checks the validity of the block", func() {
Eventually(fakeUpdatableBlockVerifier.VerifyBlockCallCount, eventuallyTO).Should(Equal(1))
block := fakeUpdatableBlockVerifier.VerifyBlockArgsForCall(0)
Expect(proto.Equal(block, &common.Block{
Expect(block).To(ProtoEqual(&common.Block{
Header: &common.BlockHeader{Number: 8},
Data: &common.BlockData{
Data: [][]byte{protoutil.MarshalOrPanic(env)},
},
})).To(BeTrue())
}))
})

It("handle the block and updates the verifier config", func() {
Eventually(fakeBlockHandler.HandleBlockCallCount, eventuallyTO).Should(Equal(1))
channelID, block := fakeBlockHandler.HandleBlockArgsForCall(0)
Expect(channelID).To(Equal("channel-id"))
Expect(block).To(Equal(&common.Block{
Expect(block).To(ProtoEqual(&common.Block{
Header: &common.BlockHeader{Number: 8},
Data: &common.BlockData{
Data: [][]byte{protoutil.MarshalOrPanic(env)},
Expand Down
11 changes: 6 additions & 5 deletions common/grpclogging/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/hyperledger/fabric/common/grpclogging"
"github.com/hyperledger/fabric/common/grpclogging/fakes"
"github.com/hyperledger/fabric/common/grpclogging/testpb"
. "github.com/hyperledger/fabric/internal/test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.uber.org/zap"
Expand Down Expand Up @@ -105,7 +106,7 @@ var _ = Describe("Server", func() {

resp, err := echoServiceClient.Echo(ctx, &testpb.Message{Message: "hi"})
Expect(err).NotTo(HaveOccurred())
Expect(resp).To(Equal(&testpb.Message{Message: "hi", Sequence: 1}))
Expect(resp).To(ProtoEqual(&testpb.Message{Message: "hi", Sequence: 1}))

var logMessages []string
for _, entry := range observed.AllUntimed() {
Expand Down Expand Up @@ -332,7 +333,7 @@ var _ = Describe("Server", func() {

msg, err := streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 1}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 1}))

err = streamClient.CloseSend()
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -420,7 +421,7 @@ var _ = Describe("Server", func() {

msg, err := streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 1}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 1}))

err = streamClient.CloseSend()
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -472,7 +473,7 @@ var _ = Describe("Server", func() {

msg, err := streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 1}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 1}))

err = streamClient.CloseSend()
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -595,7 +596,7 @@ var _ = Describe("Server", func() {
Expect(err).NotTo(HaveOccurred())
msg, err := streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 1}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 1}))

err = streamClient.CloseSend()
Expect(err).NotTo(HaveOccurred())
Expand Down
11 changes: 6 additions & 5 deletions common/grpcmetrics/interceptor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/hyperledger/fabric/common/grpcmetrics"
"github.com/hyperledger/fabric/common/grpcmetrics/fakes"
"github.com/hyperledger/fabric/common/grpcmetrics/testpb"
. "github.com/hyperledger/fabric/internal/test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"google.golang.org/grpc"
Expand Down Expand Up @@ -120,7 +121,7 @@ var _ = Describe("Interceptor", func() {
It("records request duration", func() {
resp, err := echoServiceClient.Echo(context.Background(), &testpb.Message{Message: "yo"})
Expect(err).NotTo(HaveOccurred())
Expect(resp).To(Equal(&testpb.Message{Message: "yo", Sequence: 1}))
Expect(resp).To(ProtoEqual(&testpb.Message{Message: "yo", Sequence: 1}))

Expect(fakeRequestDuration.WithCallCount()).To(Equal(1))
labelValues := fakeRequestDuration.WithArgsForCall(0)
Expand All @@ -142,7 +143,7 @@ var _ = Describe("Interceptor", func() {

resp, err := echoServiceClient.Echo(context.Background(), &testpb.Message{Message: "yo"})
Expect(err).NotTo(HaveOccurred())
Expect(resp).To(Equal(&testpb.Message{Message: "yo", Sequence: 1}))
Expect(resp).To(ProtoEqual(&testpb.Message{Message: "yo", Sequence: 1}))

Expect(fakeRequestsReceived.WithCallCount()).To(Equal(1))
labelValues := fakeRequestsReceived.WithArgsForCall(0)
Expand All @@ -162,7 +163,7 @@ var _ = Describe("Interceptor", func() {

resp, err := echoServiceClient.Echo(context.Background(), &testpb.Message{Message: "yo"})
Expect(err).NotTo(HaveOccurred())
Expect(resp).To(Equal(&testpb.Message{Message: "yo", Sequence: 1}))
Expect(resp).To(ProtoEqual(&testpb.Message{Message: "yo", Sequence: 1}))

Expect(fakeRequestsCompleted.WithCallCount()).To(Equal(1))
labelValues := fakeRequestsCompleted.WithArgsForCall(0)
Expand Down Expand Up @@ -313,10 +314,10 @@ func streamMessages(streamClient testpb.EchoService_EchoStreamClient) {

msg, err := streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 1}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 1}))
msg, err = streamClient.Recv()
Expect(err).NotTo(HaveOccurred())
Expect(msg).To(Equal(&testpb.Message{Message: "hello", Sequence: 3}))
Expect(msg).To(ProtoEqual(&testpb.Message{Message: "hello", Sequence: 3}))

err = streamClient.CloseSend()
Expect(err).NotTo(HaveOccurred())
Expand Down
Loading

0 comments on commit 08df37e

Please sign in to comment.