Improves test coverage

codex/stores/memorystore.nim

@@ -55,11 +55,7 @@ method getBlock*(self: MemoryStore, cid: Cid): Future[?!Block] {.async.} =
   if cid notin self.table:
     return failure (ref BlockNotFoundError)(msg: "Block not in memory store")
 
-  try:
   return success self.table[cid].value.val
-  except CatchableError as exc:
-    trace "Error getting block from memory store", cid, error = exc.msg
-    return failure exc
 
 method hasBlock*(self: MemoryStore, cid: Cid): Future[?!bool] {.async.} =
   trace "Checking MemoryStore for block presence", cid
@@ -76,6 +72,21 @@ func cids(self: MemoryStore): (iterator: Cid {.gcsafe.}) =
       yield it.value.key
       it = it.next
 
+proc isOfBlockType(cid: Cid, blockType: BlockType): ?!bool =
+  without isManifest =? cid.isManifest, err:
+    trace "Error checking if cid is a manifest", err = err.msg
+    return failure("Unable to determine if CID is a manifest")
+
+  case blockType:
+    of BlockType.Manifest:
+      return success(isManifest)
+    of BlockType.Block:
+      return success(not isManifest)
+    of BlockType.Both:
+      return success(true)
+
+  return failure("Unknown block type: " & $blockType)
+
 method listBlocks*(self: MemoryStore, blockType = BlockType.Manifest): Future[?!BlocksIter] {.async.} =
   var
     iter = BlocksIter()
@@ -97,26 +108,14 @@ method listBlocks*(self: MemoryStore, blockType = BlockType.Manifest): Future[?!
         iter.finished = true
         return Cid.none
 
-      without isManifest =? cid.isManifest, err:
+      without isCorrectBlockType =? isOfBlockType(cid, blockType), err:
-        trace "Error checking if cid is a manifest", err = err.msg
+        warn "Error checking if cid of blocktype", err = err.msg
         return Cid.none
 
-      case blockType:
+      if not isCorrectBlockType:
-      of BlockType.Manifest:
+        trace "Cid does not match blocktype, skipping", cid, blockType
-        if not isManifest:
-          trace "Cid is not manifest, skipping", cid
         continue
 
-        break
-      of BlockType.Block:
-        if isManifest:
-          trace "Cid is a manifest, skipping", cid
-          continue
-
-        break
-      of BlockType.Both:
-        break
-
       return cid.some
 
   iter.next = next
@@ -156,7 +155,7 @@ method delBlock*(self: MemoryStore, cid: Cid): Future[?!void] {.async.} =
     return success()
 
   if cid notin self.table:
-    return failure (ref BlockNotFoundError)(msg: "Block not in memory store")
+    return success()
 
   let nodeToRemove = self.table[cid]
 

tests/codex/stores/commonstoretests.nim

@@ -79,6 +79,11 @@ proc commonBlockStoreTests*(
 
       check not (await store.hasBlock(newBlock1.cid)).tryGet()
 
+    test "delBlock silently ignores missing block":
+      check:
+        not (await store.hasBlock(newBlock1.cid)).tryGet()
+        not (await store.delBlock(newBlock1.cid)).isErr
+
     test "listBlocks Blocks":
       let
         blocks = @[newBlock1, newBlock2, newBlock3]

tests/codex/stores/testcachestore.nim

@@ -39,6 +39,15 @@ suite "Cache Store":
       newBlock == received2
       backingStore.numberOfGetCalls == 1
 
+  test "getBlock should return empty block immediately":
+    let expectedEmptyBlock = newBlock.cid.emptyBlock
+
+    let received = (await store.getBlock(expectedEmptyBlock.cid)).tryGet()
+
+    check:
+      expectedEmptyBlock == received
+      backingStore.numberOfGetCalls == 0
+
 commonBlockStoreTests(
   "Cache", proc: BlockStore =
     BlockStore(CacheStore.new(MemoryStore.new())))

tests/codex/stores/testmemorystore.nim

@@ -13,16 +13,89 @@ import ../helpers
 import ./commonstoretests
 
 suite "MemoryStore":
-  test "Should store initial blocks":
   let
     capacity = 100
     blk = createTestBlock(10)
+    emptyBlk = blk.cid.emptyBlock
 
+  test "Should store initial blocks":
     let store = MemoryStore.new([blk], capacity)
 
-    let receivedBlk = await store.getBlock(blk.cid)
+    let receivedBlk = (await store.getBlock(blk.cid)).tryGet()
 
-    check receivedBlk.tryGet() == blk
+    check receivedBlk == blk
+
+  test "getBlock should return empty block":
+    let store = MemoryStore.new([], capacity)
+
+    let received = (await store.getBlock(emptyBlk.cid)).tryGet()
+
+    check:
+      emptyBlk == received
+
+  test "hasBlock should return true for empty block":
+    let store = MemoryStore.new([], capacity)
+
+    let hasBlock = (await store.hasBlock(emptyBlk.cid)).tryGet()
+
+    check hasBlock
+
+  test "getBlock should return failure when getting an unknown block":
+    let
+      store = MemoryStore.new([blk], capacity)
+      unknownBlock = createTestBlock(11)
+
+    let received = (await store.getBlock(unknownBlock.cid))
+
+    check received.isErr
+
+  test "putBlock should increase bytes used":
+    let store = MemoryStore.new([], capacity)
+
+    check:
+      store.capacity == capacity
+      store.bytesUsed == 0
+
+    (await store.putBlock(blk)).tryGet()
+
+    check:
+      store.capacity == capacity
+      store.bytesUsed == blk.data.len
+
+  test "putBlock should fail when memorystore is full":
+    let
+      largeBlk = createTestBlock(capacity)
+      store = MemoryStore.new([largeBlk], capacity)
+
+    check:
+      store.capacity == capacity
+      store.bytesUsed == capacity
+
+    let response  = await store.putBlock(blk)
+
+    check response.isErr
+
+  test "putBlock should ignore empty blocks":
+    let store = MemoryStore.new([], capacity)
+
+    (await store.putBlock(emptyBlk)).tryGet()
+    (await store.putBlock(emptyBlk)).tryGet()
+    (await store.putBlock(emptyBlk)).tryGet()
+
+    check:
+      store.capacity == capacity
+      store.bytesUsed == 0
+
+  test "delBlock should ignore empty blocks":
+    let store = MemoryStore.new([], capacity)
+
+    (await store.delBlock(emptyBlk.cid)).tryGet()
+    (await store.delBlock(emptyBlk.cid)).tryGet()
+    (await store.delBlock(emptyBlk.cid)).tryGet()
+
+    check:
+      store.capacity == capacity
+      store.bytesUsed == 0
 
 commonBlockStoreTests(
   "MemoryStore", proc: BlockStore =

tests/codex/teststores.nim

@@ -2,5 +2,6 @@ import ./stores/testcachestore
 import ./stores/testrepostore
 import ./stores/testmaintenance
 import ./stores/testmemorystore
+import ./stores/testkeyutils
 
 {.warning[UnusedImport]: off.}