[FAB-4323] Improve code coverage of orderer filter

Add UT for orderer common filter and configfilter.

Change-Id: Iefdd2a702a3ce397dcda4466e5d754c1d40d95de
Signed-off-by: Jay Guo <[email protected]>

Author: guoger

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")
 }

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 {