From 60fe868de80fcfb28dfb964ce1b4d5248700334f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Verg=C3=A9s?= Date: Sun, 1 Mar 2026 11:13:12 +0100 Subject: [PATCH 1/4] Show command on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Xavier Vergés --- utils/utils.go | 47 +++++++++++++++------------------------------ utils/utils_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 343f6f8a0..435f8d6a4 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -192,40 +192,25 @@ func TruncateString(str string, maxLength int) string { func RunCmdAndWait(name string, args ...string) (string, error) { cmd := exec.Command(name, args...) - - stdout, err := cmd.StdoutPipe() - if err != nil { - return "", err - } - stderr, err := cmd.StderrPipe() - if err != nil { - return "", err - } - - err = cmd.Start() + resp, err := cmd.Output() if err != nil { - return "", err - } - - resp, err := io.ReadAll(stdout) - if err != nil { - return "", err - } - errB, err := io.ReadAll(stderr) - if err != nil { - //nolint - return "", nil - } - - err = cmd.Wait() - if err != nil { - // in case of error, capture the exact message. - if len(errB) > 0 { - return "", errors.New(string(errB)) + cmdStr := name + " " + strings.Join(args, " ") + output := "" + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + output = strings.TrimSpace(string(exitErr.Stderr)) } - return "", err + if output == "" { + output = strings.TrimSpace(string(resp)) + } + if output != "" { + if exitErr != nil { + return "", fmt.Errorf("error running \"%s\" (exit code %d): %s", cmdStr, exitErr.ExitCode(), output) + } + return "", fmt.Errorf("error running \"%s\": %s", cmdStr, output) + } + return "", fmt.Errorf("error running \"%s\": %w", cmdStr, err) } - return string(resp), nil } diff --git a/utils/utils_test.go b/utils/utils_test.go index 95a0d7ec1..0b910b57d 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -504,6 +504,53 @@ func cleanupTempDir(t *testing.T, fileName string) { assert.NoError(t, err) } +func TestRunCmdAndWait(t *testing.T) { + t.Run("successful command returns stdout", func(t *testing.T) { + t.Parallel() + var out string + var err error + if runtime.GOOS == "windows" { + out, err = RunCmdAndWait("cmd", "/c", "echo", "hello") + } else { + out, err = RunCmdAndWait("echo", "hello") + } + assert.NoError(t, err) + assert.Contains(t, out, "hello") + }) + + t.Run("failed command with stderr includes command name and stderr in error", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping on Windows") + } + t.Parallel() + _, err := RunCmdAndWait("sh", "-c", "echo myerror >&2; exit 1") + assert.Error(t, err) + assert.Contains(t, err.Error(), `error running "sh -c echo myerror >&2; exit 1" (exit code 1)`) + assert.Contains(t, err.Error(), "myerror") + }) + + t.Run("failed command with no stderr falls back to stdout in error", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping on Windows") + } + t.Parallel() + _, err := RunCmdAndWait("sh", "-c", "echo myoutput; exit 1") + assert.Error(t, err) + assert.Contains(t, err.Error(), `error running "sh -c echo myoutput; exit 1" (exit code 1)`) + assert.Contains(t, err.Error(), "myoutput") + }) + + t.Run("failed command with no output wraps original error with command name", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping on Windows") + } + t.Parallel() + _, err := RunCmdAndWait("sh", "-c", "exit 1") + assert.Error(t, err) + assert.Contains(t, err.Error(), `error running "sh -c exit 1"`) + }) +} + func TestSanitizeDir(t *testing.T) { testcases := []struct { name string From 07a5b900e8eb9ed823ec7773d82731381bc8a1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Verg=C3=A9s?= Date: Wed, 4 Mar 2026 16:13:24 +0100 Subject: [PATCH 2/4] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Xavier Vergés --- utils/utils.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/utils/utils.go b/utils/utils.go index 435f8d6a4..45ee8bde7 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -194,7 +194,7 @@ func RunCmdAndWait(name string, args ...string) (string, error) { cmd := exec.Command(name, args...) resp, err := cmd.Output() if err != nil { - cmdStr := name + " " + strings.Join(args, " ") + cmdStr := strings.Join(append([]string{name}, args...), " ") output := "" var exitErr *exec.ExitError if errors.As(err, &exitErr) { @@ -209,6 +209,9 @@ func RunCmdAndWait(name string, args ...string) (string, error) { } return "", fmt.Errorf("error running \"%s\": %s", cmdStr, output) } + if exitErr != nil { + return "", fmt.Errorf("error running \"%s\" (exit code %d): %w", cmdStr, exitErr.ExitCode(), err) + } return "", fmt.Errorf("error running \"%s\": %w", cmdStr, err) } return string(resp), nil From affe0755f0bf2232871d0b75a6d0dbe3b8c2b2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Verg=C3=A9s?= Date: Thu, 5 Mar 2026 19:53:12 +0100 Subject: [PATCH 3/4] Apply Albert's simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Xavier Vergés --- utils/utils.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 45ee8bde7..b9bb17132 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -195,22 +195,13 @@ func RunCmdAndWait(name string, args ...string) (string, error) { resp, err := cmd.Output() if err != nil { cmdStr := strings.Join(append([]string{name}, args...), " ") - output := "" var exitErr *exec.ExitError if errors.As(err, &exitErr) { - output = strings.TrimSpace(string(exitErr.Stderr)) - } - if output == "" { - output = strings.TrimSpace(string(resp)) - } - if output != "" { - if exitErr != nil { - return "", fmt.Errorf("error running \"%s\" (exit code %d): %s", cmdStr, exitErr.ExitCode(), output) + output := strings.TrimSpace(string(exitErr.Stderr)) + if output == "" { + output = err.Error() } - return "", fmt.Errorf("error running \"%s\": %s", cmdStr, output) - } - if exitErr != nil { - return "", fmt.Errorf("error running \"%s\" (exit code %d): %w", cmdStr, exitErr.ExitCode(), err) + return "", fmt.Errorf("error running \"%s\" (exit code %d): %s", cmdStr, exitErr.ExitCode(), output) } return "", fmt.Errorf("error running \"%s\": %w", cmdStr, err) } From 7893b7c1ffb666a98fabbc0c39b7b3af1621e815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Verg=C3=A9s?= Date: Thu, 5 Mar 2026 20:06:17 +0100 Subject: [PATCH 4/4] Remove race condition in test cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Xavier Vergés --- utils/utils_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/utils_test.go b/utils/utils_test.go index 0b910b57d..5ce7f321d 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -174,7 +174,7 @@ func TestGetVersionAndImageVariant(t *testing.T) { func TestValidateFilePaths(t *testing.T) { dirName := createTempDir(t, "test_validate_paths") - defer cleanupTempDir(t, dirName) + t.Cleanup(func() { cleanupTempDir(t, dirName) }) validFile := createTempFile(t, dirName, "valid_test_file.yaml") testcases := []struct { name string @@ -311,7 +311,7 @@ func TestResolveHomeDir(t *testing.T) { func TestReadFile(t *testing.T) { fileName := createTempFile(t, "", "test_read_file") - defer cleanupTempDir(t, fileName) + t.Cleanup(func() { cleanupTempDir(t, fileName) }) testcases := []struct { name string input string