ci: fix build cache, drop duplicated test run

.github/workflows/go-ci.yml

@@ -17,12 +17,34 @@ jobs:
         run: echo "go-version=$(grep 'golang' mise.toml | cut -d'"' -f2)" >> $GITHUB_OUTPUT
         shell: bash
       - name: Set up Go
-        uses: sourcegraph/actions/go-setup@main
+        uses: actions/setup-go@v7
         with:
           go-version: ${{ steps.mise.outputs.go-version }}
-      - run: |
-          go test -race -v ./...
-          go test -v ./...
+          # setup-go's built-in cache keys on go.sum alone, has no fallback
+          # when go.sum changes, and shares one key across every job on the
+          # same OS, so the first job to finish (even one that compiled
+          # nothing) decides what everyone else restores. See the explicit
+          # cache step below instead.
+          cache: false
+      - name: Find Go cache directories
+        id: go-cache
+        shell: bash
+        run: |
+          echo "build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT"
+          echo "modules=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT"
+      - name: Cache Go modules and build output
+        uses: actions/cache@v6
+        with:
+          path: |
+            ${{ steps.go-cache.outputs.modules }}
+            ${{ steps.go-cache.outputs.build }}
+          key: ${{ github.job }}-${{ runner.os }}-${{ runner.arch }}-go${{ steps.mise.outputs.go-version }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ github.job }}-${{ runner.os }}-${{ runner.arch }}-go${{ steps.mise.outputs.go-version }}-
+      # bash on every OS: the default pwsh on Windows does not fail the step
+      # when a command in a multi-line script exits non-zero.
+      - run: go test -race -v ./...
+        shell: bash
 
   # This job provides a stable name for branch protection rules
   # It will only pass if all Go CI matrix tests above pass

.github/workflows/go-lint.yml

@@ -14,7 +14,24 @@ jobs:
         id: mise
         run: echo "go-version=$(grep 'golang' mise.toml | cut -d'"' -f2)" >> $GITHUB_OUTPUT
       - name: Set up Go
-        uses: sourcegraph/actions/go-setup@main
+        uses: actions/setup-go@v7
         with:
           go-version: ${{ steps.mise.outputs.go-version }}
+          # See go-ci.yml for why the built-in cache is off.
+          cache: false
+      - name: Find Go cache directories
+        id: go-cache
+        run: |
+          echo "build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT"
+          echo "modules=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT"
+      - name: Cache Go modules, build output, and golangci-lint analysis
+        uses: actions/cache@v6
+        with:
+          path: |
+            ${{ steps.go-cache.outputs.modules }}
+            ${{ steps.go-cache.outputs.build }}
+            ~/.cache/golangci-lint
+          key: ${{ github.job }}-${{ runner.os }}-${{ runner.arch }}-go${{ steps.mise.outputs.go-version }}-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ github.job }}-${{ runner.os }}-${{ runner.arch }}-go${{ steps.mise.outputs.go-version }}-
       - run: ./dev/go-lint.sh

.github/workflows/goreleaser-check.yml

@@ -10,15 +10,8 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
-      - name: Get Go version from mise
-        id: mise
-        run: echo "go-version=$(grep 'golang' mise.toml | cut -d'"' -f2)" >> $GITHUB_OUTPUT
-      - name: Set up Go
-        uses: sourcegraph/actions/go-setup@main
-        with:
-          go-version: ${{ steps.mise.outputs.go-version }}
+      # `goreleaser check` only parses the config; it needs neither Go nor
+      # git history.
       - name: Check GoReleaser config
         uses: goreleaser/goreleaser-action@v5
         with:

internal/api/mock/api.go

@@ -34,8 +34,14 @@ func (m *Client) NewGzippedQuery(query string) api.Request {
 	return args.Get(0).(api.Request)
 }
 
+// RequestBody wraps the body handed to NewHTTPRequest. testify fmt-prints every
+// recorded argument, and printing a bare *io.PipeReader reflects into the
+// pipe's internals, racing with the goroutine writing to it. Behind a struct
+// field, fmt prints the pointer's address instead.
+type RequestBody struct{ io.Reader }
+
 func (m *Client) NewHTTPRequest(ctx context.Context, method, path string, body io.Reader) (*http.Request, error) {
-	args := m.Called(ctx, method, path, body)
+	args := m.Called(ctx, method, path, RequestBody{body})
 	var obj *http.Request
 	if args.Get(0) != nil {
 		obj = args.Get(0).(*http.Request)

internal/batches/service/remote.go

@@ -175,8 +175,15 @@ func (svc *Service) uploadFile(ctx context.Context, workingDir, filePath, batchS
 	pipeReader, pipeWriter := io.Pipe()
 	multipartWriter := multipart.NewWriter(pipeWriter)
 
+	request, err := svc.client.NewHTTPRequest(ctx, http.MethodPost, fmt.Sprintf(".api/files/batch-changes/%s", batchSpecID), pipeReader)
+	if err != nil {
+		return err
+	}
+	request.Header.Set("Content-Type", multipartWriter.FormDataContentType())
+
 	// Write in a separate goroutine to properly chunk the file content. Writing to the pipe lets us not have
-	// to put the whole file in memory.
+	// to put the whole file in memory. Started only once the request exists, so a NewHTTPRequest error
+	// doesn't leak a goroutine blocked on a pipe nobody reads.
 	go func() {
 		defer pipeWriter.Close()
 		defer multipartWriter.Close()
@@ -186,12 +193,6 @@ func (svc *Service) uploadFile(ctx context.Context, workingDir, filePath, batchS
 		}
 	}()
 
-	request, err := svc.client.NewHTTPRequest(ctx, http.MethodPost, fmt.Sprintf(".api/files/batch-changes/%s", batchSpecID), pipeReader)
-	if err != nil {
-		return err
-	}
-	request.Header.Set("Content-Type", multipartWriter.FormDataContentType())
-
 	resp, err := svc.client.Do(request)
 	if err != nil {
 		// Errors passed to pipeWriter.CloseWithError come through here.

internal/batches/service/remote_norace_test.go → internal/batches/service/remote_upload_test.go

@@ -1,5 +1,3 @@
-//go:build !race
-
 package service_test
 
 import (
@@ -48,7 +46,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Once()
@@ -90,7 +88,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Once()
@@ -132,7 +130,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Twice()
@@ -180,7 +178,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Twice()
@@ -229,7 +227,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Once()
@@ -281,7 +279,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Times(3)
@@ -362,7 +360,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Once()
@@ -400,7 +398,7 @@ func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) {
 				req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 				client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 					Run(func(args mock.Arguments) {
-						req.Body = args[3].(*io.PipeReader)
+						req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 					}).
 					Return(req, nil).
 					Once()

internal/batches/service/remote_windows_test.go

@@ -47,7 +47,7 @@ func TestService_UploadBatchSpecWorkspaceFiles_Windows_Path(t *testing.T) {
 	req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil)
 	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything).
 		Run(func(args mock.Arguments) {
-			req.Body = args[3].(*io.PipeReader)
+			req.Body = io.NopCloser(args[3].(mockclient.RequestBody))
 		}).
 		Return(req, nil).
 		Once()