go: store/datas/pull: pull_chunk_tracker.go: PR feedback.

Author: reltuk

go/store/datas/pull/pull_chunk_fetcher_test.go

@@ -190,15 +190,25 @@ func TestPop(t *testing.T) {
 		backing[i] = new(int)
 		*backing[i] = i
 	}
+	// s is pointers of [0, 1, 2, ..., 16]
 	s := backing[:]
+	assert.Len(t, s, 16)
+	// pop continuously and assert that we see
+	// the sequence 0, 1, 2, ..., 16 and each
+	// prior element at the end of the list is
+	// |nil|.
 	for i := range 16 {
-		assert.Len(t, s, 16-i)
 		var p *int
 		p, s = pop(s)
 		assert.Len(t, s, 16-i-1)
 		assert.Equal(t, i, *p)
+		// One off the end of the new |s| is now nil.
 		assert.Nil(t, backing[16-i-1], "i is %d", i)
 	}
+	// pop has a precondition of len > 0. This should panic.
+	assert.Panics(t, func() {
+		pop(backing[:0])
+	})
 }
 
 func TestAppendAbsent(t *testing.T) {

go/store/datas/pull/pull_chunk_tracker.go

@@ -186,11 +186,12 @@ func (t *PullChunkTracker) reqRespThread(ctx context.Context, initial hash.HashS
 				outstanding -= 1
 				if resp.err != nil {
 					err = errors.Join(err, resp.err)
+				} else if len(resp.hs) > 0 {
+					// Add all the resp.hs hashes, those which are not already present
+					// in dest, to our batches of absent hashes we will return through
+					// GetChunksToFetch.
+					absent = appendAbsent(absent, resp.hs, t.cfg.BatchSize)
 				}
-				// Add all the resp.hs hashes, those which are not already present
-				// in dest, to our batches of absent hashes we will return through
-				// GetChunksToFetch.
-				absent = appendAbsent(absent, resp.hs, t.cfg.BatchSize)
 			case thisHasManyReqCh <- hasManyReq:
 				// We delivered a hasMany request to a hasManyThread.
 				// Remove it here. We do not need to update |outstanding|, since
@@ -232,7 +233,7 @@ func (t *PullChunkTracker) reqRespThread(ctx context.Context, initial hash.HashS
 	return eg.Wait()
 }
 
-// Pop returns the first element of s and the remaining elements of
+// pop returns the first element of s and the remaining elements of
 // s. It copies any suffix to the front of |s| and nils the last
 // element of |s| so that memory doesn't leak through |s[1:]| retaining
 // s[0].