fix: send package-search sorts the API accepts; download a small dataset in e2e

e2e/dataset_test.go

@@ -50,15 +50,21 @@ func TestDatasetStatusFirst(t *testing.T) {
 	}
 }
 
-// TestDatasetDownloadFirst lists datasets, takes the first Blob-type one
-// with an uploaded version, downloads it to a temp path, and asserts the file was written and is
+// downloadSizeCap bounds the dataset TestDatasetDownloadFirst will fetch.
+// Shared instances keep large blobs left over by other suites, and the first
+// one listed can take longer than the command timeout to download.
+const downloadSizeCap = 20 << 20 // 20 MiB
+
+// TestDatasetDownloadFirst lists datasets, takes the smallest Blob-type one
+// with an uploaded version and a listed size within downloadSizeCap,
+// downloads it to a temp path, and asserts the file was written and is
 // non-empty.
 func TestDatasetDownloadFirst(t *testing.T) {
 	requireCreds(t)
 	list := runOK(t, "dataset", "list").combined()
-	id := firstIDOfType(list, "Blob")
+	id := smallestIDOfType(list, "Blob", downloadSizeCap)
 	if id == "" {
-		t.Skip("no Blob-type datasets with an uploaded version on this instance to download")
+		t.Skipf("no Blob-type datasets with an uploaded version of at most %d bytes on this instance to download", downloadSizeCap)
 	}
 
 	dest := filepath.Join(t.TempDir(), "dataset.bin")

e2e/helpers_test.go

@@ -5,6 +5,7 @@ package e2e
 import (
 	"fmt"
 	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 )
@@ -84,20 +85,61 @@ func firstID(out string) string {
 // has no version to inspect or download, so status/download fail on it by
 // design and it cannot drive a happy-path test.
 func firstIDOfType(out, dtype string) string {
+	if entries := datasetEntriesOfType(out, dtype); len(entries) > 0 {
+		return entries[0].id
+	}
+	return ""
+}
+
+// smallestIDOfType is firstIDOfType restricted to entries whose listed size is
+// known and in (0, maxBytes], returning the smallest. Download tests use it so
+// they never pull whatever large blob another suite left behind on a shared
+// instance.
+func smallestIDOfType(out, dtype string, maxBytes int64) string {
+	best, bestSize := "", int64(-1)
+	for _, e := range datasetEntriesOfType(out, dtype) {
+		if e.size <= 0 || e.size > maxBytes {
+			continue
+		}
+		if bestSize < 0 || e.size < bestSize {
+			best, bestSize = e.id, e.size
+		}
+	}
+	return best
+}
+
+type datasetEntry struct {
+	id   string
+	size int64 // -1 when the entry has no "Size:" line
+}
+
+var reSizeLine = regexp.MustCompile(`(?m)^Size:\s*(\d+)\s*bytes\s*$`)
+
+// datasetEntriesOfType returns, in listing order, the `dataset list` entries of
+// type dtype with at least one uploaded version.
+func datasetEntriesOfType(out, dtype string) []datasetEntry {
 	typeRe := regexp.MustCompile(`(?m)^Type:\s*` + regexp.QuoteMeta(dtype) + `\s*$`)
 	verRe := regexp.MustCompile(`(?m)^Version:\s*v[1-9][0-9]*\s*$`)
+	var entries []datasetEntry
 	ids := reIDLine.FindAllStringSubmatchIndex(out, -1)
 	for i, loc := range ids {
 		end := len(out)
 		if i+1 < len(ids) {
 			end = ids[i+1][0]
 		}
 		entry := out[loc[0]:end]
-		if typeRe.MatchString(entry) && verRe.MatchString(entry) {
-			return out[loc[2]:loc[3]]
+		if !typeRe.MatchString(entry) || !verRe.MatchString(entry) {
+			continue
+		}
+		size := int64(-1)
+		if m := reSizeLine.FindStringSubmatch(entry); len(m) == 2 {
+			if n, err := strconv.ParseInt(m[1], 10, 64); err == nil {
+				size = n
+			}
 		}
+		entries = append(entries, datasetEntry{id: out[loc[2]:loc[3]], size: size})
 	}
-	return ""
+	return entries
 }
 
 // TestFirstIDOfType pins the listing-parse behaviour firstIDOfType relies on:
@@ -144,6 +186,52 @@ func TestFirstIDOfType(t *testing.T) {
 	}
 }
 
+// TestSmallestIDOfType pins the download tests' dataset choice: the smallest
+// uploaded Blob within the cap, never one over it, one of unknown size, or an
+// empty one. Pure output parsing.
+func TestSmallestIDOfType(t *testing.T) {
+	listing := "Found 5 dataset(s):\n" +
+		"\n" +
+		"ID: 1aaaaaaa-0000-0000-0000-000000000001\n" +
+		"Name: large_blob\n" +
+		"Size: 1073741824 bytes\n" +
+		"Type: Blob\n" +
+		"Version: v1\n" +
+		"\n" +
+		"ID: 1aaaaaaa-0000-0000-0000-000000000002\n" +
+		"Name: medium\n" +
+		"Size: 4096 bytes\n" +
+		"Type: Blob\n" +
+		"Version: v3\n" +
+		"\n" +
+		"ID: 1aaaaaaa-0000-0000-0000-000000000003\n" +
+		"Name: tiny_tree\n" +
+		"Size: 10 bytes\n" +
+		"Type: BlobTree\n" +
+		"Version: v1\n" +
+		"\n" +
+		"ID: 1aaaaaaa-0000-0000-0000-000000000004\n" +
+		"Name: small\n" +
+		"Size: 51 bytes\n" +
+		"Type: Blob\n" +
+		"Version: v1\n" +
+		"\n" +
+		"ID: 1aaaaaaa-0000-0000-0000-000000000005\n" +
+		"Name: no_size_line\n" +
+		"Type: Blob\n" +
+		"Version: v1\n"
+
+	if got := smallestIDOfType(listing, "Blob", 10<<20); got != "1aaaaaaa-0000-0000-0000-000000000004" {
+		t.Errorf("smallestIDOfType(Blob) = %q, want the 51-byte Blob", got)
+	}
+	if got := smallestIDOfType(listing, "Blob", 50); got != "" {
+		t.Errorf("smallestIDOfType(Blob, cap 50) = %q, want empty (every Blob is over the cap or unsized)", got)
+	}
+	if got := firstIDOfType(listing, "Blob"); got != "1aaaaaaa-0000-0000-0000-000000000001" {
+		t.Errorf("firstIDOfType(Blob) = %q, want the first Blob regardless of size", got)
+	}
+}
+
 // firstField returns the value of the first "<field>: <value>" line (case-insensitive).
 func firstField(out, field string) string {
 	re := regexp.MustCompile(`(?mi)^` + regexp.QuoteMeta(field) + `:\s*(.+)$`)

packages.go

@@ -341,14 +341,18 @@ func planRESTPages(limit, offset int) (pages []restPage, skip int) {
 // stars (the server's default). Name breaks ties: packages of one repository
 // share a score, and without a tiebreak their order changes between requests,
 // so consecutive pages could repeat or skip rows.
+//
+// Every sort key carries a direction sign: the API rejects unsigned keys
+// (`^[+-].+$`), and servers only honour the `-` form, so the tiebreak is
+// descending. Any fixed direction keeps the paging stable.
 func buildPackagesInfoQuery(search string, registryNames []string, page restPage) url.Values {
 	q := url.Values{}
 	q.Set("sorts[0]", "-stargazers_count")
 	if search != "" {
 		q.Set("name", search)
 		q.Set("sorts[0]", "-score")
 	}
-	q.Set("sorts[1]", "name")
+	q.Set("sorts[1]", "-name")
 	if len(registryNames) > 0 {
 		q.Set("registries", strings.Join(registryNames, ","))
 	}

packages_test.go

@@ -6,7 +6,9 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"reflect"
+	"regexp"
 	"strconv"
+	"strings"
 	"testing"
 )
 
@@ -129,15 +131,30 @@ func TestPlanRESTPages(t *testing.T) {
 
 func TestBuildPackagesInfoQuery(t *testing.T) {
 	q := buildPackagesInfoQuery("plots", []string{"General", "MyReg"}, restPage{Page: 2, PerPage: 25})
-	want := "name=plots&pagination%5Bpage%5D=2&pagination%5Bper_page%5D=25&pagination%5Btype%5D=offset&registries=General%2CMyReg&sorts%5B0%5D=-score&sorts%5B1%5D=name"
+	want := "name=plots&pagination%5Bpage%5D=2&pagination%5Bper_page%5D=25&pagination%5Btype%5D=offset&registries=General%2CMyReg&sorts%5B0%5D=-score&sorts%5B1%5D=-name"
 	if got := q.Encode(); got != want {
 		t.Errorf("query = %s\nwant    %s", got, want)
 	}
 	// Without a search term: most-starred first, still with the name tiebreak.
 	q = buildPackagesInfoQuery("", nil, restPage{1, 10})
-	if q.Has("name") || q.Get("sorts[0]") != "-stargazers_count" || q.Get("sorts[1]") != "name" {
+	if q.Has("name") || q.Get("sorts[0]") != "-stargazers_count" || q.Get("sorts[1]") != "-name" {
 		t.Errorf("unexpected params without a search term: %s", q.Encode())
 	}
+	// The API's Sorts schema requires a direction sign on every key.
+	signed := regexp.MustCompile(`^[+-].+$`)
+	for _, search := range []string{"plots", ""} {
+		q = buildPackagesInfoQuery(search, nil, restPage{1, 10})
+		for key, vals := range q {
+			if !strings.HasPrefix(key, "sorts[") {
+				continue
+			}
+			for _, v := range vals {
+				if !signed.MatchString(v) {
+					t.Errorf("search %q: %s=%q does not match the API's sort pattern ^[+-].+$", search, key, v)
+				}
+			}
+		}
+	}
 }
 
 func TestFetchRESTPackagesFromWindow(t *testing.T) {