Merge pull request #917 from dnephin/cleanup-test-configfile

Don't set a default filename for ConfigFile

cli/command/registry/login_test.go

@@ -5,14 +5,14 @@ import (
 	"fmt"
 	"testing"
 
-	"golang.org/x/net/context"
-
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/client"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
+	"github.com/gotestyourself/gotestyourself/fs"
+	"golang.org/x/net/context"
 )
 
 const userErr = "userunknownError"
@@ -131,22 +131,27 @@ func TestRunLogin(t *testing.T) {
 			expectedErr:     testAuthErrMsg,
 		},
 	}
-	for _, tc := range testCases {
-		cli := test.NewFakeCli(&fakeClient{})
-		errBuf := new(bytes.Buffer)
-		cli.SetErr(errBuf)
-		if tc.inputStoredCred != nil {
-			cred := *tc.inputStoredCred
-			cli.ConfigFile().GetCredentialsStore(cred.ServerAddress).Store(cred)
-		}
-		loginErr := runLogin(cli, tc.inputLoginOption)
-		if tc.expectedErr != "" {
-			assert.Check(t, is.Equal(tc.expectedErr, loginErr.Error()))
-		} else {
-			assert.Check(t, loginErr)
-			savedCred, credStoreErr := cli.ConfigFile().GetCredentialsStore(tc.inputStoredCred.ServerAddress).Get(tc.inputStoredCred.ServerAddress)
+	for i, tc := range testCases {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			tmpFile := fs.NewFile(t, "test-run-login")
+			defer tmpFile.Remove()
+			cli := test.NewFakeCli(&fakeClient{})
+			configfile := cli.ConfigFile()
+			configfile.Filename = tmpFile.Path()
+
+			if tc.inputStoredCred != nil {
+				cred := *tc.inputStoredCred
+				configfile.GetCredentialsStore(cred.ServerAddress).Store(cred)
+			}
+			loginErr := runLogin(cli, tc.inputLoginOption)
+			if tc.expectedErr != "" {
+				assert.Error(t, loginErr, tc.expectedErr)
+				return
+			}
+			assert.NilError(t, loginErr)
+			savedCred, credStoreErr := configfile.GetCredentialsStore(tc.inputStoredCred.ServerAddress).Get(tc.inputStoredCred.ServerAddress)
 			assert.Check(t, credStoreErr)
-			assert.Check(t, is.DeepEqual(tc.expectedSavedCred, savedCred))
-		}
+			assert.DeepEqual(t, tc.expectedSavedCred, savedCred)
+		})
 	}
 }

cli/config/config.go

@@ -75,18 +75,18 @@ func Load(configDir string) (*configfile.ConfigFile, error) {
 	if _, err := os.Stat(filename); err == nil {
 		file, err := os.Open(filename)
 		if err != nil {
-			return configFile, errors.Errorf("%s - %v", filename, err)
+			return configFile, errors.Wrap(err, filename)
 		}
 		defer file.Close()
 		err = configFile.LoadFromReader(file)
 		if err != nil {
-			err = errors.Errorf("%s - %v", filename, err)
+			err = errors.Wrap(err, filename)
 		}
 		return configFile, err
 	} else if !os.IsNotExist(err) {
 		// if file is there but we can't stat it for any reason other
 		// than it doesn't exist then stop
-		return configFile, errors.Errorf("%s - %v", filename, err)
+		return configFile, errors.Wrap(err, filename)
 	}
 
 	// Can't find latest config file so check for the old one
@@ -96,12 +96,12 @@ func Load(configDir string) (*configfile.ConfigFile, error) {
 	}
 	file, err := os.Open(confFile)
 	if err != nil {
-		return configFile, errors.Errorf("%s - %v", confFile, err)
+		return configFile, errors.Wrap(err, confFile)
 	}
 	defer file.Close()
 	err = configFile.LegacyLoadFromReader(file)
 	if err != nil {
-		return configFile, errors.Errorf("%s - %v", confFile, err)
+		return configFile, errors.Wrap(err, confFile)
 	}
 	return configFile, nil
 }

cli/config/config_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"bytes"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -10,10 +11,10 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/config/credentials"
-	"github.com/docker/cli/internal/test/testutil"
 	"github.com/docker/docker/pkg/homedir"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
+	"github.com/pkg/errors"
 )
 
 func setupConfigDir(t *testing.T) (string, func()) {
@@ -78,7 +79,8 @@ func TestEmptyFile(t *testing.T) {
 	assert.NilError(t, err)
 
 	_, err = Load(tmpHome)
-	testutil.ErrorContains(t, err, "EOF")
+	assert.Equal(t, errors.Cause(err), io.EOF)
+	assert.ErrorContains(t, err, ConfigFileName)
 }
 
 func TestEmptyJSON(t *testing.T) {
@@ -122,7 +124,7 @@ email`: "Invalid auth configuration file",
 		assert.NilError(t, err)
 
 		_, err = Load(tmpHome)
-		testutil.ErrorContains(t, err, expectedError)
+		assert.ErrorContains(t, err, expectedError)
 	}
 }
 
@@ -469,7 +471,7 @@ func TestJSONSaveWithNoFile(t *testing.T) {
 	config, err := LoadFromReader(strings.NewReader(js))
 	assert.NilError(t, err)
 	err = config.Save()
-	testutil.ErrorContains(t, err, "with empty filename")
+	assert.ErrorContains(t, err, "with empty filename")
 
 	tmpHome, err := ioutil.TempDir("", "config-test")
 	assert.NilError(t, err)
@@ -500,7 +502,7 @@ func TestLegacyJSONSaveWithNoFile(t *testing.T) {
 	config, err := LegacyLoadFromReader(strings.NewReader(js))
 	assert.NilError(t, err)
 	err = config.Save()
-	testutil.ErrorContains(t, err, "with empty filename")
+	assert.ErrorContains(t, err, "with empty filename")
 
 	tmpHome, err := ioutil.TempDir("", "config-test")
 	assert.NilError(t, err)

internal/test/cli.go

@@ -40,12 +40,14 @@ func NewFakeCli(client client.APIClient) *FakeCli {
 	outBuffer := new(bytes.Buffer)
 	errBuffer := new(bytes.Buffer)
 	return &FakeCli{
-		client:     client,
-		out:        command.NewOutStream(outBuffer),
-		outBuffer:  outBuffer,
-		err:        errBuffer,
-		in:         command.NewInStream(ioutil.NopCloser(strings.NewReader(""))),
-		configfile: configfile.New("configfile"),
+		client:    client,
+		out:       command.NewOutStream(outBuffer),
+		outBuffer: outBuffer,
+		err:       errBuffer,
+		in:        command.NewInStream(ioutil.NopCloser(strings.NewReader(""))),
+		// Use an empty string for filename so that tests don't create configfiles
+		// Set cli.ConfigFile().Filename to a tempfile to support Save.
+		configfile: configfile.New(""),
 	}
 }