Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
(MAINT) Better error outputting to terminal
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
petergmurphy committed May 31, 2022
1 parent 8eb5913 commit 41e3b20
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 23 deletions.
3 changes: 0 additions & 3 deletions acceptance/exec/exec_tool_common_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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!")
Expand Down
17 changes: 8 additions & 9 deletions acceptance/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)",
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/prm/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
29 changes: 20 additions & 9 deletions pkg/prm/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 41e3b20

Please sign in to comment.