Make parallel downloads test more strict

We can assert now that only one thread downloaded the file, and all
other threads used the cache.

Signed-off-by: Nir Soffer <[email protected]>

Author: nirs

pkg/downloader/downloader_test.go

@@ -8,7 +8,6 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
-	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -31,11 +30,6 @@ type downloadResult struct {
 // races quicker. 20 parallel downloads take about 120 milliseconds on M1 Pro.
 const parallelDownloads = 20
 
-// When downloading in parallel usually all downloads completed with
-// StatusDownload, but some may be delayed and find the data file when they
-// start. Can be reproduced locally using 100 parallel downloads.
-var parallelStatus = []Status{StatusDownloaded, StatusUsedCache}
-
 func TestDownloadRemote(t *testing.T) {
 	ts := httptest.NewServer(http.FileServer(http.Dir("testdata")))
 	t.Cleanup(ts.Close)
@@ -103,15 +97,10 @@ func TestDownloadRemote(t *testing.T) {
 					results <- downloadResult{r, err}
 				}()
 			}
-			// We must process all results before cleanup.
-			for i := 0; i < parallelDownloads; i++ {
-				result := <-results
-				if result.err != nil {
-					t.Errorf("Download failed: %s", result.err)
-				} else if !slices.Contains(parallelStatus, result.r.Status) {
-					t.Errorf("Expected download status %s, got %s", parallelStatus, result.r.Status)
-				}
-			}
+			// Only one thread should download, the rest should use the cache.
+			downloaded, cached := countResults(t, results)
+			assert.Equal(t, downloaded, 1)
+			assert.Equal(t, cached, parallelDownloads-1)
 		})
 	})
 	t.Run("caching-only mode", func(t *testing.T) {
@@ -146,15 +135,10 @@ func TestDownloadRemote(t *testing.T) {
 					results <- downloadResult{r, err}
 				}()
 			}
-			// We must process all results before cleanup.
-			for i := 0; i < parallelDownloads; i++ {
-				result := <-results
-				if result.err != nil {
-					t.Errorf("Download failed: %s", result.err)
-				} else if !slices.Contains(parallelStatus, result.r.Status) {
-					t.Errorf("Expected download status %s, got %s", parallelStatus, result.r.Status)
-				}
-			}
+			// Only one thread should download, the rest should use the cache.
+			downloaded, cached := countResults(t, results)
+			assert.Equal(t, downloaded, 1)
+			assert.Equal(t, cached, parallelDownloads-1)
 		})
 	})
 	t.Run("cached", func(t *testing.T) {
@@ -188,6 +172,26 @@ func TestDownloadRemote(t *testing.T) {
 	})
 }
 
+func countResults(t *testing.T, results chan downloadResult) (downloaded int, cached int) {
+	t.Helper()
+	for i := 0; i < parallelDownloads; i++ {
+		result := <-results
+		if result.err != nil {
+			t.Errorf("Download failed: %s", result.err)
+		} else {
+			switch result.r.Status {
+			case StatusDownloaded:
+				downloaded++
+			case StatusUsedCache:
+				cached++
+			default:
+				t.Errorf("Unexpected download status %q", result.r.Status)
+			}
+		}
+	}
+	return downloaded, cached
+}
+
 func TestRedownloadRemote(t *testing.T) {
 	remoteDir := t.TempDir()
 	ts := httptest.NewServer(http.FileServer(http.Dir(remoteDir)))