Simplify sortCachedPacksFirst test in internal/repository

The test now uses the fact that the sort is stable. It's not guaranteed
to be, but the test is cleaner and more exhaustive. sortCachedPacksFirst
no longer needs a return value.

internal/repository/repository.go

@@ -116,15 +116,14 @@ type haver interface {
 }
 
 // sortCachedPacksFirst moves all cached pack files to the front of blobs.
-// It overwrites blobs.
+func sortCachedPacksFirst(cache haver, blobs []restic.PackedBlob) {
-func sortCachedPacksFirst(cache haver, blobs []restic.PackedBlob) []restic.PackedBlob {
 	if cache == nil {
-		return blobs
+		return
 	}
 
 	// no need to sort a list with one element
 	if len(blobs) == 1 {
-		return blobs
+		return
 	}
 
 	cached := blobs[:0]
@@ -138,7 +137,7 @@ func sortCachedPacksFirst(cache haver, blobs []restic.PackedBlob) []restic.Packe
 		noncached = append(noncached, blob)
 	}
 
-	return append(cached, noncached...)
+	copy(blobs[len(cached):], noncached)
 }
 
 // LoadBlob loads a blob of type t from the repository.
@@ -154,7 +153,7 @@ func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic.
 	}
 
 	// try cached pack files first
-	blobs = sortCachedPacksFirst(r.Cache, blobs)
+	sortCachedPacksFirst(r.Cache, blobs)
 
 	var lastError error
 	for _, blob := range blobs {

internal/repository/repository_internal_test.go

@@ -2,52 +2,45 @@ package repository
 
 import (
 	"math/rand"
+	"sort"
 	"testing"
 
 	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
 )
 
-type mapcache map[restic.Handle]struct{}
+type mapcache map[restic.Handle]bool
 
-func (c mapcache) Has(h restic.Handle) bool {
+func (c mapcache) Has(h restic.Handle) bool { return c[h] }
-	_, ok := c[h]
-	return ok
-}
 
 func TestSortCachedPacksFirst(t *testing.T) {
 	var (
-		blobs   [100]restic.PackedBlob
+		blobs, sorted [100]restic.PackedBlob
-		blobset = make(map[restic.PackedBlob]struct{})
+
-		cache   = make(mapcache)
+		cache = make(mapcache)
-		r       = rand.New(rand.NewSource(1261))
+		r     = rand.New(rand.NewSource(1261))
 	)
 
 	for i := 0; i < len(blobs); i++ {
 		var id restic.ID
 		r.Read(id[:])
 		blobs[i] = restic.PackedBlob{PackID: id}
-		blobset[blobs[i]] = struct{}{}
 
 		if i%3 == 0 {
 			h := restic.Handle{Name: id.String(), Type: restic.DataFile}
-			cache[h] = struct{}{}
+			cache[h] = true
 		}
 	}
 
-	sorted := sortCachedPacksFirst(cache, blobs[:])
+	copy(sorted[:], blobs[:])
+	sort.SliceStable(sorted[:], func(i, j int) bool {
+		hi := restic.Handle{Type: restic.DataFile, Name: sorted[i].PackID.String()}
+		hj := restic.Handle{Type: restic.DataFile, Name: sorted[j].PackID.String()}
+		return cache.Has(hi) && !cache.Has(hj)
+	})
 
-	rtest.Equals(t, len(blobs), len(sorted))
+	sortCachedPacksFirst(cache, blobs[:])
-	for i := 0; i < len(blobs); i++ {
+	rtest.Equals(t, sorted, blobs)
-		h := restic.Handle{Type: restic.DataFile, Name: sorted[i].PackID.String()}
-		if i < len(cache) {
-			rtest.Assert(t, cache.Has(h), "non-cached blob at front of sorted output")
-		} else {
-			rtest.Assert(t, !cache.Has(h), "cached blob at end of sorted output")
-		}
-		_, ok := blobset[sorted[i]]
-		rtest.Assert(t, ok, "sortCachedPacksFirst changed blob id")
-	}
 }
 
 func BenchmarkSortCachedPacksFirst(b *testing.B) {
@@ -66,7 +59,7 @@ func BenchmarkSortCachedPacksFirst(b *testing.B) {
 
 		if i%3 == 0 {
 			h := restic.Handle{Name: id.String(), Type: restic.DataFile}
-			cache[h] = struct{}{}
+			cache[h] = true
 		}
 	}