From 83bcfcb2ca3cbb17513b50619f5b3d1907982d71 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Sat, 29 Aug 2015 15:35:28 -0500 Subject: [PATCH] Improve mru inventory map and test cov to 100%. This commit improves the most-recently used inventory map human readable string to only show the inventory vectors. and adds tests for the entire structure to bring its coverage to 100%. In addition, removes the type assertion check in the Add function since the internal inventory list is only managed by the type itself and the tests would now catch any mistakes in the type of entries in the list. --- mruinvmap.go | 25 ++++++--- mruinvmap_test.go | 134 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 8 deletions(-) diff --git a/mruinvmap.go b/mruinvmap.go index dba371a5..f2e1f6ab 100644 --- a/mruinvmap.go +++ b/mruinvmap.go @@ -1,10 +1,11 @@ -// Copyright (c) 2013-2014 The btcsuite developers +// Copyright (c) 2013-2015 The btcsuite developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. package main import ( + "bytes" "container/list" "fmt" @@ -21,7 +22,19 @@ type MruInventoryMap struct { // String returns the map as a human-readable string. func (m MruInventoryMap) String() string { - return fmt.Sprintf("<%d>%v", m.limit, m.invMap) + lastEntryNum := len(m.invMap) - 1 + curEntry := 0 + buf := bytes.NewBufferString("[") + for iv := range m.invMap { + buf.WriteString(fmt.Sprintf("%v", iv)) + if curEntry < lastEntryNum { + buf.WriteString(", ") + } + curEntry++ + } + buf.WriteString("]") + + return fmt.Sprintf("<%d>%s", m.limit, buf.String()) } // Exists returns whether or not the passed inventory item is in the map. @@ -33,7 +46,8 @@ func (m *MruInventoryMap) Exists(iv *wire.InvVect) bool { } // Add adds the passed inventory to the map and handles eviction of the oldest -// item if adding the new item would exceed the max limit. +// item if adding the new item would exceed the max limit. Adding an existing +// item makes it the most recently used item. func (m *MruInventoryMap) Add(iv *wire.InvVect) { // When the limit is zero, nothing can be added to the map, so just // return. @@ -53,10 +67,7 @@ func (m *MruInventoryMap) Add(iv *wire.InvVect) { // node so a new one doesn't have to be allocated. if uint(len(m.invMap))+1 > m.limit { node := m.invList.Back() - lru, ok := node.Value.(*wire.InvVect) - if !ok { - return - } + lru := node.Value.(*wire.InvVect) // Evict least recently used item. delete(m.invMap, *lru) diff --git a/mruinvmap_test.go b/mruinvmap_test.go index f9a19907..10a2d9ee 100644 --- a/mruinvmap_test.go +++ b/mruinvmap_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2013-2014 The btcsuite developers +// Copyright (c) 2013-2015 The btcsuite developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -6,11 +6,143 @@ package main import ( "crypto/rand" + "fmt" "testing" "github.com/btcsuite/btcd/wire" ) +// TestMruInventoryMap ensures the MruInventoryMap behaves as expected including +// limiting, eviction of least-recently used entries, specific entry removal, +// and existence tests. +func TestMruInventoryMap(t *testing.T) { + // Create a bunch of fake inventory vectors to use in testing the mru + // inventory code. + numInvVects := 10 + invVects := make([]*wire.InvVect, 0, numInvVects) + for i := 0; i < numInvVects; i++ { + hash := &wire.ShaHash{byte(i)} + iv := wire.NewInvVect(wire.InvTypeBlock, hash) + invVects = append(invVects, iv) + } + + tests := []struct { + name string + limit int + }{ + {name: "limit 0", limit: 0}, + {name: "limit 1", limit: 1}, + {name: "limit 5", limit: 5}, + {name: "limit 7", limit: 7}, + {name: "limit one less than available", limit: numInvVects - 1}, + {name: "limit all available", limit: numInvVects}, + } + +testLoop: + for i, test := range tests { + // Create a new mru inventory map limited by the specified test + // limit and add all of the test inventory vectors. This will + // cause evicition since there are more test inventory vectors + // than the limits. + mruInvMap := NewMruInventoryMap(uint(test.limit)) + for j := 0; j < numInvVects; j++ { + mruInvMap.Add(invVects[j]) + } + + // Ensure the limited number of most recent entries in the + // inventory vector list exist. + for j := numInvVects - 1; j >= numInvVects-test.limit; j-- { + if !mruInvMap.Exists(invVects[j]) { + t.Errorf("Exists #%d (%s) entry %s does not "+ + "exist", i, test.name, *invVects[j]) + continue testLoop + } + } + + // Ensure the entries before the limited number of most recent + // entries in the inventory vector list do not exist. + for j := numInvVects - test.limit - 1; j >= 0; j-- { + if mruInvMap.Exists(invVects[j]) { + t.Errorf("Exists #%d (%s) entry %s exists", i, + test.name, *invVects[j]) + continue testLoop + } + } + + // Readd the entry that should currently be the least-recently + // used entry so it becomes the most-recently used entry, then + // force an eviction by adding an entry that doesn't exist and + // ensure the evicted entry is the new least-recently used + // entry. + // + // This check needs at least 2 entries. + if test.limit > 1 { + origLruIndex := numInvVects - test.limit + mruInvMap.Add(invVects[origLruIndex]) + + iv := wire.NewInvVect(wire.InvTypeBlock, + &wire.ShaHash{0x00, 0x01}) + mruInvMap.Add(iv) + + // Ensure the original lru entry still exists since it + // was updated and should've have become the mru entry. + if !mruInvMap.Exists(invVects[origLruIndex]) { + t.Errorf("MRU #%d (%s) entry %s does not exist", + i, test.name, *invVects[origLruIndex]) + continue testLoop + } + + // Ensure the entry that should've become the new lru + // entry was evicted. + newLruIndex := origLruIndex + 1 + if mruInvMap.Exists(invVects[newLruIndex]) { + t.Errorf("MRU #%d (%s) entry %s exists", i, + test.name, *invVects[newLruIndex]) + continue testLoop + } + } + + // Delete all of the entries in the inventory vector list, + // including those that don't exist in the map, and ensure they + // no longer exist. + for j := 0; j < numInvVects; j++ { + mruInvMap.Delete(invVects[j]) + if mruInvMap.Exists(invVects[j]) { + t.Errorf("Delete #%d (%s) entry %s exists", i, + test.name, *invVects[j]) + continue testLoop + } + } + } +} + +// TestMruInventoryMapStringer tests the stringized output for the +// MruInventoryMap type. +func TestMruInventoryMapStringer(t *testing.T) { + // Create a couple of fake inventory vectors to use in testing the mru + // inventory stringer code. + hash1 := &wire.ShaHash{0x01} + hash2 := &wire.ShaHash{0x02} + iv1 := wire.NewInvVect(wire.InvTypeBlock, hash1) + iv2 := wire.NewInvVect(wire.InvTypeBlock, hash2) + + // Create new mru inventory map and add the inventory vectors. + mruInvMap := NewMruInventoryMap(uint(2)) + mruInvMap.Add(iv1) + mruInvMap.Add(iv2) + + // Ensure the stringer gives the expected result. Since map iteration + // is not ordered, either entry could be first, so account for both + // cases. + wantStr1 := fmt.Sprintf("<%d>[%s, %s]", 2, *iv1, *iv2) + wantStr2 := fmt.Sprintf("<%d>[%s, %s]", 2, *iv2, *iv1) + gotStr := mruInvMap.String() + if gotStr != wantStr1 && gotStr != wantStr2 { + t.Fatalf("unexpected string representation - got %q, want %q "+ + "or %q", gotStr, wantStr1, wantStr2) + } +} + // BenchmarkMruInventoryList performs basic benchmarks on the most recently // used inventory handling. func BenchmarkMruInventoryList(b *testing.B) {