Skip to content

Commit

Permalink
blockcache: respond to PR review comments
Browse files Browse the repository at this point in the history
This commit embeds the mutex into the mockChainBackend struct,
unexports the getter by appending the prefix `get` to the method
name, and undoes the split of GetBlock into two methods, which
breaks the atomicity of the parent call.
  • Loading branch information
MStreet3 committed Oct 13, 2022
1 parent f35b937 commit 6ec1db1
Showing 1 changed file with 18 additions and 28 deletions.
46 changes: 18 additions & 28 deletions blockcache/blockcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,10 @@ func newMockChain() *mockChainBackend {
// to track the number of backend calls and returns the block found in the internal
// map of blocks.
func (m *mockChainBackend) GetBlock(blockHash *chainhash.Hash) (*wire.MsgBlock, error) {
m.incrementCallCount()
return m.getBlock(blockHash)
}

// ChainCallCount is a getter for the chainCallCount property.
func (m *mockChainBackend) ChainCallCount() int {
m.RLock()
defer m.RUnlock()

return m.chainCallCount
}

func (m *mockChainBackend) addBlock(block *wire.MsgBlock, nonce uint32) {
m.Lock()
defer m.Unlock()
block.Header.Nonce = nonce
hash := block.Header.BlockHash()
m.blocks[hash] = block
}

func (m *mockChainBackend) getBlock(blockHash *chainhash.Hash) (*wire.MsgBlock, error) {
m.RLock()
defer m.RUnlock()
m.chainCallCount++

block, ok := m.blocks[*blockHash]

Expand All @@ -63,11 +44,20 @@ func (m *mockChainBackend) getBlock(blockHash *chainhash.Hash) (*wire.MsgBlock,
return block, nil
}

func (m *mockChainBackend) incrementCallCount() {
// getChainCallCount is a getter for the chainCallCount property.
func (m *mockChainBackend) getChainCallCount() int {
m.RLock()
defer m.RUnlock()

return m.chainCallCount
}

func (m *mockChainBackend) addBlock(block *wire.MsgBlock, nonce uint32) {
m.Lock()
defer m.Unlock()

m.chainCallCount++
block.Header.Nonce = nonce
hash := block.Header.BlockHash()
m.blocks[hash] = block
}

func (m *mockChainBackend) resetChainCallCount() {
Expand Down Expand Up @@ -114,7 +104,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
_, err := bc.GetBlock(&blockhash1, getBlockImpl)
require.NoError(t, err)
require.Equal(t, 1, bc.Cache.Len())
require.Equal(t, 1, mc.ChainCallCount())
require.Equal(t, 1, mc.getChainCallCount())
mc.resetChainCallCount()

_, err = bc.Cache.Get(*inv1)
Expand All @@ -126,7 +116,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
_, err = bc.GetBlock(&blockhash2, getBlockImpl)
require.NoError(t, err)
require.Equal(t, 2, bc.Cache.Len())
require.Equal(t, 1, mc.ChainCallCount())
require.Equal(t, 1, mc.getChainCallCount())
mc.resetChainCallCount()

_, err = bc.Cache.Get(*inv1)
Expand All @@ -141,7 +131,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
_, err = bc.GetBlock(&blockhash1, getBlockImpl)
require.NoError(t, err)
require.Equal(t, 2, bc.Cache.Len())
require.Equal(t, 0, mc.ChainCallCount())
require.Equal(t, 0, mc.getChainCallCount())
mc.resetChainCallCount()

// Since the Cache is now at its max capacity, it is expected that when
Expand All @@ -152,7 +142,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
_, err = bc.GetBlock(&blockhash3, getBlockImpl)
require.NoError(t, err)
require.Equal(t, 2, bc.Cache.Len())
require.Equal(t, 1, mc.ChainCallCount())
require.Equal(t, 1, mc.getChainCallCount())
mc.resetChainCallCount()

_, err = bc.Cache.Get(*inv1)
Expand Down Expand Up @@ -207,5 +197,5 @@ func TestBlockCacheMutexes(t *testing.T) {
}

wg.Wait()
require.Equal(t, 2, mc.ChainCallCount())
require.Equal(t, 2, mc.getChainCallCount())
}

0 comments on commit 6ec1db1

Please sign in to comment.