From 2a6e71a753c14b2563714e69d2e97ea5472dc8b0 Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Thu, 31 Aug 2017 13:41:28 +0200 Subject: [PATCH 1/4] Reformat tests to extract common setup --- state/state_test.go | 59 +++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/state/state_test.go b/state/state_test.go index 713d76f8..2576b35d 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -8,7 +8,9 @@ import ( "github.com/stretchr/testify/assert" abci "github.com/tendermint/abci/types" + crypto "github.com/tendermint/go-crypto" + cmn "github.com/tendermint/tmlibs/common" dbm "github.com/tendermint/tmlibs/db" "github.com/tendermint/tmlibs/log" @@ -17,31 +19,35 @@ import ( "github.com/tendermint/tendermint/types" ) -func TestStateCopyEquals(t *testing.T) { - assert := assert.New(t) - config := cfg.ResetTestRoot("state_") +func setupTestCase(t *testing.T) (func(t *testing.T), dbm.DB, *State) { - // Get State db + config := cfg.ResetTestRoot("state_") stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) state := GetState(stateDB, config.GenesisFile()) state.SetLogger(log.TestingLogger()) + tearDown := func(t *testing.T) {} + + return tearDown, stateDB, state +} + +func TestStateCopy(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) + assert := assert.New(t) + stateCopy := state.Copy() assert.True(state.Equals(stateCopy), cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) - stateCopy.LastBlockHeight += 1 + stateCopy.LastBlockHeight++ assert.False(state.Equals(stateCopy), cmn.Fmt("expected states to be different. got same %v", state)) } func TestStateSaveLoad(t *testing.T) { - assert := assert.New(t) - config := cfg.ResetTestRoot("state_") - // Get State db - stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) - state := GetState(stateDB, config.GenesisFile()) - state.SetLogger(log.TestingLogger()) + tearDown, stateDB, state := setupTestCase(t) + defer tearDown(t) - state.LastBlockHeight += 1 + state.LastBlockHeight++ state.Save() loadedState := LoadState(stateDB) @@ -49,14 +55,11 @@ func TestStateSaveLoad(t *testing.T) { } func TestABCIResponsesSaveLoad(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) assert := assert.New(t) - config := cfg.ResetTestRoot("state_") - stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) - state := GetState(stateDB, config.GenesisFile()) - state.SetLogger(log.TestingLogger()) - - state.LastBlockHeight += 1 + state.LastBlockHeight++ // build mock responses block := makeBlock(2, state) @@ -77,12 +80,9 @@ func TestABCIResponsesSaveLoad(t *testing.T) { } func TestValidatorSimpleSaveLoad(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) assert := assert.New(t) - config := cfg.ResetTestRoot("state_") - // Get State db - stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) - state := GetState(stateDB, config.GenesisFile()) - state.SetLogger(log.TestingLogger()) // cant load anything for height 0 v, err := state.LoadValidators(0) @@ -94,7 +94,7 @@ func TestValidatorSimpleSaveLoad(t *testing.T) { assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") // increment height, save; should be able to load for next height - state.LastBlockHeight += 1 + state.LastBlockHeight++ state.saveValidatorsInfo() v, err = state.LoadValidators(state.LastBlockHeight + 1) assert.Nil(err, "expected no err") @@ -113,12 +113,9 @@ func TestValidatorSimpleSaveLoad(t *testing.T) { } func TestValidatorChangesSaveLoad(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) assert := assert.New(t) - config := cfg.ResetTestRoot("state_") - // Get State db - stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) - state := GetState(stateDB, config.GenesisFile()) - state.SetLogger(log.TestingLogger()) // change vals at these heights changeHeights := []int{1, 2, 4, 5, 10, 15, 16, 17, 20} @@ -141,7 +138,7 @@ func TestValidatorChangesSaveLoad(t *testing.T) { // when we get to a change height, // use the next pubkey if changeIndex < len(changeHeights) && i == changeHeights[changeIndex] { - changeIndex += 1 + changeIndex++ pubkey = pubkeys[changeIndex] } header, parts, responses := makeHeaderPartsResponses(state, i, pubkey) @@ -157,7 +154,7 @@ func TestValidatorChangesSaveLoad(t *testing.T) { // we we get to the height after a change height // use the next pubkey (note our counter starts at 0 this time) if changeIndex < len(changeHeights) && i == changeHeights[changeIndex]+1 { - changeIndex += 1 + changeIndex++ pubkey = pubkeys[changeIndex] } testCases[i-1] = valChangeTestCase{i, pubkey} From 8eda3efa28dd7d129ef403a73830a7519719adb6 Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Thu, 31 Aug 2017 13:54:08 +0200 Subject: [PATCH 2/4] Cleanup lines to fit within 72 characters --- state/state.go | 21 ++++++++++++--------- state/state_test.go | 15 ++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/state/state.go b/state/state.go index 72c3e64d..a9a2fee8 100644 --- a/state/state.go +++ b/state/state.go @@ -42,17 +42,18 @@ type State struct { GenesisDoc *types.GenesisDoc ChainID string - // updated at end of SetBlockAndValidators - LastBlockHeight int // Genesis state has this set to 0. So, Block(H=0) does not exist. + // Updated at end of SetBlockAndValidators + // Genesis state has this set to 0. So, Block(H=0) does not exist + LastBlockHeight int LastBlockID types.BlockID LastBlockTime time.Time Validators *types.ValidatorSet - LastValidators *types.ValidatorSet // block.LastCommit validated against this - + // block.LastCommit validated against LastValidators + LastValidators *types.ValidatorSet // AppHash is updated after Commit AppHash []byte - TxIndexer txindex.TxIndexer `json:"-"` // Transaction indexer. + TxIndexer txindex.TxIndexer `json:"-"` // Transaction indexer // When a block returns a validator set change via EndBlock, // the change only applies to the next block. @@ -224,7 +225,8 @@ func (s *State) SetBlockAndValidators(header *types.Header, blockPartsHeader typ nextValSet.IncrementAccum(1) s.setBlockAndValidators(header.Height, - types.BlockID{header.Hash(), blockPartsHeader}, header.Time, + types.BlockID{header.Hash(), blockPartsHeader}, + header.Time, prevValSet, nextValSet) } @@ -258,7 +260,7 @@ func GetState(stateDB dbm.DB, genesisFile string) *State { return state } -//-------------------------------------------------- +//------------------------------------------------------------------------ // ABCIResponses retains the responses of the various ABCI calls during block processing. // It is persisted to disk before calling Commit. @@ -298,10 +300,11 @@ func (vi *ValidatorsInfo) Bytes() []byte { return wire.BinaryBytes(*vi) } -//----------------------------------------------------------------------------- +//------------------------------------------------------------------------ // Genesis -// MakeGenesisStateFromFile reads and unmarshals state from the given file. +// MakeGenesisStateFromFile reads and unmarshals state from the given +// file. // // Used during replay and in tests. func MakeGenesisStateFromFile(db dbm.DB, genDocFile string) *State { diff --git a/state/state_test.go b/state/state_test.go index 2576b35d..a22e9958 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -19,8 +19,8 @@ import ( "github.com/tendermint/tendermint/types" ) +// setupTestCase does setup common to all test cases func setupTestCase(t *testing.T) (func(t *testing.T), dbm.DB, *State) { - config := cfg.ResetTestRoot("state_") stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) state := GetState(stateDB, config.GenesisFile()) @@ -38,7 +38,8 @@ func TestStateCopy(t *testing.T) { stateCopy := state.Copy() - assert.True(state.Equals(stateCopy), cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) + assert.True(state.Equals(stateCopy), + cmn.Fmt("exppppppected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) stateCopy.LastBlockHeight++ assert.False(state.Equals(stateCopy), cmn.Fmt("expected states to be different. got same %v", state)) } @@ -51,7 +52,8 @@ func TestStateSaveLoad(t *testing.T) { state.Save() loadedState := LoadState(stateDB) - assert.True(state.Equals(loadedState), cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", loadedState, state)) + assert.True(state.Equals(loadedState), + cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", loadedState, state)) } func TestABCIResponsesSaveLoad(t *testing.T) { @@ -76,7 +78,8 @@ func TestABCIResponsesSaveLoad(t *testing.T) { state.SaveABCIResponses(abciResponses) abciResponses2 := state.LoadABCIResponses() - assert.Equal(abciResponses, abciResponses2, cmn.Fmt("ABCIResponses don't match: Got %v, Expected %v", abciResponses2, abciResponses)) + assert.Equal(abciResponses, abciResponses2, + cmn.Fmt("ABCIResponses don't match: Got %v, Expected %v", abciResponses2, abciResponses)) } func TestValidatorSimpleSaveLoad(t *testing.T) { @@ -170,7 +173,9 @@ func TestValidatorChangesSaveLoad(t *testing.T) { } } -func makeHeaderPartsResponses(state *State, height int, pubkey crypto.PubKey) (*types.Header, types.PartSetHeader, *ABCIResponses) { +func makeHeaderPartsResponses(state *State, height int, + pubkey crypto.PubKey) (*types.Header, types.PartSetHeader, *ABCIResponses) { + block := makeBlock(height, state) _, val := state.Validators.GetByIndex(0) abciResponses := &ABCIResponses{ From 870a98ccc3cd61b13d7c999d5bf6ac89755a113a Mon Sep 17 00:00:00 2001 From: Adrian Brink Date: Tue, 12 Sep 2017 17:12:19 +0200 Subject: [PATCH 3/4] Last fixes --- state/state_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/state/state_test.go b/state/state_test.go index a22e9958..4ff367fa 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -39,7 +39,7 @@ func TestStateCopy(t *testing.T) { stateCopy := state.Copy() assert.True(state.Equals(stateCopy), - cmn.Fmt("exppppppected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) + cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) stateCopy.LastBlockHeight++ assert.False(state.Equals(stateCopy), cmn.Fmt("expected states to be different. got same %v", state)) } @@ -47,6 +47,7 @@ func TestStateCopy(t *testing.T) { func TestStateSaveLoad(t *testing.T) { tearDown, stateDB, state := setupTestCase(t) defer tearDown(t) + assert := assert.New(t) state.LastBlockHeight++ state.Save() From bf576f0097b45d096f29b8de5f56e555993f11a9 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 12 Sep 2017 14:37:32 -0400 Subject: [PATCH 4/4] state: minor comment fixes --- state/state.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/state/state.go b/state/state.go index a9a2fee8..7e22e04d 100644 --- a/state/state.go +++ b/state/state.go @@ -42,14 +42,15 @@ type State struct { GenesisDoc *types.GenesisDoc ChainID string - // Updated at end of SetBlockAndValidators - // Genesis state has this set to 0. So, Block(H=0) does not exist + // These fields are updated by SetBlockAndValidators. + // LastBlockHeight=0 at genesis (ie. block(H=0) does not exist) + // LastValidators is used to validate block.LastCommit. LastBlockHeight int LastBlockID types.BlockID LastBlockTime time.Time Validators *types.ValidatorSet - // block.LastCommit validated against LastValidators - LastValidators *types.ValidatorSet + LastValidators *types.ValidatorSet + // AppHash is updated after Commit AppHash []byte