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 GetBlock call.

Simplifies GetBlock docstring and removes docstring for the
getter.
  • Loading branch information
MStreet3 committed Oct 13, 2022
1 parent f35b937 commit 9e22ba6
Showing 1 changed file with 19 additions and 31 deletions.
50 changes: 19 additions & 31 deletions blockcache/blockcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,33 +26,13 @@ func newMockChain() *mockChainBackend {
}
}

// GetBlock is a mock block fetching implementation, it increments chainCallCount
// to track the number of backend calls and returns the block found in the internal
// map of blocks.
// GetBlock is a mock implementation of block fetching that tracks the number
// of backend calls and returns the block found for the given hash or an error.
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 +43,19 @@ func (m *mockChainBackend) getBlock(blockHash *chainhash.Hash) (*wire.MsgBlock,
return block, nil
}

func (m *mockChainBackend) incrementCallCount() {
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 +102,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 +114,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 +129,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 +140,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 +195,5 @@ func TestBlockCacheMutexes(t *testing.T) {
}

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

0 comments on commit 9e22ba6

Please sign in to comment.