From 0b2eaa7f7299333dc04d76f5ef0c20d0369727ba Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Thu, 22 Jul 2021 10:28:05 +0200
Subject: [PATCH] cli/command: don't use client.CustomHTTPHeaders(), and
 simplify asserts

It's the only use of this function, and it's better to check that
the client actually sends the header.

This also simplifies some asserts, and makes sure that "actual" and "expected"
are in the correct order.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/cli_test.go | 62 +++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go
index 370ecec803..d48470ecd8 100644
--- a/cli/command/cli_test.go
+++ b/cli/command/cli_test.go
@@ -6,8 +6,11 @@ import (
 	"crypto/x509"
 	"fmt"
 	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	cliconfig "github.com/docker/cli/cli/config"
@@ -18,7 +21,6 @@ import (
 	"github.com/docker/docker/client"
 	"github.com/pkg/errors"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/env"
 	"gotest.tools/v3/fs"
 )
@@ -29,42 +31,54 @@ func TestNewAPIClientFromFlags(t *testing.T) {
 		host = "npipe://./"
 	}
 	opts := &flags.CommonOptions{Hosts: []string{host}}
-	configFile := &configfile.ConfigFile{
-		HTTPHeaders: map[string]string{
-			"My-Header": "Custom-Value",
-		},
-	}
-	apiclient, err := NewAPIClientFromFlags(opts, configFile)
+	apiClient, err := NewAPIClientFromFlags(opts, &configfile.ConfigFile{})
 	assert.NilError(t, err)
-	assert.Check(t, is.Equal(host, apiclient.DaemonHost()))
-
-	expectedHeaders := map[string]string{
-		"My-Header":  "Custom-Value",
-		"User-Agent": UserAgent(),
-	}
-	assert.Check(t, is.DeepEqual(expectedHeaders, apiclient.(*client.Client).CustomHTTPHeaders()))
-	assert.Check(t, is.Equal(api.DefaultVersion, apiclient.ClientVersion()))
-	assert.DeepEqual(t, configFile.HTTPHeaders, map[string]string{"My-Header": "Custom-Value"})
+	assert.Equal(t, apiClient.DaemonHost(), host)
+	assert.Equal(t, apiClient.ClientVersion(), api.DefaultVersion)
 }
 
 func TestNewAPIClientFromFlagsForDefaultSchema(t *testing.T) {
 	host := ":2375"
 	opts := &flags.CommonOptions{Hosts: []string{host}}
+	apiClient, err := NewAPIClientFromFlags(opts, &configfile.ConfigFile{})
+	assert.NilError(t, err)
+	assert.Equal(t, apiClient.DaemonHost(), "tcp://localhost"+host)
+	assert.Equal(t, apiClient.ClientVersion(), api.DefaultVersion)
+}
+
+func TestNewAPIClientFromFlagsWithCustomHeaders(t *testing.T) {
+	var received map[string]string
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		received = map[string]string{
+			"My-Header":  r.Header.Get("My-Header"),
+			"User-Agent": r.Header.Get("User-Agent"),
+		}
+		_, _ = w.Write([]byte("OK"))
+	}))
+	defer ts.Close()
+	host := strings.Replace(ts.URL, "http://", "tcp://", 1)
+	opts := &flags.CommonOptions{Hosts: []string{host}}
 	configFile := &configfile.ConfigFile{
 		HTTPHeaders: map[string]string{
 			"My-Header": "Custom-Value",
 		},
 	}
-	apiclient, err := NewAPIClientFromFlags(opts, configFile)
+
+	apiClient, err := NewAPIClientFromFlags(opts, configFile)
 	assert.NilError(t, err)
-	assert.Check(t, is.Equal("tcp://localhost"+host, apiclient.DaemonHost()))
+	assert.Equal(t, apiClient.DaemonHost(), host)
+	assert.Equal(t, apiClient.ClientVersion(), api.DefaultVersion)
+
+	// verify User-Agent is not appended to the configfile. see https://github.com/docker/cli/pull/2756
+	assert.DeepEqual(t, configFile.HTTPHeaders, map[string]string{"My-Header": "Custom-Value"})
 
 	expectedHeaders := map[string]string{
 		"My-Header":  "Custom-Value",
 		"User-Agent": UserAgent(),
 	}
-	assert.Check(t, is.DeepEqual(expectedHeaders, apiclient.(*client.Client).CustomHTTPHeaders()))
-	assert.Check(t, is.Equal(api.DefaultVersion, apiclient.ClientVersion()))
+	_, err = apiClient.Ping(context.Background())
+	assert.NilError(t, err)
+	assert.DeepEqual(t, received, expectedHeaders)
 }
 
 func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) {
@@ -76,7 +90,7 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) {
 	configFile := &configfile.ConfigFile{}
 	apiclient, err := NewAPIClientFromFlags(opts, configFile)
 	assert.NilError(t, err)
-	assert.Check(t, is.Equal(customVersion, apiclient.ClientVersion()))
+	assert.Equal(t, apiclient.ClientVersion(), customVersion)
 }
 
 type fakeClient struct {
@@ -142,8 +156,8 @@ func TestInitializeFromClient(t *testing.T) {
 
 			cli := &DockerCli{client: apiclient}
 			cli.initializeFromClient()
-			assert.Check(t, is.DeepEqual(testcase.expectedServer, cli.serverInfo))
-			assert.Check(t, is.Equal(testcase.negotiated, apiclient.negotiated))
+			assert.DeepEqual(t, cli.serverInfo, testcase.expectedServer)
+			assert.Equal(t, apiclient.negotiated, testcase.negotiated)
 		})
 	}
 }
@@ -186,7 +200,7 @@ func TestExperimentalCLI(t *testing.T) {
 			err := cli.Initialize(flags.NewClientOptions())
 			assert.NilError(t, err)
 			// For backward-compatibility, HasExperimental will always be "true"
-			assert.Check(t, is.Equal(true, cli.ClientInfo().HasExperimental))
+			assert.Equal(t, cli.ClientInfo().HasExperimental, true)
 		})
 	}
 }