command: no visual warning hierarchy in -no-color

Commit e865faf adds visual indentation for diagnostic messages using various
vertical line characters. The present commit disables this behaviour when
running with colourised output disabled.

While the contents of stderr are not intended to be part of the Terraform API,
this is currently how the hashicorp/terraform-exec library detects certain
error types in order to present them as well-known Go errors to the user. Such
detection is complicated when vertical lines are added to the CLI output at
unpredictable points.

I expect this change will also be helpful for screen reader users.
This commit is contained in:
Katy Moe 2021-01-26 18:13:32 +00:00 committed by Martin Atkins
parent bc51932e08
commit 51c687c2db
9 changed files with 488 additions and 46 deletions

View File

@ -9,6 +9,8 @@ import (
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform/e2e"
)
@ -330,18 +332,17 @@ func TestInitProviderNotFound(t *testing.T) {
fixturePath := filepath.Join("testdata", "provider-not-found")
tf := e2e.NewBinary(terraformBin, fixturePath)
tf.AddEnv("TF_CLI_ARGS=-no-color")
defer tf.Close()
t.Run("registry provider not found", func(t *testing.T) {
_, stderr, err := tf.Run("init")
_, stderr, err := tf.Run("init", "-no-color")
if err == nil {
t.Fatal("expected error, got success")
}
oneLineStderr := strings.ReplaceAll(stderr, "\n", "")
oneLineStderr := strings.ReplaceAll(stderr, "\n", " ")
if !strings.Contains(oneLineStderr, "provider registry registry.terraform.io does not have a provider named registry.terraform.io/hashicorp/nonexist") {
t.Errorf("expected error message is missing from output:\n%s", oneLineStderr)
t.Errorf("expected error message is missing from output:\n%s", stderr)
}
})
@ -352,14 +353,33 @@ func TestInitProviderNotFound(t *testing.T) {
t.Fatal(err)
}
_, stderr, err := tf.Run("init", "-plugin-dir="+pluginDir)
_, stderr, err := tf.Run("init", "-no-color", "-plugin-dir="+pluginDir)
if err == nil {
t.Fatal("expected error, got success")
}
oneLineStderr := strings.ReplaceAll(stderr, "\n│", "")
if !strings.Contains(oneLineStderr, "provider registry.terraform.io/hashicorp/nonexist was not found in any of the search locations - "+pluginDir) {
t.Errorf("expected error message is missing from output:\n%s", oneLineStderr)
if !strings.Contains(stderr, "provider registry.terraform.io/hashicorp/nonexist was not\nfound in any of the search locations\n\n - "+pluginDir) {
t.Errorf("expected error message is missing from output:\n%s", stderr)
}
})
t.Run("special characters enabled", func(t *testing.T) {
_, stderr, err := tf.Run("init")
if err == nil {
t.Fatal("expected error, got success")
}
expectedErr := `
Error: Failed to query available provider packages
` + ` ` + `
Could not retrieve the list of available versions for provider
hashicorp/nonexist: provider registry registry.terraform.io does not have a
provider named registry.terraform.io/hashicorp/nonexist
`
if stripAnsi(stderr) != expectedErr {
t.Errorf("wrong output:\n%s", cmp.Diff(stripAnsi(stderr), expectedErr))
}
})
}

View File

@ -0,0 +1,13 @@
package e2etest
import (
"regexp"
)
const ansi = "[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))"
var ansiRe = regexp.MustCompile(ansi)
func stripAnsi(str string) string {
return ansiRe.ReplaceAllString(str, "")
}

View File

@ -16,6 +16,11 @@ import (
"github.com/zclconf/go-cty/cty"
)
var disabledColorize = &colorstring.Colorize{
Colors: colorstring.DefaultColors,
Disable: true,
}
// Diagnostic formats a single diagnostic message.
//
// The width argument specifies at what column the diagnostic messages will
@ -115,6 +120,57 @@ func Diagnostic(diag tfdiags.Diagnostic, sources map[string][]byte, color *color
return ruleBuf.String()
}
// DiagnosticPlain is an alternative to Diagnostic which minimises the use of
// virtual terminal formatting sequences.
//
// It is intended for use in automation and other contexts in which diagnostic
// messages are parsed from the Terraform output.
func DiagnosticPlain(diag tfdiags.Diagnostic, sources map[string][]byte, width int) string {
if diag == nil {
// No good reason to pass a nil diagnostic in here...
return ""
}
var buf bytes.Buffer
switch diag.Severity() {
case tfdiags.Error:
buf.WriteString("\nError: ")
case tfdiags.Warning:
buf.WriteString("\nWarning: ")
default:
buf.WriteString("\n")
}
desc := diag.Description()
sourceRefs := diag.Source()
// We don't wrap the summary, since we expect it to be terse, and since
// this is where we put the text of a native Go error it may not always
// be pure text that lends itself well to word-wrapping.
fmt.Fprintf(&buf, "%s\n\n", desc.Summary)
if sourceRefs.Subject != nil {
buf = appendSourceSnippets(buf, diag, sources, disabledColorize)
}
if desc.Detail != "" {
if width > 1 {
lines := strings.Split(desc.Detail, "\n")
for _, line := range lines {
if !strings.HasPrefix(line, " ") {
line = wordwrap.WrapString(line, uint(width-1))
}
fmt.Fprintf(&buf, "%s\n", line)
}
} else {
fmt.Fprintf(&buf, "%s\n", desc.Detail)
}
}
return buf.String()
}
// DiagnosticWarningsCompact is an alternative to Diagnostic for when all of
// the given diagnostics are warnings and we want to show them compactly,
// with only two lines per warning and excluding all of the detail information.

View File

@ -230,6 +230,212 @@ func TestDiagnostic(t *testing.T) {
}
}
func TestDiagnosticPlain(t *testing.T) {
tests := map[string]struct {
Diag interface{}
Want string
}{
"sourceless error": {
tfdiags.Sourceless(
tfdiags.Error,
"A sourceless error",
"It has no source references but it does have a pretty long detail that should wrap over multiple lines.",
),
`
Error: A sourceless error
It has no source references but it does
have a pretty long detail that should
wrap over multiple lines.
`,
},
"sourceless warning": {
tfdiags.Sourceless(
tfdiags.Warning,
"A sourceless warning",
"It has no source references but it does have a pretty long detail that should wrap over multiple lines.",
),
`
Warning: A sourceless warning
It has no source references but it does
have a pretty long detail that should
wrap over multiple lines.
`,
},
"error with source code subject": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
},
`
Error: Bad bad bad
on test.tf line 1:
1: test source code
Whatever shall we do?
`,
},
"error with source code subject and known expression": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
Expression: hcltest.MockExprTraversal(hcl.Traversal{
hcl.TraverseRoot{Name: "boop"},
hcl.TraverseAttr{Name: "beep"},
}),
EvalContext: &hcl.EvalContext{
Variables: map[string]cty.Value{
"boop": cty.ObjectVal(map[string]cty.Value{
"beep": cty.StringVal("blah"),
}),
},
},
},
`
Error: Bad bad bad
on test.tf line 1:
1: test source code
boop.beep is "blah"
Whatever shall we do?
`,
},
"error with source code subject and expression referring to sensitive value": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
Expression: hcltest.MockExprTraversal(hcl.Traversal{
hcl.TraverseRoot{Name: "boop"},
hcl.TraverseAttr{Name: "beep"},
}),
EvalContext: &hcl.EvalContext{
Variables: map[string]cty.Value{
"boop": cty.ObjectVal(map[string]cty.Value{
"beep": cty.StringVal("blah").Mark("sensitive"),
}),
},
},
},
`
Error: Bad bad bad
on test.tf line 1:
1: test source code
boop.beep has a sensitive value
Whatever shall we do?
`,
},
"error with source code subject and unknown string expression": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
Expression: hcltest.MockExprTraversal(hcl.Traversal{
hcl.TraverseRoot{Name: "boop"},
hcl.TraverseAttr{Name: "beep"},
}),
EvalContext: &hcl.EvalContext{
Variables: map[string]cty.Value{
"boop": cty.ObjectVal(map[string]cty.Value{
"beep": cty.UnknownVal(cty.String),
}),
},
},
},
`
Error: Bad bad bad
on test.tf line 1:
1: test source code
boop.beep is a string, known only after apply
Whatever shall we do?
`,
},
"error with source code subject and unknown expression of unknown type": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
Expression: hcltest.MockExprTraversal(hcl.Traversal{
hcl.TraverseRoot{Name: "boop"},
hcl.TraverseAttr{Name: "beep"},
}),
EvalContext: &hcl.EvalContext{
Variables: map[string]cty.Value{
"boop": cty.ObjectVal(map[string]cty.Value{
"beep": cty.UnknownVal(cty.DynamicPseudoType),
}),
},
},
},
`
Error: Bad bad bad
on test.tf line 1:
1: test source code
boop.beep will be known only after apply
Whatever shall we do?
`,
},
}
sources := map[string][]byte{
"test.tf": []byte(`test source code`),
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
var diags tfdiags.Diagnostics
diags = diags.Append(test.Diag) // to normalize it into a tfdiag.Diagnostic
diag := diags[0]
got := strings.TrimSpace(DiagnosticPlain(diag, sources, 40))
want := strings.TrimSpace(test.Want)
if got != want {
t.Errorf("wrong result\ngot:\n%s\n\nwant:\n%s\n\n", got, want)
}
})
}
}
func TestDiagnosticWarningsCompact(t *testing.T) {
var diags tfdiags.Diagnostics
diags = diags.Append(tfdiags.SimpleWarning("foo"))
@ -390,6 +596,49 @@ func TestDiagnostic_emptyOverlapHighlightContext(t *testing.T) {
}
}
func TestDiagnosticPlain_emptyOverlapHighlightContext(t *testing.T) {
var diags tfdiags.Diagnostics
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Some error",
Detail: "...",
Subject: &hcl.Range{
Filename: "source.tf",
Start: hcl.Pos{Line: 3, Column: 10, Byte: 38},
End: hcl.Pos{Line: 4, Column: 1, Byte: 39},
},
Context: &hcl.Range{
Filename: "source.tf",
Start: hcl.Pos{Line: 2, Column: 13, Byte: 27},
End: hcl.Pos{Line: 4, Column: 1, Byte: 39},
},
})
sources := map[string][]byte{
"source.tf": []byte(`variable "x" {
default = {
"foo"
}
`),
}
expected := `
Error: Some error
on source.tf line 3, in variable "x":
2: default = {
3: "foo"
4: }
...
`
output := DiagnosticPlain(diags[0], sources, 80)
if output != expected {
t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected)
}
}
func TestDiagnostic_wrapDetailIncludingCommand(t *testing.T) {
var diags tfdiags.Diagnostics
@ -422,3 +671,31 @@ func TestDiagnostic_wrapDetailIncludingCommand(t *testing.T) {
t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected)
}
}
func TestDiagnosticPlain_wrapDetailIncludingCommand(t *testing.T) {
var diags tfdiags.Diagnostics
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Everything went wrong",
Detail: "This is a very long sentence about whatever went wrong which is supposed to wrap onto multiple lines. Thank-you very much for listening.\n\nTo fix this, run this very long command:\n terraform read-my-mind -please -thanks -but-do-not-wrap-this-line-because-it-is-prefixed-with-spaces\n\nHere is a coda which is also long enough to wrap and so it should eventually make it onto multiple lines. THE END",
})
expected := `
Error: Everything went wrong
This is a very long sentence about whatever went wrong which is supposed to
wrap onto multiple lines. Thank-you very much for listening.
To fix this, run this very long command:
terraform read-my-mind -please -thanks -but-do-not-wrap-this-line-because-it-is-prefixed-with-spaces
Here is a coda which is also long enough to wrap and so it should
eventually make it onto multiple lines. THE END
`
output := DiagnosticPlain(diags[0], nil, 76)
if output != expected {
t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected)
}
}

View File

@ -9,15 +9,9 @@ import (
"github.com/hashicorp/terraform/providers"
"github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/terraform"
"github.com/mitchellh/colorstring"
"github.com/zclconf/go-cty/cty"
)
var disabledColorize = &colorstring.Colorize{
Colors: colorstring.DefaultColors,
Disable: true,
}
func TestState(t *testing.T) {
tests := []struct {
State *StateOpts

View File

@ -656,14 +656,20 @@ func (m *Meta) showDiagnostics(vals ...interface{}) {
}
for _, diag := range diags {
msg := format.Diagnostic(diag, m.configSources(), m.Colorize(), outputWidth)
var msg string
if m.Color {
msg = format.Diagnostic(diag, m.configSources(), m.Colorize(), outputWidth)
} else {
msg = format.DiagnosticPlain(diag, m.configSources(), outputWidth)
}
switch diag.Severity() {
case tfdiags.Error:
m.Ui.Error(strings.TrimSpace(msg))
m.Ui.Error(msg)
case tfdiags.Warning:
m.Ui.Warn(strings.TrimSpace(msg))
m.Ui.Warn(msg)
default:
m.Ui.Output(strings.TrimSpace(msg))
m.Ui.Output(msg)
}
}
}

View File

@ -4,16 +4,21 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/mitchellh/cli"
"github.com/mitchellh/colorstring"
"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/states"
)
var disabledColorize = &colorstring.Colorize{
Colors: colorstring.DefaultColors,
Disable: true,
}
func TestStateMv(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
@ -270,7 +275,8 @@ func TestStateMv_resourceToInstanceErr(t *testing.T) {
statePath := testStateFile(t, state)
p := testProvider()
ui := new(cli.MockUi)
ui := cli.NewMockUi()
c := &StateMvCommand{
StateMeta{
Meta: Meta{
@ -290,19 +296,88 @@ func TestStateMv_resourceToInstanceErr(t *testing.T) {
t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
}
expectedErr := `
Error: Invalid target address
Cannot move test_instance.foo to test_instance.bar[0]: the source is a
whole resource (not a resource instance) so the target must also be a whole
resource.
expectedErr := `
Error: Invalid target address
Cannot move test_instance.foo to test_instance.bar[0]: the source is a whole
resource (not a resource instance) so the target must also be a whole
resource.
`
errOutput := ui.ErrorWriter.String()
if errOutput != expectedErr {
t.Errorf("wrong output\n%s", cmp.Diff(errOutput, expectedErr))
}
}
func TestStateMv_resourceToInstanceErrInAutomation(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_instance",
Name: "foo",
}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance),
&states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"id":"bar","foo":"value","bar":"value"}`),
Status: states.ObjectReady,
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
Module: addrs.RootModule,
},
)
s.SetResourceProvider(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_instance",
Name: "bar",
}.Absolute(addrs.RootModuleInstance),
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
Module: addrs.RootModule,
},
)
})
statePath := testStateFile(t, state)
p := testProvider()
ui := new(cli.MockUi)
c := &StateMvCommand{
StateMeta{
Meta: Meta{
testingOverrides: metaOverridesForProvider(p),
Ui: ui,
RunningInAutomation: true,
},
},
}
args := []string{
"-state", statePath,
"test_instance.foo",
"test_instance.bar[0]",
}
if code := c.Run(args); code == 0 {
t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
}
expectedErr := `
Error: Invalid target address
Cannot move test_instance.foo to test_instance.bar[0]: the source is a whole
resource (not a resource instance) so the target must also be a whole
resource.
`
errOutput := ui.ErrorWriter.String()
if errOutput != expectedErr {
t.Errorf("Unexpected diff.\ngot:\n%s\nwant:\n%s\n", errOutput, expectedErr)
t.Errorf("%s", cmp.Diff(errOutput, expectedErr))
}
}
func TestStateMv_instanceToResource(t *testing.T) {
state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
@ -502,13 +577,14 @@ func TestStateMv_differentResourceTypes(t *testing.T) {
t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
}
gotErr := strings.TrimSpace(ui.ErrorWriter.String())
wantErr := strings.TrimSpace(`
Error: Invalid state move request
Cannot move test_instance.foo to test_network.bar: resource types don't
match.
`)
gotErr := ui.ErrorWriter.String()
wantErr := `
Error: Invalid state move request
Cannot move test_instance.foo to test_network.bar: resource types don't
match.
`
if gotErr != wantErr {
t.Fatalf("expected initialization error\ngot:\n%s\n\nwant:%s", gotErr, wantErr)
}

View File

@ -361,12 +361,12 @@ func TestTaint_missingAllow(t *testing.T) {
// Check for the warning
actual := strings.TrimSpace(ui.ErrorWriter.String())
expected := strings.TrimSpace(`
Warning: No such resource instance
Resource instance test_instance.bar was not found, but this is not an error
because -allow-missing was set.
expected := strings.TrimSpace(`
Warning: No such resource instance
Resource instance test_instance.bar was not found, but this is not an error
because -allow-missing was set.
`)
if diff := cmp.Diff(expected, actual); diff != "" {
t.Fatalf("wrong output\n%s", diff)

View File

@ -389,12 +389,12 @@ func TestUntaint_missingAllow(t *testing.T) {
// Check for the warning
actual := strings.TrimSpace(ui.ErrorWriter.String())
expected := strings.TrimSpace(`
Warning: No such resource instance
Resource instance test_instance.bar was not found, but this is not an error
because -allow-missing was set.
expected := strings.TrimSpace(`
Warning: No such resource instance
Resource instance test_instance.bar was not found, but this is not an error
because -allow-missing was set.
`)
if diff := cmp.Diff(expected, actual); diff != "" {
t.Fatalf("wrong output\n%s", diff)