From a315437e1c3a03a404fceb18323e2d03d412001c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 23 Oct 2025 11:25:28 +0200 Subject: [PATCH 1/3] client: rename transportFunc -> testRoundTripper Rename it so that it's clearer that it's intended for test-purposes, and adding a `skipConfigureTransport()` method to the signature to prevent IDEs considering is a redundant convert, and to be more explicit on intent. Signed-off-by: Sebastiaan van Stijn --- client/client.go | 8 -------- client/client_mock_test.go | 6 ++++-- client/options.go | 16 +++++++++++++--- vendor/github.com/moby/moby/client/client.go | 8 -------- vendor/github.com/moby/moby/client/options.go | 16 +++++++++++++--- 5 files changed, 30 insertions(+), 24 deletions(-) diff --git a/client/client.go b/client/client.go index 8af2e0d242..02bfa87471 100644 --- a/client/client.go +++ b/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) -} diff --git a/client/client_mock_test.go b/client/client_mock_test.go index 6cbc9dc168..6f27db6f6e 100644 --- a/client/client_mock_test.go +++ b/client/client_mock_test.go @@ -30,7 +30,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,7 +45,7 @@ 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), }) } diff --git a/client/options.go b/client/options.go index bdbcd01435..1b1e92bbba 100644 --- a/client/options.go +++ b/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. diff --git a/vendor/github.com/moby/moby/client/client.go b/vendor/github.com/moby/moby/client/client.go index 8af2e0d242..02bfa87471 100644 --- a/vendor/github.com/moby/moby/client/client.go +++ b/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) -} diff --git a/vendor/github.com/moby/moby/client/options.go b/vendor/github.com/moby/moby/client/options.go index bdbcd01435..1b1e92bbba 100644 --- a/vendor/github.com/moby/moby/client/options.go +++ b/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. From b9dd7c0d59e47a8b103a68e1e6e173e009a060df Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 23 Oct 2025 11:58:55 +0200 Subject: [PATCH 2/3] client: tidy-up mock-utilities - Add a `mockResponse` utility, and slightly enhance it to also include the request Headers and Status message, to be more closely to actual responses. - Add a `mockJSONResponse` utility, implemented using `mockResponse` - Remove `plainTextErrorMock` in favor of `mockResponse` Signed-off-by: Sebastiaan van Stijn --- client/client_mock_test.go | 54 ++++++++++++++++++++++---------------- client/request_test.go | 2 +- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/client/client_mock_test.go b/client/client_mock_test.go index 6f27db6f6e..552fe92a0d 100644 --- a/client/client_mock_test.go +++ b/client/client_mock_test.go @@ -1,7 +1,6 @@ package client import ( - "bytes" "encoding/json" "fmt" "io" @@ -50,30 +49,41 @@ func WithMockClient(doer func(*http.Request) (*http.Response, error)) Opt { } 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 } } diff --git a/client/request_test.go b/client/request_test.go index 9b3b30e56b..bc9b989041 100644 --- a/client/request_test.go +++ b/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)) From 0fe6be8c3852df4b0cce162ddf993f95a21c2a2e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 23 Oct 2025 12:03:29 +0200 Subject: [PATCH 3/3] client: remove roundTripFunc, bytesBufferClose - Use the `mockResponse` instead - `bytesBufferClose` was our own implementation of `io.NopCloser` Signed-off-by: Sebastiaan van Stijn --- client/client_test.go | 39 +++++++-------------------------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 20a8ba7404..2d37c51845 100644 --- a/client/client_test.go +++ b/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) }), }