Merge pull request #3074 from ipfs/feat/test-cover-blockstore

test: 81% coverage on blockstore

blocks/blockstore/arc_cache_test.go

@@ -1,14 +1,18 @@
 package blockstore
 
 import (
-	"github.com/ipfs/go-ipfs/blocks"
 	"testing"
 
+	"github.com/ipfs/go-ipfs/blocks"
+	"github.com/ipfs/go-ipfs/blocks/key"
+
 	ds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore"
 	syncds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/sync"
 	context "gx/ipfs/QmZy2y8t9zQH2a1b8q2ZSLKp17ATuJoCNxxyMFG5qFExpt/go-net/context"
 )
 
+var exampleBlock = blocks.NewBlock([]byte("foo"))
+
 func testArcCached(bs GCBlockstore, ctx context.Context) (*arccache, error) {
 	if ctx == nil {
 		ctx = context.TODO()
@@ -24,15 +28,29 @@ func testArcCached(bs GCBlockstore, ctx context.Context) (*arccache, error) {
 	}
 }
 
-func TestRemoveCacheEntryOnDelete(t *testing.T) {
-	b := blocks.NewBlock([]byte("foo"))
+func createStores(t *testing.T) (*arccache, *blockstore, *callbackDatastore) {
 	cd := &callbackDatastore{f: func() {}, ds: ds.NewMapDatastore()}
 	bs := NewBlockstore(syncds.MutexWrap(cd))
-	cachedbs, err := testArcCached(bs, nil)
+	arc, err := testArcCached(bs, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
-	cachedbs.Put(b)
+	return arc, bs, cd
+}
+
+func trap(message string, cd *callbackDatastore, t *testing.T) {
+	cd.SetFunc(func() {
+		t.Fatal(message)
+	})
+}
+func untrap(cd *callbackDatastore) {
+	cd.SetFunc(func() {})
+}
+
+func TestRemoveCacheEntryOnDelete(t *testing.T) {
+	arc, _, cd := createStores(t)
+
+	arc.Put(exampleBlock)
 
 	cd.Lock()
 	writeHitTheDatastore := false
@@ -42,26 +60,119 @@ func TestRemoveCacheEntryOnDelete(t *testing.T) {
 		writeHitTheDatastore = true
 	})
 
-	cachedbs.DeleteBlock(b.Key())
-	cachedbs.Put(b)
+	arc.DeleteBlock(exampleBlock.Key())
+	arc.Put(exampleBlock)
 	if !writeHitTheDatastore {
 		t.Fail()
 	}
 }
 
 func TestElideDuplicateWrite(t *testing.T) {
-	cd := &callbackDatastore{f: func() {}, ds: ds.NewMapDatastore()}
-	bs := NewBlockstore(syncds.MutexWrap(cd))
-	cachedbs, err := testArcCached(bs, nil)
+	arc, _, cd := createStores(t)
+
+	arc.Put(exampleBlock)
+	trap("write hit datastore", cd, t)
+	arc.Put(exampleBlock)
+}
+
+func TestHasRequestTriggersCache(t *testing.T) {
+	arc, _, cd := createStores(t)
+
+	arc.Has(exampleBlock.Key())
+	trap("has hit datastore", cd, t)
+	if has, err := arc.Has(exampleBlock.Key()); has || err != nil {
+		t.Fatal("has was true but there is no such block")
+	}
+
+	untrap(cd)
+	err := arc.Put(exampleBlock)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	b1 := blocks.NewBlock([]byte("foo"))
+	trap("has hit datastore", cd, t)
 
-	cachedbs.Put(b1)
-	cd.SetFunc(func() {
-		t.Fatal("write hit the datastore")
-	})
-	cachedbs.Put(b1)
+	if has, err := arc.Has(exampleBlock.Key()); !has || err != nil {
+		t.Fatal("has returned invalid result")
+	}
+}
+
+func TestGetFillsCache(t *testing.T) {
+	arc, _, cd := createStores(t)
+
+	if bl, err := arc.Get(exampleBlock.Key()); bl != nil || err == nil {
+		t.Fatal("block was found or there was no error")
+	}
+
+	trap("has hit datastore", cd, t)
+
+	if has, err := arc.Has(exampleBlock.Key()); has || err != nil {
+		t.Fatal("has was true but there is no such block")
+	}
+
+	untrap(cd)
+
+	if err := arc.Put(exampleBlock); err != nil {
+		t.Fatal(err)
+	}
+
+	trap("has hit datastore", cd, t)
+
+	if has, err := arc.Has(exampleBlock.Key()); !has || err != nil {
+		t.Fatal("has returned invalid result")
+	}
+}
+
+func TestGetAndDeleteFalseShortCircuit(t *testing.T) {
+	arc, _, cd := createStores(t)
+
+	arc.Has(exampleBlock.Key())
+
+	trap("get hit datastore", cd, t)
+
+	if bl, err := arc.Get(exampleBlock.Key()); bl != nil || err != ErrNotFound {
+		t.Fatal("get returned invalid result")
+	}
+
+	if arc.DeleteBlock(exampleBlock.Key()) != ErrNotFound {
+		t.Fatal("expected ErrNotFound error")
+	}
+}
+
+func TestArcCreationFailure(t *testing.T) {
+	if arc, err := arcCached(nil, -1); arc != nil || err == nil {
+		t.Fatal("expected error and no cache")
+	}
+}
+
+func TestInvalidKey(t *testing.T) {
+	arc, _, _ := createStores(t)
+
+	bl, err := arc.Get(key.Key(""))
+
+	if bl != nil {
+		t.Fatal("blocks should be nil")
+	}
+	if err == nil {
+		t.Fatal("expected error")
+	}
+}
+
+func TestHasAfterSucessfulGetIsCached(t *testing.T) {
+	arc, bs, cd := createStores(t)
+
+	bs.Put(exampleBlock)
+
+	arc.Get(exampleBlock.Key())
+
+	trap("has hit datastore", cd, t)
+	arc.Has(exampleBlock.Key())
+}
+
+func TestPutManyCaches(t *testing.T) {
+	arc, _, cd := createStores(t)
+	arc.PutMany([]blocks.Block{exampleBlock})
+
+	trap("has hit datastore", cd, t)
+	arc.Has(exampleBlock.Key())
 }

blocks/blockstore/blockstore_test.go

@@ -8,23 +8,23 @@ import (
 	ds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore"
 	dsq "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/query"
 	ds_sync "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/sync"
+	u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
 	context "gx/ipfs/QmZy2y8t9zQH2a1b8q2ZSLKp17ATuJoCNxxyMFG5qFExpt/go-net/context"
 
 	blocks "github.com/ipfs/go-ipfs/blocks"
 	key "github.com/ipfs/go-ipfs/blocks/key"
 )
 
-// TODO(brian): TestGetReturnsNil
-
 func TestGetWhenKeyNotPresent(t *testing.T) {
 	bs := NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore()))
-	_, err := bs.Get(key.Key("not present"))
+	bl, err := bs.Get(key.Key("not present"))
 
-	if err != nil {
-		t.Log("As expected, block is not present")
-		return
+	if bl != nil {
+		t.Error("nil block expected")
+	}
+	if err == nil {
+		t.Error("error expected, got nil")
 	}
-	t.Fail()
 }
 
 func TestGetWhenKeyIsEmptyString(t *testing.T) {
@@ -54,18 +54,29 @@ func TestPutThenGetBlock(t *testing.T) {
 }
 
 func TestRuntimeHashing(t *testing.T) {
+	orginalDebug := u.Debug
+	defer (func() {
+		u.Debug = orginalDebug
+	})()
+	u.Debug = false
+
 	bs := NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore()))
 	bl := blocks.NewBlock([]byte("some data"))
 	blBad, err := blocks.NewBlockWithHash([]byte("some other data"), bl.Key().ToMultihash())
 	if err != nil {
-		t.Fatal("Debug is enabled")
+		t.Fatal("debug is off, still got an error")
 	}
-
+	bl2 := blocks.NewBlock([]byte("some other data"))
 	bs.Put(blBad)
+	bs.Put(bl2)
 	bs.RuntimeHashing(true)
 
 	if _, err := bs.Get(bl.Key()); err != ErrHashMismatch {
-		t.Fatalf("Expected '%v' got '%v'\n", ErrHashMismatch, err)
+		t.Fatalf("expected '%v' got '%v'\n", ErrHashMismatch, err)
+	}
+
+	if b, err := bs.Get(bl2.Key()); err != nil || b.String() != bl2.String() {
+		t.Fatal("got wrong blocks")
 	}
 }
 

blocks/blockstore/bloom_cache_test.go

@@ -66,6 +66,31 @@ func TestHasIsBloomCached(t *testing.T) {
 	if float64(cacheFails)/float64(1000) > float64(0.05) {
 		t.Fatal("Bloom filter has cache miss rate of more than 5%")
 	}
+
+	cacheFails = 0
+	block := blocks.NewBlock([]byte("newBlock"))
+
+	cachedbs.PutMany([]blocks.Block{block})
+	if cacheFails != 2 {
+		t.Fatalf("expected two datastore hits: %d", cacheFails)
+	}
+	cachedbs.Put(block)
+	if cacheFails != 3 {
+		t.Fatalf("expected datastore hit: %d", cacheFails)
+	}
+
+	if has, err := cachedbs.Has(block.Key()); !has || err != nil {
+		t.Fatal("has gave wrong response")
+	}
+
+	bl, err := cachedbs.Get(block.Key())
+	if bl.String() != block.String() {
+		t.Fatal("block data doesn't match")
+	}
+
+	if err != nil {
+		t.Fatal("there should't be an error")
+	}
 }
 
 type callbackDatastore struct {

blocks/blockstore/caching_test.go

@@ -0,0 +1,35 @@
+package blockstore
+
+import "testing"
+
+func TestCachingOptsLessThanZero(t *testing.T) {
+	opts := DefaultCacheOpts()
+	opts.HasARCCacheSize = -1
+
+	if _, err := CachedBlockstore(nil, nil, opts); err == nil {
+		t.Error("wrong ARC setting was not detected")
+	}
+
+	opts = DefaultCacheOpts()
+	opts.HasBloomFilterSize = -1
+
+	if _, err := CachedBlockstore(nil, nil, opts); err == nil {
+		t.Error("negative bloom size was not detected")
+	}
+
+	opts = DefaultCacheOpts()
+	opts.HasBloomFilterHashes = -1
+
+	if _, err := CachedBlockstore(nil, nil, opts); err == nil {
+		t.Error("negative hashes setting was not detected")
+	}
+}
+
+func TestBloomHashesAtZero(t *testing.T) {
+	opts := DefaultCacheOpts()
+	opts.HasBloomFilterHashes = 0
+
+	if _, err := CachedBlockstore(nil, nil, opts); err == nil {
+		t.Error("zero hashes setting with positive size was not detected")
+	}
+}