From f66c5a33d0b3d5f8f5ffde1078e8c19b80343648 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 2 Apr 2025 14:08:01 +0200 Subject: [PATCH 1/3] cli/command: TestHooksEnabled: fix test when config file is present This test verifies the default behavior, but when running the test in an environment that already has a ~/.docker/config.json present, it may fail. This patch updates the test to configure the config-directory to point to an empty directory, making sure it's not affected by state. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 9a51a3fbcd..553bbbf3f6 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -302,6 +302,8 @@ func TestInitializeShouldAlwaysCreateTheContextStore(t *testing.T) { func TestHooksEnabled(t *testing.T) { t.Run("disabled by default", func(t *testing.T) { + // Make sure we don't depend on any existing ~/.docker/config.json + config.SetDir(t.TempDir()) cli, err := NewDockerCli() assert.NilError(t, err) From ce4b75227463f4b6ba43ddca15ed8aeab0726600 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 2 Apr 2025 14:14:54 +0200 Subject: [PATCH 2/3] cli/command: TestNewDockerCliAndOperators fix unhandled errors Assert that the write succeeded; also changing `Fprintf` to `Fprint`, because we were not using templating (we should check why no linter complained about this). Signed-off-by: Sebastiaan van Stijn --- cli/command/cli_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 553bbbf3f6..8e351cac11 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -280,12 +280,14 @@ func TestNewDockerCliAndOperators(t *testing.T) { assert.NilError(t, err) assert.Equal(t, string(inputStream), "input") // Check output stream - fmt.Fprintf(cli.Out(), "output") + _, err = fmt.Fprint(cli.Out(), "output") + assert.NilError(t, err) outputStream, err := io.ReadAll(outbuf) assert.NilError(t, err) assert.Equal(t, string(outputStream), "output") // Check error stream - fmt.Fprintf(cli.Err(), "error") + _, err = fmt.Fprint(cli.Err(), "error") + assert.NilError(t, err) errStream, err := io.ReadAll(errbuf) assert.NilError(t, err) assert.Equal(t, string(errStream), "error") From db44e59be76c55b2f234720880669ab471874603 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 1 Apr 2025 12:07:57 +0200 Subject: [PATCH 3/3] cli/command: use stdlib for temp-dirs gotest.tools' fs package only provides very minimal benefits here; use stdlib functions to make things slightly more transparent. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli_test.go | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 8e351cac11..ea67d40363 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/httptest" + "os" "path/filepath" "runtime" "strings" @@ -22,7 +23,6 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/client" "gotest.tools/v3/assert" - "gotest.tools/v3/fs" ) func TestNewAPIClientFromFlags(t *testing.T) { @@ -205,8 +205,8 @@ func TestInitializeFromClient(t *testing.T) { // Makes sure we don't hang forever on the initial connection. // https://github.com/docker/cli/issues/3652 func TestInitializeFromClientHangs(t *testing.T) { - dir := t.TempDir() - socket := filepath.Join(dir, "my.sock") + tmpDir := t.TempDir() + socket := filepath.Join(tmpDir, "my.sock") l, err := net.Listen("unix", socket) assert.NilError(t, err) @@ -317,12 +317,11 @@ func TestHooksEnabled(t *testing.T) { "features": { "hooks": "true" }}` - dir := fs.NewDir(t, "", fs.WithFile("config.json", configFile)) - defer dir.Remove() + config.SetDir(t.TempDir()) + err := os.WriteFile(filepath.Join(config.Dir(), "config.json"), []byte(configFile), 0o600) + assert.NilError(t, err) cli, err := NewDockerCli() assert.NilError(t, err) - config.SetDir(dir.Path()) - assert.Check(t, cli.HooksEnabled()) }) @@ -332,12 +331,11 @@ func TestHooksEnabled(t *testing.T) { "hooks": "true" }}` t.Setenv("DOCKER_CLI_HOOKS", "false") - dir := fs.NewDir(t, "", fs.WithFile("config.json", configFile)) - defer dir.Remove() + config.SetDir(t.TempDir()) + err := os.WriteFile(filepath.Join(config.Dir(), "config.json"), []byte(configFile), 0o600) + assert.NilError(t, err) cli, err := NewDockerCli() assert.NilError(t, err) - config.SetDir(dir.Path()) - assert.Check(t, !cli.HooksEnabled()) }) @@ -347,12 +345,11 @@ func TestHooksEnabled(t *testing.T) { "hooks": "true" }}` t.Setenv("DOCKER_CLI_HINTS", "false") - dir := fs.NewDir(t, "", fs.WithFile("config.json", configFile)) - defer dir.Remove() + config.SetDir(t.TempDir()) + err := os.WriteFile(filepath.Join(config.Dir(), "config.json"), []byte(configFile), 0o600) + assert.NilError(t, err) cli, err := NewDockerCli() assert.NilError(t, err) - config.SetDir(dir.Path()) - assert.Check(t, !cli.HooksEnabled()) }) }