diff --git a/client/client_test.go b/client/client_test.go index 14739f2ca9..4d91dddb1e 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -87,22 +87,21 @@ func TestNewClientWithOpsFromEnv(t *testing.T) { } defer env.PatchAll(t, nil)() - for _, c := range testcases { - env.PatchAll(t, c.envs) - apiclient, err := NewClientWithOpts(FromEnv) - if c.expectedError != "" { - assert.Check(t, is.Error(err, c.expectedError), c.doc) + for _, tc := range testcases { + env.PatchAll(t, tc.envs) + client, err := NewClientWithOpts(FromEnv) + if tc.expectedError != "" { + assert.Check(t, is.Error(err, tc.expectedError), tc.doc) } else { - assert.Check(t, err, c.doc) - version := apiclient.ClientVersion() - assert.Check(t, is.Equal(c.expectedVersion, version), c.doc) + assert.Check(t, err, tc.doc) + assert.Check(t, is.Equal(client.ClientVersion(), tc.expectedVersion), tc.doc) } - if c.envs["DOCKER_TLS_VERIFY"] != "" { + if tc.envs["DOCKER_TLS_VERIFY"] != "" { // pedantic checking that this is handled correctly - tr := apiclient.client.Transport.(*http.Transport) - assert.Assert(t, tr.TLSClientConfig != nil, c.doc) - assert.Check(t, is.Equal(tr.TLSClientConfig.InsecureSkipVerify, false), c.doc) + tr := client.client.Transport.(*http.Transport) + assert.Assert(t, tr.TLSClientConfig != nil, tc.doc) + assert.Check(t, is.Equal(tr.TLSClientConfig.InsecureSkipVerify, false), tc.doc) } } } @@ -127,10 +126,13 @@ func TestGetAPIPath(t *testing.T) { } ctx := context.TODO() - for _, testcase := range testcases { - c := Client{version: testcase.version, basePath: "/"} - actual := c.getAPIPath(ctx, testcase.path, testcase.query) - assert.Check(t, is.Equal(actual, testcase.expected)) + for _, tc := range testcases { + client := Client{ + version: tc.version, + basePath: "/", + } + actual := client.getAPIPath(ctx, tc.path, tc.query) + assert.Check(t, is.Equal(actual, tc.expected)) } } @@ -167,7 +169,7 @@ func TestParseHostURL(t *testing.T) { if testcase.expectedErr != "" { assert.Check(t, is.ErrorContains(err, testcase.expectedErr)) } - assert.Check(t, is.DeepEqual(testcase.expected, actual)) + assert.Check(t, is.DeepEqual(actual, testcase.expected)) } } @@ -183,41 +185,36 @@ func TestNewClientWithOpsFromEnvSetsDefaultVersion(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Check(t, is.Equal(client.version, api.DefaultVersion)) + assert.Check(t, is.Equal(client.ClientVersion(), api.DefaultVersion)) - expected := "1.22" - os.Setenv("DOCKER_API_VERSION", expected) + const expected = "1.22" + _ = os.Setenv("DOCKER_API_VERSION", expected) client, err = NewClientWithOpts(FromEnv) if err != nil { t.Fatal(err) } - assert.Check(t, is.Equal(expected, client.version)) + assert.Check(t, is.Equal(client.ClientVersion(), expected)) } -// TestNegotiateAPIVersionEmpty asserts that client.Client can -// negotiate a compatible APIVersion when omitted +// TestNegotiateAPIVersionEmpty asserts that client.Client version negotiation +// downgrades to the correct API version if the API's ping response does not +// return an API version. func TestNegotiateAPIVersionEmpty(t *testing.T) { defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": ""})() client, err := NewClientWithOpts(FromEnv) assert.NilError(t, err) - ping := types.Ping{ - APIVersion: "", - OSType: "linux", - Experimental: false, - } - // set our version to something new client.version = "1.25" // if no version from server, expect the earliest // version before APIVersion was implemented - expected := "1.24" + const expected = "1.24" // test downgrade - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) + client.NegotiateAPIVersionPing(types.Ping{}) + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersion asserts that client.Client can @@ -245,28 +242,22 @@ func TestNegotiateAPIVersion(t *testing.T) { expected = "1.20" client.version = expected client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) + assert.Check(t, is.Equal(client.version, expected)) } -// TestNegotiateAPIVersionOverride asserts that we honor -// the environment variable DOCKER_API_VERSION when negotiating versions +// TestNegotiateAPIVersionOverride asserts that we honor the DOCKER_API_VERSION +// environment variable when negotiating versions. func TestNegotiateAPVersionOverride(t *testing.T) { - expected := "9.99" + const expected = "9.99" defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": expected})() client, err := NewClientWithOpts(FromEnv) assert.NilError(t, err) - ping := types.Ping{ - APIVersion: "1.24", - OSType: "linux", - Experimental: false, - } - // test that we honored the env var - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) + client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.24"}) + assert.Equal(t, client.ClientVersion(), expected) } func TestNegotiateAPIVersionAutomatic(t *testing.T) { @@ -278,24 +269,28 @@ func TestNegotiateAPIVersionAutomatic(t *testing.T) { return resp, nil }) + ctx := context.Background() client, err := NewClientWithOpts( WithHTTPClient(httpClient), WithAPIVersionNegotiation(), ) assert.NilError(t, err) - ctx := context.Background() - assert.Equal(t, client.ClientVersion(), api.DefaultVersion) + // Client defaults to use api.DefaultVersion before version-negotiation. + expected := api.DefaultVersion + assert.Equal(t, client.ClientVersion(), expected) // First request should trigger negotiation pingVersion = "1.35" + expected = "1.35" _, _ = client.Info(ctx) - assert.Equal(t, client.ClientVersion(), "1.35") + assert.Equal(t, client.ClientVersion(), expected) // Once successfully negotiated, subsequent requests should not re-negotiate pingVersion = "1.25" + expected = "1.35" _, _ = client.Info(ctx) - assert.Equal(t, client.ClientVersion(), "1.35") + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersionWithEmptyVersion asserts that initializing a client @@ -304,18 +299,20 @@ func TestNegotiateAPIVersionWithEmptyVersion(t *testing.T) { client, err := NewClientWithOpts(WithVersion("")) assert.NilError(t, err) - client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.35"}) - assert.Equal(t, client.version, "1.35") + const expected = "1.35" + client.NegotiateAPIVersionPing(types.Ping{APIVersion: expected}) + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersionWithFixedVersion asserts that initializing a client -// with an fixed version disables API-version negotiation +// with a fixed version disables API-version negotiation func TestNegotiateAPIVersionWithFixedVersion(t *testing.T) { - client, err := NewClientWithOpts(WithVersion("1.35")) + const customVersion = "1.35" + client, err := NewClientWithOpts(WithVersion(customVersion)) assert.NilError(t, err) client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.31"}) - assert.Equal(t, client.version, "1.35") + assert.Equal(t, client.ClientVersion(), customVersion) } type roundTripFunc func(*http.Request) (*http.Response, error) @@ -362,13 +359,13 @@ func TestClientRedirect(t *testing.T) { req, err := http.NewRequest(tc.httpMethod, "/redirectme", nil) assert.Check(t, err) resp, err := client.Do(req) - assert.Check(t, is.Equal(tc.statusCode, resp.StatusCode)) + assert.Check(t, is.Equal(resp.StatusCode, tc.statusCode)) if tc.expectedErr == nil { assert.Check(t, is.Nil(err)) } else { urlError, ok := err.(*url.Error) assert.Assert(t, ok, "%T is not *url.Error", err) - assert.Check(t, is.Equal(*tc.expectedErr, *urlError)) + assert.Check(t, is.Equal(*urlError, *tc.expectedErr)) } } }