Merge pull request #51276 from thaJeztah/tidy_client_utils

client: tidy-up test/mock-utilities

client/client.go

@@ -452,11 +452,3 @@ func (cli *Client) dialer() func(context.Context) (net.Conn, error) {
 		}
 	}
 }
-
-// transportFunc allows us to inject a mock transport for testing. We define it
-// here so we can detect the tlsconfig and return nil for only this type.
-type transportFunc func(*http.Request) (*http.Response, error)
-
-func (tf transportFunc) RoundTrip(req *http.Request) (*http.Response, error) {
-	return tf(req)
-}

client/client_mock_test.go

@@ -1,7 +1,6 @@
 package client
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -30,7 +29,9 @@ func assertRequest(req *http.Request, expMethod string, expectedPath string) err
 	return nil
 }
 
-func transportEnsureBody(f transportFunc) transportFunc {
+// ensureBody makes sure the response has a Body, using [http.NoBody] if
+// none is present, and returns it as a testRoundTripper.
+func ensureBody(f func(req *http.Request) (*http.Response, error)) testRoundTripper {
 	return func(req *http.Request) (*http.Response, error) {
 		resp, err := f(req)
 		if resp != nil && resp.Body == nil {
@@ -43,35 +44,46 @@ func transportEnsureBody(f transportFunc) transportFunc {
 // WithMockClient is a test helper that allows you to inject a mock client for testing.
 func WithMockClient(doer func(*http.Request) (*http.Response, error)) Opt {
 	return WithHTTPClient(&http.Client{
-		Transport: transportEnsureBody(transportFunc(doer)),
+		Transport: ensureBody(doer),
 	})
 }
 
 func errorMock(statusCode int, message string) func(req *http.Request) (*http.Response, error) {
+	return mockJSONResponse(statusCode, nil, common.ErrorResponse{
+		Message: message,
+	})
+}
+
+func mockJSONResponse[T any](statusCode int, headers http.Header, resp T) func(req *http.Request) (*http.Response, error) {
+	respBody, err := json.Marshal(&resp)
+	if err != nil {
+		panic(err)
+	}
+	hdr := make(http.Header)
+	if headers != nil {
+		hdr = headers.Clone()
+	}
+	hdr.Set("Content-Type", "application/json")
+	return mockResponse(statusCode, hdr, string(respBody))
+}
+
+func mockResponse(statusCode int, headers http.Header, respBody string) func(req *http.Request) (*http.Response, error) {
+	if headers == nil {
+		headers = make(http.Header)
+	}
+	var body io.ReadCloser
+	if respBody == "" {
+		body = http.NoBody
+	} else {
+		body = io.NopCloser(strings.NewReader(respBody))
+	}
 	return func(req *http.Request) (*http.Response, error) {
-		header := http.Header{}
-		header.Set("Content-Type", "application/json")
-
-		body, err := json.Marshal(&common.ErrorResponse{
-			Message: message,
-		})
-		if err != nil {
-			return nil, err
-		}
-
 		return &http.Response{
+			Status:     fmt.Sprintf("%d %s", statusCode, http.StatusText(statusCode)),
 			StatusCode: statusCode,
-			Body:       io.NopCloser(bytes.NewReader(body)),
-			Header:     header,
-		}, nil
-	}
-}
-
-func plainTextErrorMock(statusCode int, message string) func(req *http.Request) (*http.Response, error) {
-	return func(req *http.Request) (*http.Response, error) {
-		return &http.Response{
-			StatusCode: statusCode,
-			Body:       io.NopCloser(bytes.NewReader([]byte(message))),
+			Header:     headers,
+			Body:       body,
+			Request:    req,
 		}, nil
 	}
 }

client/client_test.go

@@ -1,14 +1,11 @@
 package client
 
 import (
-	"bytes"
 	"context"
 	"errors"
-	"io"
 	"net/http"
 	"net/url"
 	"runtime"
-	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -365,15 +362,11 @@ func TestNegotiateAPIVersionConnectionFailure(t *testing.T) {
 func TestNegotiateAPIVersionAutomatic(t *testing.T) {
 	var pingVersion string
 
-	ctx := context.Background()
+	ctx := t.Context()
 	client, err := NewClientWithOpts(
-		WithHTTPClient(&http.Client{
-			Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
-				resp := &http.Response{StatusCode: http.StatusOK, Header: http.Header{}}
-				resp.Header.Set("Api-Version", pingVersion)
-				resp.Body = io.NopCloser(strings.NewReader("OK"))
-				return resp, nil
-			}),
+		WithMockClient(func(req *http.Request) (*http.Response, error) {
+			hdr := http.Header{"Api-Version": []string{pingVersion}}
+			return mockResponse(http.StatusOK, hdr, "OK")(req)
 		}),
 		WithAPIVersionNegotiation(),
 	)
@@ -493,32 +486,14 @@ func TestCustomAPIVersion(t *testing.T) {
 	}
 }
 
-type roundTripFunc func(*http.Request) (*http.Response, error)
-
-func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
-	return rtf(req)
-}
-
-type bytesBufferClose struct {
-	*bytes.Buffer
-}
-
-func (bbc bytesBufferClose) Close() error {
-	return nil
-}
-
 func TestClientRedirect(t *testing.T) {
 	client := &http.Client{
 		CheckRedirect: CheckRedirect,
-		Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
+		Transport: ensureBody(func(req *http.Request) (*http.Response, error) {
 			if req.URL.String() == "/bla" {
-				return &http.Response{StatusCode: http.StatusNotFound}, nil
+				return mockResponse(http.StatusNotFound, nil, "")(req)
 			}
-			return &http.Response{
-				StatusCode: http.StatusMovedPermanently,
-				Header:     http.Header{"Location": {"/bla"}},
-				Body:       bytesBufferClose{bytes.NewBuffer(nil)},
-			}, nil
+			return mockResponse(http.StatusMovedPermanently, http.Header{"Location": {"/bla"}}, "")(req)
 		}),
 	}
 

client/options.go

@@ -108,15 +108,25 @@ func WithHost(host string) Opt {
 		if transport, ok := c.client.Transport.(*http.Transport); ok {
 			return sockets.ConfigureTransport(transport, c.proto, c.addr)
 		}
-		// For test transports (like transportFunc), we skip transport configuration
-		// but still set the host fields so that the client can use them for headers
-		if _, ok := c.client.Transport.(transportFunc); ok {
+		// For test transports, we skip transport configuration but still
+		// set the host fields so that the client can use them for headers
+		if _, ok := c.client.Transport.(testRoundTripper); ok {
 			return nil
 		}
 		return fmt.Errorf("cannot apply host to transport: %T", c.client.Transport)
 	}
 }
 
+// testRoundTripper allows us to inject a mock-transport for testing. We define it
+// here so we can detect the tlsconfig and return nil for only this type.
+type testRoundTripper func(*http.Request) (*http.Response, error)
+
+func (tf testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	return tf(req)
+}
+
+func (testRoundTripper) skipConfigureTransport() bool { return true }
+
 // WithHostFromEnv overrides the client host with the host specified in the
 // DOCKER_HOST ([EnvOverrideHost]) environment variable. If DOCKER_HOST is not set,
 // or set to an empty value, the host is not modified.

client/request_test.go

@@ -78,7 +78,7 @@ func TestSetHostHeader(t *testing.T) {
 // API versions < 1.24 returned plain text errors, but we may encounter
 // other situations where a non-JSON error is returned.
 func TestPlainTextError(t *testing.T) {
-	client, err := NewClientWithOpts(WithMockClient(plainTextErrorMock(http.StatusInternalServerError, "Server error")))
+	client, err := NewClientWithOpts(WithMockClient(mockResponse(http.StatusInternalServerError, nil, "Server error")))
 	assert.NilError(t, err)
 	_, err = client.ContainerList(context.Background(), ContainerListOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))

vendor/github.com/moby/moby/client/client.go

@@ -452,11 +452,3 @@ func (cli *Client) dialer() func(context.Context) (net.Conn, error) {
 		}
 	}
 }
-
-// transportFunc allows us to inject a mock transport for testing. We define it
-// here so we can detect the tlsconfig and return nil for only this type.
-type transportFunc func(*http.Request) (*http.Response, error)
-
-func (tf transportFunc) RoundTrip(req *http.Request) (*http.Response, error) {
-	return tf(req)
-}

vendor/github.com/moby/moby/client/options.go

@@ -108,15 +108,25 @@ func WithHost(host string) Opt {
 		if transport, ok := c.client.Transport.(*http.Transport); ok {
 			return sockets.ConfigureTransport(transport, c.proto, c.addr)
 		}
-		// For test transports (like transportFunc), we skip transport configuration
-		// but still set the host fields so that the client can use them for headers
-		if _, ok := c.client.Transport.(transportFunc); ok {
+		// For test transports, we skip transport configuration but still
+		// set the host fields so that the client can use them for headers
+		if _, ok := c.client.Transport.(testRoundTripper); ok {
 			return nil
 		}
 		return fmt.Errorf("cannot apply host to transport: %T", c.client.Transport)
 	}
 }
 
+// testRoundTripper allows us to inject a mock-transport for testing. We define it
+// here so we can detect the tlsconfig and return nil for only this type.
+type testRoundTripper func(*http.Request) (*http.Response, error)
+
+func (tf testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	return tf(req)
+}
+
+func (testRoundTripper) skipConfigureTransport() bool { return true }
+
 // WithHostFromEnv overrides the client host with the host specified in the
 // DOCKER_HOST ([EnvOverrideHost]) environment variable. If DOCKER_HOST is not set,
 // or set to an empty value, the host is not modified.