From 4ae322bfcae0fa55888dbb8fbc6927863684e054 Mon Sep 17 00:00:00 2001 From: Jay Guo Date: Sun, 4 Jun 2017 19:21:02 +0800 Subject: [PATCH] [FAB-4323] Improve code coverage of orderer filter Add UT for orderer common filter and configfilter. Change-Id: Iefdd2a702a3ce397dcda4466e5d754c1d40d95de Signed-off-by: Jay Guo --- orderer/common/configtxfilter/filter_test.go | 100 +++++++++++++++---- orderer/common/filter/filter_test.go | 6 ++ 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/orderer/common/configtxfilter/filter_test.go b/orderer/common/configtxfilter/filter_test.go index f78377c9c0e..83aabff6a00 100644 --- a/orderer/common/configtxfilter/filter_test.go +++ b/orderer/common/configtxfilter/filter_test.go @@ -18,7 +18,6 @@ package configtxfilter import ( "fmt" - "reflect" "testing" mockconfigtx "github.com/hyperledger/fabric/common/mocks/configtx" @@ -27,16 +26,60 @@ import ( "github.com/hyperledger/fabric/protos/utils" "github.com/golang/protobuf/proto" + "github.com/stretchr/testify/assert" ) -func TestForwardNonConfig(t *testing.T) { +func TestForwardOpaquePayload(t *testing.T) { cf := NewFilter(&mockconfigtx.Manager{}) result, _ := cf.Apply(&cb.Envelope{ Payload: []byte("Opaque"), }) - if result != filter.Forward { - t.Fatal("Should have forwarded opaque message") - } + assert.EqualValues(t, filter.Forward, result, "Should have forwarded opaque message") +} + +func TestForwardNilHeader(t *testing.T) { + cf := NewFilter(&mockconfigtx.Manager{}) + result, _ := cf.Apply(&cb.Envelope{ + Payload: utils.MarshalOrPanic(&cb.Payload{ + Header: nil, + }), + }) + assert.EqualValues(t, filter.Forward, result, "Should have forwarded message with nil header") +} + +func TestForwardBadHeader(t *testing.T) { + cf := NewFilter(&mockconfigtx.Manager{}) + result, _ := cf.Apply(&cb.Envelope{ + Payload: utils.MarshalOrPanic(&cb.Payload{ + Header: &cb.Header{ChannelHeader: []byte("Hello, world!")}, + }), + }) + assert.EqualValues(t, filter.Forward, result, "Should have forwarded message with bad header") +} + +func TestForwardNonConfig(t *testing.T) { + cf := NewFilter(&mockconfigtx.Manager{}) + result, _ := cf.Apply(&cb.Envelope{ + Payload: utils.MarshalOrPanic(&cb.Payload{ + Header: &cb.Header{ChannelHeader: []byte{}}, + }), + }) + assert.EqualValues(t, filter.Forward, result, "Should have forwarded message with non-config message") +} + +func TestRejectMalformedData(t *testing.T) { + cf := NewFilter(&mockconfigtx.Manager{}) + result, _ := cf.Apply(&cb.Envelope{ + Payload: utils.MarshalOrPanic(&cb.Payload{ + Header: &cb.Header{ + ChannelHeader: utils.MarshalOrPanic(&cb.ChannelHeader{ + Type: int32(cb.HeaderType_CONFIG), + }), + }, + Data: []byte("Hello, world!"), + }), + }) + assert.EqualValues(t, filter.Reject, result, "Should have rejected message with malformed payload data") } func TestAcceptGoodConfig(t *testing.T) { @@ -65,19 +108,43 @@ func TestAcceptGoodConfig(t *testing.T) { Payload: configBytes, } result, committer := cf.Apply(configEnvelope) - if result != filter.Accept { - t.Fatal("Should have indicated a good config message causes a reconfig") - } - - if !committer.Isolated() { - t.Fatal("Config transactions should be isolated to their own block") - } + assert.EqualValues(t, filter.Accept, result, "Should have indicated a good config message causes a reconfig") + assert.True(t, committer.Isolated(), "Config transactions should be isolated to their own block") committer.Commit() + assert.Equal(t, mcm.AppliedConfigUpdateEnvelope, configEnv, "Should have applied new config on commit got %+v and %+v", mcm.AppliedConfigUpdateEnvelope, configEnv.LastUpdate) +} - if !reflect.DeepEqual(mcm.AppliedConfigUpdateEnvelope, configEnv) { - t.Fatalf("Should have applied new config on commit got %+v and %+v", mcm.AppliedConfigUpdateEnvelope, configEnv.LastUpdate) +func TestPanicApplyingValidatedConfig(t *testing.T) { + mcm := &mockconfigtx.Manager{ApplyVal: fmt.Errorf("Error applying config tx")} + cf := NewFilter(mcm) + configGroup := cb.NewConfigGroup() + configGroup.Values["Foo"] = &cb.ConfigValue{} + configUpdateEnv := &cb.ConfigUpdateEnvelope{ + ConfigUpdate: utils.MarshalOrPanic(configGroup), + } + configEnv := &cb.ConfigEnvelope{ + LastUpdate: &cb.Envelope{ + Payload: utils.MarshalOrPanic(&cb.Payload{ + Header: &cb.Header{ + ChannelHeader: utils.MarshalOrPanic(&cb.ChannelHeader{ + Type: int32(cb.HeaderType_CONFIG_UPDATE), + }), + }, + Data: utils.MarshalOrPanic(configUpdateEnv), + }), + }, } + configEnvBytes := utils.MarshalOrPanic(configEnv) + configBytes := utils.MarshalOrPanic(&cb.Payload{Header: &cb.Header{ChannelHeader: utils.MarshalOrPanic(&cb.ChannelHeader{Type: int32(cb.HeaderType_CONFIG)})}, Data: configEnvBytes}) + configEnvelope := &cb.Envelope{ + Payload: configBytes, + } + result, committer := cf.Apply(configEnvelope) + + assert.EqualValues(t, filter.Accept, result, "Should have indicated a good config message causes a reconfig") + assert.True(t, committer.Isolated(), "Config transactions should be isolated to their own block") + assert.Panics(t, func() { committer.Commit() }, "Should panic upon error applying a validated config tx") } func TestRejectBadConfig(t *testing.T) { @@ -87,7 +154,6 @@ func TestRejectBadConfig(t *testing.T) { result, _ := cf.Apply(&cb.Envelope{ Payload: configBytes, }) - if result != filter.Reject { - t.Fatal("Should have rejected bad config message") - } + + assert.EqualValues(t, filter.Reject, result, "Should have rejected bad config message") } diff --git a/orderer/common/filter/filter_test.go b/orderer/common/filter/filter_test.go index 4e20a8b9891..5781a86b699 100644 --- a/orderer/common/filter/filter_test.go +++ b/orderer/common/filter/filter_test.go @@ -20,6 +20,7 @@ import ( "testing" cb "github.com/hyperledger/fabric/protos/common" + "github.com/stretchr/testify/assert" ) var RejectRule = Rule(rejectRule{}) @@ -38,6 +39,11 @@ func (r forwardRule) Apply(message *cb.Envelope) (Action, Committer) { return Forward, nil } +func TestNoopCommitter(t *testing.T) { + var nc noopCommitter + assert.False(t, nc.Isolated(), "Should return false") +} + func TestEmptyRejectRule(t *testing.T) { result, _ := EmptyRejectRule.Apply(&cb.Envelope{}) if result != Reject {