From 970cd62bf1ccab3d85a259be57685838afd5e5c6 Mon Sep 17 00:00:00 2001 From: Mayank Daruka Date: Thu, 23 May 2024 13:44:15 -0500 Subject: [PATCH 1/3] Initial commit to include stderr in error returned by executing command --- pkg/commandhandler/cmd.go | 16 ++++++++++------ pkg/commandhandler/cmd_test.go | 12 +++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/commandhandler/cmd.go b/pkg/commandhandler/cmd.go index f576d20..1ddab34 100644 --- a/pkg/commandhandler/cmd.go +++ b/pkg/commandhandler/cmd.go @@ -3,13 +3,14 @@ package commandhandler import ( - "github.com/Azure/azure-extension-platform/pkg/constants" - "github.com/Azure/azure-extension-platform/pkg/logging" - "github.com/pkg/errors" "io" "os" "os/exec" "path/filepath" + + "github.com/Azure/azure-extension-platform/pkg/constants" + "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/pkg/errors" ) type ICommandHandler interface { @@ -70,10 +71,13 @@ func execCmdInDirWithAction(cmd, workingDir, logDir string, waitForCompletion bo stdOutFile.Close() } - stdErrFile, err3 := os.OpenFile(errFileName, os.O_RDONLY, constants.FilePermissions_UserOnly_ReadWrite) + stdErrContents, err3 := os.ReadFile(errFileName) if err3 == nil { - el.InfoFromStream("stderr:", stdErrFile) - stdErrFile.Close() + el.Info("stderr: %s", stdErrContents) + + if execErr != nil { + return exitCode, errors.Wrap(execErr, string(stdErrContents)) + } } } else { diff --git a/pkg/commandhandler/cmd_test.go b/pkg/commandhandler/cmd_test.go index 94a37df..3b0b824 100644 --- a/pkg/commandhandler/cmd_test.go +++ b/pkg/commandhandler/cmd_test.go @@ -3,13 +3,14 @@ package commandhandler import ( - "github.com/Azure/azure-extension-platform/pkg/logging" - "github.com/stretchr/testify/assert" "io/ioutil" "os" "path" "strings" "testing" + + "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/stretchr/testify/assert" ) var workingDir = path.Join(".", "testdir", "currentWorkingDir") @@ -49,5 +50,10 @@ func TestNonExistingCommand(t *testing.T) { cmd := New() retcode, err := cmd.Execute("command_does_not_exist", workingDir, workingDir, true, extensionLogger) assert.Equal(t, commandNotExistReturnCode, retcode) - assert.Error(t, err) + assert.Error(t, err, "command execution should fail") + assert.Contains(t, err.Error(), "is recognized as an internal or external command", "error returned by cmd.Execute should include stderr") + + fileInfo, err := os.ReadFile(path.Join(workingDir, "stderr")) + assert.NoError(t, err, "stderr file should be read") + assert.Contains(t, string(fileInfo), "is not recognized as an internal or external command", "stderr message should be as expected") } From df075ed8a92528019782c1eeddca19d0b87a8821 Mon Sep 17 00:00:00 2001 From: Mayank Daruka Date: Thu, 23 May 2024 15:39:33 -0500 Subject: [PATCH 2/3] Fix test --- pkg/commandhandler/cmd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commandhandler/cmd_test.go b/pkg/commandhandler/cmd_test.go index 3b0b824..251947a 100644 --- a/pkg/commandhandler/cmd_test.go +++ b/pkg/commandhandler/cmd_test.go @@ -51,7 +51,7 @@ func TestNonExistingCommand(t *testing.T) { retcode, err := cmd.Execute("command_does_not_exist", workingDir, workingDir, true, extensionLogger) assert.Equal(t, commandNotExistReturnCode, retcode) assert.Error(t, err, "command execution should fail") - assert.Contains(t, err.Error(), "is recognized as an internal or external command", "error returned by cmd.Execute should include stderr") + assert.Contains(t, err.Error(), "is not recognized as an internal or external command", "error returned by cmd.Execute should include stderr") fileInfo, err := os.ReadFile(path.Join(workingDir, "stderr")) assert.NoError(t, err, "stderr file should be read") From 06546520dd39cc7f2dc737c9b9462cbb3334872d Mon Sep 17 00:00:00 2001 From: Mayank Daruka Date: Thu, 23 May 2024 15:44:29 -0500 Subject: [PATCH 3/3] Fix test --- pkg/commandhandler/cmd_linux_test.go | 17 ++++++++++++++++- pkg/commandhandler/cmd_test.go | 13 ------------- pkg/commandhandler/cmd_windows_test.go | 16 +++++++++++++++- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/pkg/commandhandler/cmd_linux_test.go b/pkg/commandhandler/cmd_linux_test.go index 3d30f99..01e5fd8 100644 --- a/pkg/commandhandler/cmd_linux_test.go +++ b/pkg/commandhandler/cmd_linux_test.go @@ -4,11 +4,13 @@ package commandhandler import ( "encoding/json" - "github.com/stretchr/testify/assert" "io/ioutil" + "os" "path" "strings" "testing" + + "github.com/stretchr/testify/assert" ) const ( @@ -97,3 +99,16 @@ func TestCommandWithEnvironmentVariableEmpty(t *testing.T) { assert.NoError(t, err, "stdout file should be read") assert.Contains(t, string(fileInfo), "", "stdout message should be as expected") } + +func TestNonExistingCommand(t *testing.T) { + defer cleanupTest() + cmd := New() + retcode, err := cmd.Execute("command_does_not_exist", workingDir, workingDir, true, extensionLogger) + assert.Equal(t, commandNotExistReturnCode, retcode) + assert.Error(t, err, "command execution should fail") + assert.Contains(t, err.Error(), "command_does_not_exist: not found", "error returned by cmd.Execute should include stderr") + + fileInfo, err := os.ReadFile(path.Join(workingDir, "stderr")) + assert.NoError(t, err, "stderr file should be read") + assert.Contains(t, string(fileInfo), "command_does_not_exist: not found", "stderr message should be as expected") +} diff --git a/pkg/commandhandler/cmd_test.go b/pkg/commandhandler/cmd_test.go index 251947a..9057902 100644 --- a/pkg/commandhandler/cmd_test.go +++ b/pkg/commandhandler/cmd_test.go @@ -44,16 +44,3 @@ func TestStderr(t *testing.T) { stdoutResult := strings.TrimSuffix(strings.TrimSuffix(string(fileBytes), lineReturnCharacter), " ") assert.Equal(t, "1 2 3 4", stdoutResult) } - -func TestNonExistingCommand(t *testing.T) { - defer cleanupTest() - cmd := New() - retcode, err := cmd.Execute("command_does_not_exist", workingDir, workingDir, true, extensionLogger) - assert.Equal(t, commandNotExistReturnCode, retcode) - assert.Error(t, err, "command execution should fail") - assert.Contains(t, err.Error(), "is not recognized as an internal or external command", "error returned by cmd.Execute should include stderr") - - fileInfo, err := os.ReadFile(path.Join(workingDir, "stderr")) - assert.NoError(t, err, "stderr file should be read") - assert.Contains(t, string(fileInfo), "is not recognized as an internal or external command", "stderr message should be as expected") -} diff --git a/pkg/commandhandler/cmd_windows_test.go b/pkg/commandhandler/cmd_windows_test.go index ff63a2d..d8d6633 100644 --- a/pkg/commandhandler/cmd_windows_test.go +++ b/pkg/commandhandler/cmd_windows_test.go @@ -4,12 +4,13 @@ package commandhandler import ( "encoding/json" - "github.com/stretchr/testify/assert" "io/ioutil" "os" "path" "testing" "time" + + "github.com/stretchr/testify/assert" ) const ( @@ -140,3 +141,16 @@ func TestDoesntWaitForCompletionEnvironmentVariable(t *testing.T) { duration := endTime.Sub(startTime) assert.Less(t, duration, time.Second, "execute shouldn't block") } + +func TestNonExistingCommand(t *testing.T) { + defer cleanupTest() + cmd := New() + retcode, err := cmd.Execute("command_does_not_exist", workingDir, workingDir, true, extensionLogger) + assert.Equal(t, commandNotExistReturnCode, retcode) + assert.Error(t, err, "command execution should fail") + assert.Contains(t, err.Error(), "is not recognized as an internal or external command", "error returned by cmd.Execute should include stderr") + + fileInfo, err := os.ReadFile(path.Join(workingDir, "stderr")) + assert.NoError(t, err, "stderr file should be read") + assert.Contains(t, string(fileInfo), "is not recognized as an internal or external command", "stderr message should be as expected") +}