Merge pull request #4488 from thaJeztah/registry_test_cleanups

cli/command/registry: cleanup "search" and "login" tests

cli/command/registry/formatter_search_test.go

@@ -2,12 +2,9 @@ package registry
 
 import (
 	"bytes"
-	"encoding/json"
-	"strings"
 	"testing"
 
 	"github.com/docker/cli/cli/command/formatter"
-	"github.com/docker/cli/internal/test"
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -15,50 +12,67 @@ import (
 )
 
 func TestSearchContext(t *testing.T) {
-	name := "nginx"
-	starCount := 5000
-
 	var ctx searchContext
 	cases := []struct {
 		searchCtx searchContext
 		expValue  string
 		call      func() string
 	}{
-		{searchContext{
-			s: registrytypes.SearchResult{Name: name},
-		}, name, ctx.Name},
-		{searchContext{
-			s: registrytypes.SearchResult{StarCount: starCount},
-		}, "5000", ctx.StarCount},
-		{searchContext{
-			s: registrytypes.SearchResult{IsOfficial: true},
-		}, "[OK]", ctx.IsOfficial},
-		{searchContext{
-			s: registrytypes.SearchResult{IsOfficial: false},
-		}, "", ctx.IsOfficial},
-		{searchContext{
-			s: registrytypes.SearchResult{IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
-		}, "[OK]", ctx.IsAutomated},
-		{searchContext{
-			s: registrytypes.SearchResult{},
-		}, "", ctx.IsAutomated},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{Name: "nginx"},
+			},
+			expValue: "nginx",
+			call:     ctx.Name,
+		},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{StarCount: 5000},
+			},
+			expValue: "5000",
+			call:     ctx.StarCount,
+		},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{IsOfficial: true},
+			},
+			expValue: "[OK]",
+			call:     ctx.IsOfficial,
+		},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{IsOfficial: false},
+			},
+			call: ctx.IsOfficial,
+		},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+			},
+			expValue: "[OK]",
+			call:     ctx.IsAutomated,
+		},
+		{
+			searchCtx: searchContext{
+				s: registrytypes.SearchResult{},
+			},
+			call: ctx.IsAutomated,
+		},
 	}
 
 	for _, c := range cases {
 		ctx = c.searchCtx
 		v := c.call()
-		if strings.Contains(v, ",") {
-			test.CompareMultipleValues(t, v, c.expValue)
-		} else if v != c.expValue {
-			t.Fatalf("Expected %s, was %s\n", c.expValue, v)
-		}
+		assert.Check(t, is.Equal(v, c.expValue))
 	}
 }
 
 func TestSearchContextDescription(t *testing.T) {
-	shortDescription := "Official build of Nginx."
-	longDescription := "Automated Nginx reverse proxy for docker containers"
-	descriptionWReturns := "Automated\nNginx reverse\rproxy\rfor docker\ncontainers"
+	const (
+		shortDescription    = "Official build of Nginx."
+		longDescription     = "Automated Nginx reverse proxy for docker containers"
+		descriptionWReturns = "Automated\nNginx reverse\rproxy\rfor docker\ncontainers"
+	)
 
 	var ctx searchContext
 	cases := []struct {
@@ -66,80 +80,118 @@ func TestSearchContextDescription(t *testing.T) {
 		expValue  string
 		call      func() string
 	}{
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: shortDescription},
-			trunc: true,
-		}, shortDescription, ctx.Description},
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: shortDescription},
-			trunc: false,
-		}, shortDescription, ctx.Description},
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: longDescription},
-			trunc: false,
-		}, longDescription, ctx.Description},
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: longDescription},
-			trunc: true,
-		}, formatter.Ellipsis(longDescription, 45), ctx.Description},
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: descriptionWReturns},
-			trunc: false,
-		}, longDescription, ctx.Description},
-		{searchContext{
-			s:     registrytypes.SearchResult{Description: descriptionWReturns},
-			trunc: true,
-		}, formatter.Ellipsis(longDescription, 45), ctx.Description},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: shortDescription},
+				trunc: true,
+			},
+			expValue: shortDescription,
+			call:     ctx.Description,
+		},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: shortDescription},
+				trunc: false,
+			},
+			expValue: shortDescription,
+			call:     ctx.Description,
+		},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: longDescription},
+				trunc: false,
+			},
+			expValue: longDescription,
+			call:     ctx.Description,
+		},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: longDescription},
+				trunc: true,
+			},
+			expValue: formatter.Ellipsis(longDescription, 45),
+			call:     ctx.Description,
+		},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: descriptionWReturns},
+				trunc: false,
+			},
+			expValue: longDescription,
+			call:     ctx.Description,
+		},
+		{
+			searchCtx: searchContext{
+				s:     registrytypes.SearchResult{Description: descriptionWReturns},
+				trunc: true,
+			},
+			expValue: formatter.Ellipsis(longDescription, 45),
+			call:     ctx.Description,
+		},
 	}
 
 	for _, c := range cases {
 		ctx = c.searchCtx
 		v := c.call()
-		if strings.Contains(v, ",") {
-			test.CompareMultipleValues(t, v, c.expValue)
-		} else if v != c.expValue {
-			t.Fatalf("Expected %s, was %s\n", c.expValue, v)
-		}
+		assert.Check(t, is.Equal(v, c.expValue))
 	}
 }
 
 func TestSearchContextWrite(t *testing.T) {
 	cases := []struct {
-		context  formatter.Context
-		expected string
+		doc         string
+		format      formatter.Format
+		expected    string
+		expectedErr string
 	}{
-		// Errors
 		{
-			formatter.Context{Format: "{{InvalidFunction}}"},
-			`template parsing error: template: :1: function "InvalidFunction" not defined`,
+			doc:         "Errors",
+			format:      "{{InvalidFunction}}",
+			expectedErr: `template parsing error: template: :1: function "InvalidFunction" not defined`,
 		},
 		{
-			formatter.Context{Format: "{{nil}}"},
-			`template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
-		},
-		// Table format
-		{
-			formatter.Context{Format: NewSearchFormat("table")},
-			string(golden.Get(t, "search-context-write-table.golden")),
+			doc:         "Nil format",
+			format:      "{{nil}}",
+			expectedErr: `template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
 		},
 		{
-			formatter.Context{Format: NewSearchFormat("table {{.Name}}")},
-			`NAME
+			doc:    "JSON format",
+			format: "{{json .}}",
+			expected: `{"Description":"Official build","IsAutomated":"false","IsOfficial":"true","Name":"result1","StarCount":"5000"}
+{"Description":"Not official","IsAutomated":"true","IsOfficial":"false","Name":"result2","StarCount":"5"}
+`,
+		},
+		{
+			doc:    "JSON format, single field",
+			format: "{{json .Name}}",
+			expected: `"result1"
+"result2"
+`,
+		},
+		{
+			doc:      "Table format",
+			format:   NewSearchFormat("table"),
+			expected: string(golden.Get(t, "search-context-write-table.golden")),
+		},
+		{
+			doc:    "Table format, single column",
+			format: NewSearchFormat("table {{.Name}}"),
+			expected: `NAME
 result1
 result2
 `,
 		},
-		// Custom Format
 		{
-			formatter.Context{Format: NewSearchFormat("{{.Name}}")},
-			`result1
+			doc:    "Custom format, single field",
+			format: NewSearchFormat("{{.Name}}"),
+			expected: `result1
 result2
 `,
 		},
-		// Custom Format with CreatedAt
 		{
-			formatter.Context{Format: NewSearchFormat("{{.Name}} {{.StarCount}}")},
-			`result1 5000
+			doc:    "Custom Format, two columns",
+			format: NewSearchFormat("{{.Name}} {{.StarCount}}"),
+			expected: `result1 5000
 result2 5
 `,
 		},
@@ -152,61 +204,15 @@ result2 5
 
 	for _, tc := range cases {
 		tc := tc
-		t.Run(string(tc.context.Format), func(t *testing.T) {
+		t.Run(tc.doc, func(t *testing.T) {
 			var out bytes.Buffer
-			tc.context.Output = &out
-
-			err := SearchWrite(tc.context, results)
-			if err != nil {
-				assert.Error(t, err, tc.expected)
+			err := SearchWrite(formatter.Context{Format: tc.format, Output: &out}, results)
+			if tc.expectedErr != "" {
+				assert.Check(t, is.Error(err, tc.expectedErr))
 			} else {
-				assert.Equal(t, out.String(), tc.expected)
+				assert.Check(t, err)
 			}
+			assert.Check(t, is.Equal(out.String(), tc.expected))
 		})
 	}
 }
-
-func TestSearchContextWriteJSON(t *testing.T) {
-	results := []registrytypes.SearchResult{
-		{Name: "result1", Description: "Official build", StarCount: 5000, IsOfficial: true},
-		{Name: "result2", Description: "Not official", StarCount: 5, IsOfficial: false, IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
-	}
-	expectedJSONs := []map[string]interface{}{
-		{"Name": "result1", "Description": "Official build", "StarCount": "5000", "IsOfficial": "true", "IsAutomated": "false"},
-		{"Name": "result2", "Description": "Not official", "StarCount": "5", "IsOfficial": "false", "IsAutomated": "true"},
-	}
-
-	out := bytes.NewBufferString("")
-	err := SearchWrite(formatter.Context{Format: "{{json .}}", Output: out}, results)
-	if err != nil {
-		t.Fatal(err)
-	}
-	for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") {
-		t.Logf("Output: line %d: %s", i, line)
-		var m map[string]interface{}
-		if err := json.Unmarshal([]byte(line), &m); err != nil {
-			t.Fatal(err)
-		}
-		assert.Check(t, is.DeepEqual(m, expectedJSONs[i]))
-	}
-}
-
-func TestSearchContextWriteJSONField(t *testing.T) {
-	results := []registrytypes.SearchResult{
-		{Name: "result1", Description: "Official build", StarCount: 5000, IsOfficial: true},
-		{Name: "result2", Description: "Not official", StarCount: 5, IsOfficial: false, IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
-	}
-	out := bytes.NewBufferString("")
-	err := SearchWrite(formatter.Context{Format: "{{json .Name}}", Output: out}, results)
-	if err != nil {
-		t.Fatal(err)
-	}
-	for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") {
-		t.Logf("Output: line %d: %s", i, line)
-		var s string
-		if err := json.Unmarshal([]byte(line), &s); err != nil {
-			t.Fatal(err)
-		}
-		assert.Check(t, is.Equal(s, results[i].Name))
-	}
-}

cli/command/registry/login_test.go

@@ -17,15 +17,8 @@ import (
 )
 
 const (
-	userErr        = "userunknownError"
-	testAuthErrMsg = "UNKNOWN_ERR"
-)
-
-var testAuthErrors = map[string]error{
-	userErr: fmt.Errorf(testAuthErrMsg),
-}
-
-var (
+	unknownUser     = "userunknownError"
+	errUnknownUser  = "UNKNOWN_ERR"
 	expiredPassword = "I_M_EXPIRED"
 	useToken        = "I_M_TOKEN"
 )
@@ -47,8 +40,10 @@ func (c fakeClient) RegistryLogin(_ context.Context, auth registrytypes.AuthConf
 			IdentityToken: auth.Password,
 		}, nil
 	}
-	err := testAuthErrors[auth.Username]
-	return registrytypes.AuthenticateOKBody{}, err
+	if auth.Username == unknownUser {
+		return registrytypes.AuthenticateOKBody{}, fmt.Errorf(errUnknownUser)
+	}
+	return registrytypes.AuthenticateOKBody{}, nil
 }
 
 func TestLoginWithCredStoreCreds(t *testing.T) {
@@ -63,10 +58,10 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
 		},
 		{
 			inputAuthConfig: registrytypes.AuthConfig{
-				Username: userErr,
+				Username: unknownUser,
 			},
 			expectedMsg: "Authenticating with existing credentials...\n",
-			expectedErr: fmt.Sprintf("Login did not succeed, error: %s\n", testAuthErrMsg),
+			expectedErr: fmt.Sprintf("Login did not succeed, error: %s\n", errUnknownUser),
 		},
 	}
 	ctx := context.Background()
@@ -83,10 +78,12 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
 }
 
 func TestRunLogin(t *testing.T) {
-	const storedServerAddress = "reg1"
-	const validUsername = "u1"
-	const validPassword = "p1"
-	const validPassword2 = "p2"
+	const (
+		storedServerAddress = "reg1"
+		validUsername       = "u1"
+		validPassword       = "p1"
+		validPassword2      = "p2"
+	)
 
 	validAuthConfig := configtypes.AuthConfig{
 		ServerAddress: storedServerAddress,
@@ -104,20 +101,22 @@ func TestRunLogin(t *testing.T) {
 		IdentityToken: useToken,
 	}
 	testCases := []struct {
+		doc               string
 		inputLoginOption  loginOptions
 		inputStoredCred   *configtypes.AuthConfig
 		expectedErr       string
 		expectedSavedCred configtypes.AuthConfig
 	}{
 		{
+			doc: "valid auth from store",
 			inputLoginOption: loginOptions{
 				serverAddress: storedServerAddress,
 			},
 			inputStoredCred:   &validAuthConfig,
-			expectedErr:       "",
 			expectedSavedCred: validAuthConfig,
 		},
 		{
+			doc: "expired auth",
 			inputLoginOption: loginOptions{
 				serverAddress: storedServerAddress,
 			},
@@ -125,13 +124,13 @@ func TestRunLogin(t *testing.T) {
 			expectedErr:     "Error: Cannot perform an interactive login from a non TTY device",
 		},
 		{
+			doc: "valid username and password",
 			inputLoginOption: loginOptions{
 				serverAddress: storedServerAddress,
 				user:          validUsername,
 				password:      validPassword2,
 			},
 			inputStoredCred: &validAuthConfig,
-			expectedErr:     "",
 			expectedSavedCred: configtypes.AuthConfig{
 				ServerAddress: storedServerAddress,
 				Username:      validUsername,
@@ -139,27 +138,29 @@ func TestRunLogin(t *testing.T) {
 			},
 		},
 		{
+			doc: "unknown user",
 			inputLoginOption: loginOptions{
 				serverAddress: storedServerAddress,
-				user:          userErr,
+				user:          unknownUser,
 				password:      validPassword,
 			},
 			inputStoredCred: &validAuthConfig,
-			expectedErr:     testAuthErrMsg,
+			expectedErr:     errUnknownUser,
 		},
 		{
+			doc: "valid token",
 			inputLoginOption: loginOptions{
 				serverAddress: storedServerAddress,
 				user:          validUsername,
 				password:      useToken,
 			},
 			inputStoredCred:   &validIdentityToken,
-			expectedErr:       "",
 			expectedSavedCred: validIdentityToken,
 		},
 	}
-	for i, tc := range testCases {
-		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
 			tmpFile := fs.NewFile(t, "test-run-login")
 			defer tmpFile.Remove()
 			cli := test.NewFakeCli(&fakeClient{})