From 41e3b2091996240a12750fabeb296c59b3bf0ad4 Mon Sep 17 00:00:00 2001 From: petergmurphy Date: Tue, 24 May 2022 11:13:06 +0100 Subject: [PATCH] (MAINT) Better error outputting to terminal This commit changes how errors are outputted to the terminal. Previously, when a tool returned a non-success exit code and no stderr, the terminal would just output an exit code. Now, when tool returns a non-success exit code and no stderr, the stdout will be outputted to the terminal. The tool's stdout should contain what validation errors occurred. This commit also adds a change to how the code directory is presented in the outputted text. Previously, any usage of a code directory would have `/code/` prepended onto it as it was mounted to this docker directory. Now PRM will find and remove any usage of `/code/` in the output text. --- .../exec/exec_tool_common_params_test.go | 3 -- acceptance/validate/validate_test.go | 17 +++++------ pkg/prm/docker.go | 5 ++-- pkg/prm/validate.go | 29 +++++++++++++------ 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/acceptance/exec/exec_tool_common_params_test.go b/acceptance/exec/exec_tool_common_params_test.go index 8c27a7d..fdfed7f 100644 --- a/acceptance/exec/exec_tool_common_params_test.go +++ b/acceptance/exec/exec_tool_common_params_test.go @@ -158,7 +158,6 @@ func Test_PrmExec_Tool_UseScriptBadEntrypoint(t *testing.T) { // // Exec // stdout, stderr, exitCode := testutils.RunAppCommand(fmt.Sprintf("exec puppetlabs/requires-git --alwaysBuild --toolpath %s --cachedir %s --codedir %s", toolDir, cacheDir, codeDir), "") -// fmt.Printf("stdout: %s", stdout) // // Assert // assert.Contains(t, stdout, "Tool puppetlabs/requires-git executed successfully") // GH-66 // // assert.Contains(t, stdout, "This script is so foo bar!") @@ -186,7 +185,6 @@ func Test_PrmExec_Tool_PuppetEnabled(t *testing.T) { // Exec stdout, stderr, exitCode := testutils.RunAppCommand(fmt.Sprintf("exec puppetlabs/check-puppet --alwaysBuild --toolpath %s --cachedir %s --codedir %s", toolDir, cacheDir, codeDir), "") - fmt.Printf("stdout: %s", stdout) // Assert assert.Contains(t, stdout, "Tool puppetlabs/check-puppet executed successfully") // GH-66 @@ -216,7 +214,6 @@ func Test_PrmExec_alwaysBuild_Flag(t *testing.T) { // Exec stdout, stderr, exitCode := testutils.RunAppCommand(fmt.Sprintf("exec puppetlabs/build-flag --alwaysBuild --toolpath %s --cachedir %s --codedir %s", toolDir, cacheDir, codeDir), "") - fmt.Printf("stdout: %s", stdout) // Assert assert.Contains(t, stdout, "Tool puppetlabs/build-flag executed successfully") // GH-66 assert.Contains(t, stdout, "This script is so foo bar!") diff --git a/acceptance/validate/validate_test.go b/acceptance/validate/validate_test.go index bd66a96..ec3e991 100644 --- a/acceptance/validate/validate_test.go +++ b/acceptance/validate/validate_test.go @@ -2,15 +2,16 @@ package validate_test import ( "fmt" - dircopy "github.com/otiai10/copy" - "github.com/puppetlabs/pct/acceptance/testutils" - "github.com/stretchr/testify/assert" "os" "path" "path/filepath" "regexp" "runtime" "testing" + + dircopy "github.com/otiai10/copy" + "github.com/puppetlabs/pct/acceptance/testutils" + "github.com/stretchr/testify/assert" ) const APP = "prm" @@ -48,10 +49,9 @@ func Test_PrmValidate_Single_Tool_Fail_Output_To_Terminal(t *testing.T) { // Exec stdout, stderr, exitCode := testutils.RunAppCommand(fmt.Sprintf("validate puppetlabs/puppet-lint --toolpath %s --cachedir %s --codedir %s --toolArgs=invalid.pp", toolDir, cacheDir, codeDir), "") - fmt.Println(stdout) // Assert assert.Contains(t, stdout, "Validation returned 1 error") - assert.Contains(t, stdout, "Tool exited with code: 1") + assert.Contains(t, stdout, "ERROR: invalid not in autoload module layout on line 1 (check: autoloader_layout)") assert.Equal(t, "exit status 1", stderr) assert.Equal(t, 1, exitCode) } @@ -93,7 +93,6 @@ func Test_PrmValidate_Single_Tool_Fail_Output_To_File(t *testing.T) { // Assert mustContain := []string{ - "Tool exited with code: 1", "ERROR: invalid not in autoload module layout on line 1 (check: autoloader_layout)", "WARNING: class not documented on line 1 (check: documentation)", } @@ -125,8 +124,8 @@ func Test_PrmValidate_Validate_File_Multitool_Fail_Output_To_Terminal(t *testing stdout, stderr, exitCode := testutils.RunAppCommand(fmt.Sprintf("validate --toolpath %s --cachedir %s --codedir %s --group test_group_1 --resultsView terminal", toolDir, cacheDir, codeDir), "") // Assert - assert.Contains(t, stdout, "Tool exited with code: 1") - assert.Contains(t, stdout, "Tool exited with code: 1") // GH-66 + assert.Contains(t, stdout, "WARNING: class not documented on line 1 (check: documentation)") + assert.Contains(t, stdout, "ERROR: invalid not in autoload module layout on line 1 (check: autoloader_layout)") assert.Equal(t, "exit status 1", stderr) assert.Equal(t, 1, exitCode) } @@ -156,7 +155,7 @@ func Test_PrmValidate_Validate_File_Multitool_Fail_Output_To_File(t *testing.T) // Assert mustContain := map[string][]string{ "puppet-lint": { - "Tool exited with code: 1", + "WARNING: class not documented on line 1 (check: documentation)", }, "rubocop": { "Inspecting 0 files", // TODO: Setup a proper tool for this test diff --git a/pkg/prm/docker.go b/pkg/prm/docker.go index 8770119..13f6ec6 100644 --- a/pkg/prm/docker.go +++ b/pkg/prm/docker.go @@ -6,13 +6,14 @@ import ( "context" "encoding/json" "fmt" - "github.com/spf13/viper" "io" "os" "path/filepath" "strings" "time" + "github.com/spf13/viper" + "github.com/Masterminds/semver" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -371,7 +372,7 @@ func (d *Docker) Validate(toolInfo ToolInfo, prmConfig Config, paths DirectoryPa if containerOutput.stderr != "" { err = fmt.Errorf("%s", containerOutput.stderr) } else { - err = fmt.Errorf("Tool exited with code: %d", exitValues.StatusCode) + err = fmt.Errorf("") } return VALIDATION_FAILED, containerOutput.stdout, err } diff --git a/pkg/prm/validate.go b/pkg/prm/validate.go index 5b51d89..30b75ac 100644 --- a/pkg/prm/validate.go +++ b/pkg/prm/validate.go @@ -4,14 +4,15 @@ package prm import ( "errors" "fmt" - "github.com/olekukonko/tablewriter" - "github.com/rs/zerolog/log" - "github.com/spf13/afero" "os" "path" "regexp" "strings" "time" + + "github.com/olekukonko/tablewriter" + "github.com/rs/zerolog/log" + "github.com/spf13/afero" ) type ValidateExitCode int64 @@ -87,18 +88,28 @@ func (p *Prm) outputResults(tasks []*Task[ValidationOutput], settings OutputSett func writeOutputToTerminal(tasks []*Task[ValidationOutput]) { for _, task := range tasks { output := task.Output - if output.err != nil { - text := fmt.Sprintf("%s:\n%s", task.Name, removeStringFormatting(output.err.Error())) - log.Error().Msgf(text) + if output.err == nil { + continue + } + + var errText string + if output.err.Error() != "" { + errText = output.err.Error() + } else { + errText = output.stdout } + errText = cleanOutput(errText) + + log.Error().Msgf(fmt.Sprintf("%s:\n%s", task.Name, cleanOutput(errText))) } } -func removeStringFormatting(text string) string { +func cleanOutput(text string) string { exp := regexp.MustCompile(`\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])`) text = exp.ReplaceAllString(text, "") text = strings.TrimPrefix(text, "\n") // Trim prefix newline if it exists text = strings.TrimSuffix(text, "\n") + text = strings.ReplaceAll(text, "/code/", "") return text } @@ -172,9 +183,9 @@ func writeStringToFile(file afero.File, output ValidationOutput) error { errText := "" // Remove ANSI formatting from output strings if output.err != nil { - errText = removeStringFormatting(output.err.Error()) + errText = cleanOutput(output.err.Error()) } - stdout := removeStringFormatting(output.stdout) + stdout := cleanOutput(output.stdout) _, err := file.WriteString(fmt.Sprintf("%s\n%s", stdout, errText)) if err != nil {