From 07501137c48e264623d4c4441da152fcbce91c5b Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Fri, 1 Mar 2019 14:49:06 +0100 Subject: [PATCH] reworking unit tests --- privval/signer_remote.go | 4 +- privval/signer_remote_test.go | 205 ++++++++++++++++++-- privval/signer_validator_endpoint.go | 3 + privval/signer_validator_endpoint_test.go | 223 +--------------------- privval/socket_dialers_test.go | 20 ++ 5 files changed, 221 insertions(+), 234 deletions(-) diff --git a/privval/signer_remote.go b/privval/signer_remote.go index 92e53f757..b0c898c5f 100644 --- a/privval/signer_remote.go +++ b/privval/signer_remote.go @@ -10,7 +10,7 @@ import ( // SignerRemote implements PrivValidator. // It uses a net.Conn to request signatures from an external process. type SignerRemote struct { - endpoint SignerValidatorEndpoint + endpoint *SignerValidatorEndpoint // memoized consensusPubKey crypto.PubKey @@ -20,7 +20,7 @@ type SignerRemote struct { var _ types.PrivValidator = (*SignerRemote)(nil) // NewSignerRemote returns an instance of SignerRemote. -func NewSignerRemote(endpoint SignerValidatorEndpoint) (*SignerRemote, error) { +func NewSignerRemote(endpoint *SignerValidatorEndpoint) (*SignerRemote, error) { // TODO: Fix this //// retrieve and memoize the consensus public key once. diff --git a/privval/signer_remote_test.go b/privval/signer_remote_test.go index da41903a3..7026c241f 100644 --- a/privval/signer_remote_test.go +++ b/privval/signer_remote_test.go @@ -2,29 +2,212 @@ package privval import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/types" "testing" + "time" ) +type signerTestCase struct { + chainID string + mockPV types.PrivValidator + signer *SignerRemote + signerService *SignerServiceEndpoint // TODO: Replace once it is encapsulated +} + +func getSignerTestCases(t *testing.T) []signerTestCase { + testCases := make([]signerTestCase, 0) + + for _, dtc := range getDialerTestCases(t) { + chainID := common.RandStr(12) + mockPV := types.NewMockPV() + + ve, se := getMockEndpoints(t, chainID, mockPV, dtc.addr, dtc.dialer) + sr, err := NewSignerRemote(ve) + assert.NoError(t, err) + + tc := signerTestCase{ + chainID: chainID, + mockPV: mockPV, + signer: sr, + signerService: se, + } + + testCases = append(testCases, tc) + } + + return testCases +} + +func TestSignerClose(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + err := tc.signer.Close() + assert.NoError(t, err) + + // FIXME: An error is logged but OnStop hides it + err = tc.signer.endpoint.Stop() + assert.NoError(t, err) + + //// FIXME: An error is logged but OnStop hides it + //err = tc.signerService.Stop() + //assert.NoError(t, err) + }() + } +} + func TestSignerGetPubKey(t *testing.T) { - for _, tc := range getTestCases(t) { + for _, tc := range getSignerTestCases(t) { func() { - chainID := common.RandStr(12) - mockPV := types.NewMockPV() + // FIXME: There are some errors logged that need to be checked + defer tc.signer.Close() + defer tc.signerService.OnStop() - validatorEndpoint, serviceEndpoint := getMockEndpoints(t, chainID, mockPV, tc.addr, tc.dialer) + pubKey := tc.signer.GetPubKey() + expectedPubKey := tc.mockPV.GetPubKey() - sr, err := NewSignerRemote(*validatorEndpoint) - assert.NoError(t, err) + assert.Equal(t, expectedPubKey, pubKey) + + addr := tc.signer.GetPubKey().Address() + expectedAddr := tc.mockPV.GetPubKey().Address() + + assert.Equal(t, expectedAddr, addr) + }() + } +} + +func TestSignerProposal(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + want := &types.Proposal{Timestamp: ts} + have := &types.Proposal{Timestamp: ts} + + defer tc.signer.Close() + defer tc.signerService.OnStop() + + require.NoError(t, tc.mockPV.SignProposal(tc.chainID, want)) + require.NoError(t, tc.signer.SignProposal(tc.chainID, have)) + + assert.Equal(t, want.Signature, have.Signature) + }() + } +} + +func TestSignerVote(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + want := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + have := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + + defer tc.signer.Close() + defer tc.signerService.OnStop() + + require.NoError(t, tc.mockPV.SignVote(tc.chainID, want)) + require.NoError(t, tc.signer.SignVote(tc.chainID, have)) + + assert.Equal(t, want.Signature, have.Signature) + }() + } +} + +func TestSignerVoteResetDeadline(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + want := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + have := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + + defer tc.signer.Close() + defer tc.signerService.OnStop() + + time.Sleep(testTimeoutReadWrite2o3) + + require.NoError(t, tc.mockPV.SignVote(tc.chainID, want)) + require.NoError(t, tc.signer.SignVote(tc.chainID, have)) + assert.Equal(t, want.Signature, have.Signature) + + // FIXME: Lots of ping errors that do not bubble up + // TODO: Clarify what is actually being tested + + // This would exceed the deadline if it was not extended by the previous message + time.Sleep(testTimeoutReadWrite2o3) + + require.NoError(t, tc.mockPV.SignVote(tc.chainID, want)) + require.NoError(t, tc.signer.SignVote(tc.chainID, have)) + assert.Equal(t, want.Signature, have.Signature) + }() + } +} + +func TestSignerVoteKeepAlive(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + want := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + have := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + + defer tc.signer.Close() + defer tc.signerService.OnStop() + + time.Sleep(testTimeoutReadWrite * 2) + + require.NoError(t, tc.mockPV.SignVote(tc.chainID, want)) + require.NoError(t, tc.signer.SignVote(tc.chainID, have)) + assert.Equal(t, want.Signature, have.Signature) + + // FIXME: Lots of ping errors that do not bubble up + // TODO: Clarify what is actually being tested and how it differs from TestSignerVoteResetDeadline + }() + } +} + +func TestSignerSignProposalErrors(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + proposal := &types.Proposal{Timestamp: ts} + + tc.signerService.privVal = types.NewErroringMockPV() + tc.mockPV = types.NewErroringMockPV() + + defer tc.signer.Close() + defer tc.signerService.OnStop() + + err := tc.signer.SignProposal(tc.chainID, proposal) + require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error()) + + err = tc.mockPV.SignProposal(tc.chainID, proposal) + require.Error(t, err) + + err = tc.signer.SignProposal(tc.chainID, proposal) + require.Error(t, err) + }() + } +} + +func TestSignerSignVoteErrors(t *testing.T) { + for _, tc := range getSignerTestCases(t) { + func() { + ts := time.Now() + vote := &types.Vote{Timestamp: ts, Type: types.PrecommitType} + + tc.signerService.privVal = types.NewErroringMockPV() + tc.mockPV = types.NewErroringMockPV() + + defer tc.signer.Close() + defer tc.signerService.OnStop() - defer validatorEndpoint.Stop() - defer serviceEndpoint.Stop() + err := tc.signer.SignVote(tc.chainID, vote) + require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error()) - clientKey := sr.GetPubKey() - expectedPubKey := mockPV.GetPubKey() + err = tc.mockPV.SignVote(tc.chainID, vote) + require.Error(t, err) - assert.Equal(t, expectedPubKey, clientKey) + err = tc.signer.SignVote(tc.chainID, vote) + require.Error(t, err) }() } } diff --git a/privval/signer_validator_endpoint.go b/privval/signer_validator_endpoint.go index caa1eb04d..833ae7e98 100644 --- a/privval/signer_validator_endpoint.go +++ b/privval/signer_validator_endpoint.go @@ -195,6 +195,9 @@ func (ve *SignerValidatorEndpoint) readMessage() (msg RemoteSignerMsg, err error func (ve *SignerValidatorEndpoint) writeMessage(msg RemoteSignerMsg) (err error) { // TODO: Check connection status + if ve.conn == nil { + return fmt.Errorf("endpoint is not connected") + } _, err = cdc.MarshalBinaryLengthPrefixedWriter(ve.conn, msg) if _, ok := err.(timeoutError); ok { diff --git a/privval/signer_validator_endpoint_test.go b/privval/signer_validator_endpoint_test.go index dccbde654..a0e2d108d 100644 --- a/privval/signer_validator_endpoint_test.go +++ b/privval/signer_validator_endpoint_test.go @@ -1,7 +1,6 @@ package privval import ( - "fmt" "net" "testing" "time" @@ -24,7 +23,7 @@ var ( testTimeoutHeartbeat3o2 = 6 * time.Millisecond // 3/2 of the other one ) -type socketTestCase struct { +type dialerTestCase struct { addr string dialer SocketDialer } @@ -83,146 +82,6 @@ func TestSignerRemoteRetryTCPOnly(t *testing.T) { } } -//func TestSocketPVAddress(t *testing.T) { -// for _, tc := range getTestCases(t) { -// // Execute the test within a closure to ensure the deferred statements -// // are called between each for loop iteration, for isolated test cases. -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints(t, chainID, types.NewMockPV(), tc.addr, tc.dialer) -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// serviceAddr := serviceEndpoint.privVal.GetPubKey().Address() -// validatorAddr := validatorEndpoint.GetPubKey().Address() -// -// assert.Equal(t, serviceAddr, validatorAddr) -// }() -// } -//} -// -// -//func TestSocketPVProposal(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// privProposal = &types.Proposal{Timestamp: ts} -// clientProposal = &types.Proposal{Timestamp: ts} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// require.NoError(t, serviceEndpoint.privVal.SignProposal(chainID, privProposal)) -// require.NoError(t, validatorEndpoint.SignProposal(chainID, clientProposal)) -// -// assert.Equal(t, privProposal.Signature, clientProposal.Signature) -// }() -// } -//} -// -//func TestSocketPVVote(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// vType = types.PrecommitType -// want = &types.Vote{Timestamp: ts, Type: vType} -// have = &types.Vote{Timestamp: ts, Type: vType} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// require.NoError(t, serviceEndpoint.privVal.SignVote(chainID, want)) -// require.NoError(t, validatorEndpoint.SignVote(chainID, have)) -// assert.Equal(t, want.Signature, have.Signature) -// }() -// } -//} -// -//func TestSocketPVVoteResetDeadline(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// vType = types.PrecommitType -// want = &types.Vote{Timestamp: ts, Type: vType} -// have = &types.Vote{Timestamp: ts, Type: vType} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// time.Sleep(testTimeoutReadWrite2o3) -// -// require.NoError(t, serviceEndpoint.privVal.SignVote(chainID, want)) -// require.NoError(t, validatorEndpoint.SignVote(chainID, have)) -// assert.Equal(t, want.Signature, have.Signature) -// -// // This would exceed the deadline if it was not extended by the previous message -// time.Sleep(testTimeoutReadWrite2o3) -// -// require.NoError(t, serviceEndpoint.privVal.SignVote(chainID, want)) -// require.NoError(t, validatorEndpoint.SignVote(chainID, have)) -// assert.Equal(t, want.Signature, have.Signature) -// }() -// } -//} -// -//func TestSocketPVVoteKeepalive(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// vType = types.PrecommitType -// want = &types.Vote{Timestamp: ts, Type: vType} -// have = &types.Vote{Timestamp: ts, Type: vType} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// time.Sleep(testTimeoutReadWrite * 2) -// -// require.NoError(t, serviceEndpoint.privVal.SignVote(chainID, want)) -// require.NoError(t, validatorEndpoint.SignVote(chainID, have)) -// assert.Equal(t, want.Signature, have.Signature) -// }() -// } -//} -// //func TestSocketPVDeadline(t *testing.T) { // for _, tc := range getTestCases(t) { // func() { @@ -254,67 +113,7 @@ func TestSignerRemoteRetryTCPOnly(t *testing.T) { // }() // } //} -// -//func TestRemoteSignVoteErrors(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewErroringMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// vType = types.PrecommitType -// vote = &types.Vote{Timestamp: ts, Type: vType} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// err := validatorEndpoint.SignVote("", vote) -// require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error()) -// -// err = serviceEndpoint.privVal.SignVote(chainID, vote) -// require.Error(t, err) -// err = validatorEndpoint.SignVote(chainID, vote) -// require.Error(t, err) -// }() -// } -//} -// -//func TestRemoteSignProposalErrors(t *testing.T) { -// for _, tc := range getTestCases(t) { -// func() { -// var ( -// chainID = common.RandStr(12) -// validatorEndpoint, serviceEndpoint = getMockEndpoints( -// t, -// chainID, -// types.NewErroringMockPV(), -// tc.addr, -// tc.dialer) -// -// ts = time.Now() -// proposal = &types.Proposal{Timestamp: ts} -// ) -// defer validatorEndpoint.Stop() -// defer serviceEndpoint.Stop() -// -// err := validatorEndpoint.SignProposal("", proposal) -// require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error()) -// -// err = serviceEndpoint.privVal.SignProposal(chainID, proposal) -// require.Error(t, err) -// -// err = validatorEndpoint.SignProposal(chainID, proposal) -// require.Error(t, err) -// }() -// } -//} -// + //func TestErrUnexpectedResponse(t *testing.T) { // for _, tc := range getTestCases(t) { // func() { @@ -431,24 +230,6 @@ func TestSignerRemoteRetryTCPOnly(t *testing.T) { /////////////////////////////////// -func getTestCases(t *testing.T) []socketTestCase { - tcpAddr := fmt.Sprintf("tcp://%s", testFreeTCPAddr(t)) - unixFilePath, err := testUnixAddr() - require.NoError(t, err) - unixAddr := fmt.Sprintf("unix://%s", unixFilePath) - - return []socketTestCase{ - { - addr: tcpAddr, - dialer: DialTCPFn(tcpAddr, testTimeoutReadWrite, ed25519.GenPrivKey()), - }, - { - addr: unixAddr, - dialer: DialUnixFn(unixFilePath), - }, - } -} - func newSignerValidatorEndpoint(logger log.Logger, addr string, timeoutReadWrite time.Duration) *SignerValidatorEndpoint { proto, address := common.ProtocolAndAddress(addr) diff --git a/privval/socket_dialers_test.go b/privval/socket_dialers_test.go index 9d5d5cc2b..a908c7d27 100644 --- a/privval/socket_dialers_test.go +++ b/privval/socket_dialers_test.go @@ -1,6 +1,8 @@ package privval import ( + "fmt" + "github.com/stretchr/testify/require" "testing" "time" @@ -9,6 +11,24 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" ) +func getDialerTestCases(t *testing.T) []dialerTestCase { + tcpAddr := fmt.Sprintf("tcp://%s", testFreeTCPAddr(t)) + unixFilePath, err := testUnixAddr() + require.NoError(t, err) + unixAddr := fmt.Sprintf("unix://%s", unixFilePath) + + return []dialerTestCase{ + { + addr: tcpAddr, + dialer: DialTCPFn(tcpAddr, testTimeoutReadWrite, ed25519.GenPrivKey()), + }, + { + addr: unixAddr, + dialer: DialUnixFn(unixFilePath), + }, + } +} + func TestIsConnTimeoutForFundamentalTimeouts(t *testing.T) { // Generate a networking timeout dialer := DialTCPFn(testFreeTCPAddr(t), time.Millisecond, ed25519.GenPrivKey())