Merge pull request #27761 from hashicorp/alisdair/command-views-operation-diagnostics

backend: Add per-operation diagnostic rendering

backend/backend.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"errors"
 	"io/ioutil"
+	"log"
 	"os"
 	"time"
 
@@ -208,6 +209,9 @@ type Operation struct {
 	UIIn  terraform.UIInput
 	UIOut terraform.UIOutput
 
+	// ShowDiagnostics prints diagnostic messages to the UI.
+	ShowDiagnostics func(vals ...interface{})
+
 	// If LockState is true, the Operation must Lock any
 	// statemgr.Lockers for its duration, and Unlock when complete.
 	LockState bool
@@ -240,6 +244,39 @@ func (o *Operation) Config() (*configs.Config, tfdiags.Diagnostics) {
 	return config, diags
 }
 
+// ReportResult is a helper for the common chore of setting the status of
+// a running operation and showing any diagnostics produced during that
+// operation.
+//
+// If the given diagnostics contains errors then the operation's result
+// will be set to backend.OperationFailure. It will be set to
+// backend.OperationSuccess otherwise. It will then use b.ShowDiagnostics
+// to show the given diagnostics before returning.
+//
+// Callers should feel free to do each of these operations separately in
+// more complex cases where e.g. diagnostics are interleaved with other
+// output, but terminating immediately after reporting error diagnostics is
+// common and can be expressed concisely via this method.
+func (o *Operation) ReportResult(op *RunningOperation, diags tfdiags.Diagnostics) {
+	if diags.HasErrors() {
+		op.Result = OperationFailure
+	} else {
+		op.Result = OperationSuccess
+	}
+	if o.ShowDiagnostics != nil {
+		o.ShowDiagnostics(diags)
+	} else {
+		// Shouldn't generally happen, but if it does then we'll at least
+		// make some noise in the logs to help us spot it.
+		if len(diags) != 0 {
+			log.Printf(
+				"[ERROR] Backend needs to report diagnostics but ShowDiagnostics is not set:\n%s",
+				diags.ErrWithWarnings(),
+			)
+		}
+	}
+}
+
 // RunningOperation is the result of starting an operation.
 type RunningOperation struct {
 	// For implementers of a backend, this context should not wrap the

backend/local/backend.go

@@ -43,9 +43,6 @@ type Local struct {
 	// input/output handles that CLI is connected to.
 	Streams *terminal.Streams
 
-	// ShowDiagnostics prints diagnostic messages to the UI.
-	ShowDiagnostics func(vals ...interface{})
-
 	// The State* paths are set from the backend config, and may be left blank
 	// to use the defaults. If the actual paths for the local backend state are
 	// needed, use the StatePaths method.
@@ -398,39 +395,6 @@ func (b *Local) opWait(
 	return
 }
 
-// ReportResult is a helper for the common chore of setting the status of
-// a running operation and showing any diagnostics produced during that
-// operation.
-//
-// If the given diagnostics contains errors then the operation's result
-// will be set to backend.OperationFailure. It will be set to
-// backend.OperationSuccess otherwise. It will then use b.ShowDiagnostics
-// to show the given diagnostics before returning.
-//
-// Callers should feel free to do each of these operations separately in
-// more complex cases where e.g. diagnostics are interleaved with other
-// output, but terminating immediately after reporting error diagnostics is
-// common and can be expressed concisely via this method.
-func (b *Local) ReportResult(op *backend.RunningOperation, diags tfdiags.Diagnostics) {
-	if diags.HasErrors() {
-		op.Result = backend.OperationFailure
-	} else {
-		op.Result = backend.OperationSuccess
-	}
-	if b.ShowDiagnostics != nil {
-		b.ShowDiagnostics(diags)
-	} else {
-		// Shouldn't generally happen, but if it does then we'll at least
-		// make some noise in the logs to help us spot it.
-		if len(diags) != 0 {
-			log.Printf(
-				"[ERROR] Local backend needs to report diagnostics but ShowDiagnostics is not set:\n%s",
-				diags.ErrWithWarnings(),
-			)
-		}
-	}
-}
-
 // Colorize returns the Colorize structure that can be used for colorizing
 // output. This is guaranteed to always return a non-nil value and so is useful
 // as a helper to wrap any potentially colored strings.

backend/local/backend_apply.go

@@ -35,7 +35,7 @@ func (b *Local) opApply(
 				"would mark everything for destruction, which is normally not what is desired. "+
 				"If you would like to destroy everything, run 'terraform destroy' instead.",
 		))
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
@@ -51,7 +51,7 @@ func (b *Local) opApply(
 	tfCtx, _, opState, contextDiags := b.context(op)
 	diags = diags.Append(contextDiags)
 	if contextDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 	// the state was locked during succesfull context creation; unlock the state
@@ -59,7 +59,7 @@ func (b *Local) opApply(
 	defer func() {
 		err := op.StateLocker.Unlock(nil)
 		if err != nil {
-			b.ShowDiagnostics(err)
+			op.ShowDiagnostics(err)
 			runningOp.Result = backend.OperationFailure
 		}
 	}()
@@ -73,7 +73,7 @@ func (b *Local) opApply(
 		plan, planDiags := tfCtx.Plan()
 		diags = diags.Append(planDiags)
 		if planDiags.HasErrors() {
-			b.ReportResult(runningOp, diags)
+			op.ReportResult(runningOp, diags)
 			return
 		}
 
@@ -109,7 +109,7 @@ func (b *Local) opApply(
 			// We'll show any accumulated warnings before we display the prompt,
 			// so the user can consider them when deciding how to answer.
 			if len(diags) > 0 {
-				b.ShowDiagnostics(diags)
+				op.ShowDiagnostics(diags)
 				diags = nil // reset so we won't show the same diagnostics again later
 			}
 
@@ -120,7 +120,7 @@ func (b *Local) opApply(
 			})
 			if err != nil {
 				diags = diags.Append(errwrap.Wrapf("Error asking for approval: {{err}}", err))
-				b.ReportResult(runningOp, diags)
+				op.ReportResult(runningOp, diags)
 				return
 			}
 			if v != "yes" {
@@ -167,20 +167,20 @@ func (b *Local) opApply(
 		stateFile.State = applyState
 
 		diags = diags.Append(b.backupStateForError(stateFile, err))
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
 	diags = diags.Append(applyDiags)
 	if applyDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
 	// If we've accumulated any warnings along the way then we'll show them
 	// here just before we show the summary and next steps. If we encountered
 	// errors then we would've returned early at some other point above.
-	b.ShowDiagnostics(diags)
+	op.ShowDiagnostics(diags)
 }
 
 // backupStateForError is called in a scenario where we're unable to persist the

backend/local/backend_apply_test.go

@@ -289,9 +289,10 @@ func testOperationApply(t *testing.T, configDir string) (*backend.Operation, fun
 	_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
 
 	return &backend.Operation{
-		Type:         backend.OperationTypeApply,
+		Type:            backend.OperationTypeApply,
-		ConfigDir:    configDir,
+		ConfigDir:       configDir,
-		ConfigLoader: configLoader,
+		ConfigLoader:    configLoader,
+		ShowDiagnostics: testLogDiagnostics(t),
 	}, configCleanup
 }
 

backend/local/backend_plan.go

@@ -41,7 +41,7 @@ func (b *Local) opPlan(
 			"The plan command was given a saved plan file as its input. This command generates "+
 				"a new plan, and so it requires a configuration directory as its argument.",
 		))
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
@@ -55,7 +55,7 @@ func (b *Local) opPlan(
 				"would like to destroy everything, run plan with the -destroy option. Otherwise, "+
 				"create a Terraform configuration file (.tf file) and try again.",
 		))
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
@@ -67,7 +67,7 @@ func (b *Local) opPlan(
 	tfCtx, configSnap, opState, ctxDiags := b.context(op)
 	diags = diags.Append(ctxDiags)
 	if ctxDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 	// the state was locked during succesfull context creation; unlock the state
@@ -75,7 +75,7 @@ func (b *Local) opPlan(
 	defer func() {
 		err := op.StateLocker.Unlock(nil)
 		if err != nil {
-			b.ShowDiagnostics(err)
+			op.ShowDiagnostics(err)
 			runningOp.Result = backend.OperationFailure
 		}
 	}()
@@ -103,7 +103,7 @@ func (b *Local) opPlan(
 
 	diags = diags.Append(planDiags)
 	if planDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
@@ -118,7 +118,7 @@ func (b *Local) opPlan(
 			diags = diags.Append(fmt.Errorf(
 				"PlanOutPath set without also setting PlanOutBackend (this is a bug in Terraform)"),
 			)
-			b.ReportResult(runningOp, diags)
+			op.ReportResult(runningOp, diags)
 			return
 		}
 		plan.Backend = *op.PlanOutBackend
@@ -136,7 +136,7 @@ func (b *Local) opPlan(
 				"Failed to write plan file",
 				fmt.Sprintf("The plan file could not be written: %s.", err),
 			))
-			b.ReportResult(runningOp, diags)
+			op.ReportResult(runningOp, diags)
 			return
 		}
 	}
@@ -149,7 +149,7 @@ func (b *Local) opPlan(
 			b.CLI.Output("\n" + b.Colorize().Color(strings.TrimSpace(planNoChanges)))
 			b.CLI.Output("\n" + strings.TrimSpace(format.WordWrap(planNoChangesDetail, outputColumns)))
 			// Even if there are no changes, there still could be some warnings
-			b.ShowDiagnostics(diags)
+			op.ShowDiagnostics(diags)
 			return
 		}
 
@@ -158,7 +158,7 @@ func (b *Local) opPlan(
 		// If we've accumulated any warnings along the way then we'll show them
 		// here just before we show the summary and next steps. If we encountered
 		// errors then we would've returned early at some other point above.
-		b.ShowDiagnostics(diags)
+		op.ShowDiagnostics(diags)
 
 		// Give the user some next-steps, unless we're running in an automation
 		// tool which is presumed to provide its own UI for further actions.

backend/local/backend_plan_test.go

@@ -114,6 +114,8 @@ func TestLocal_planNoConfig(t *testing.T) {
 	b.CLI = cli.NewMockUi()
 
 	op, configCleanup := testOperationPlan(t, "./testdata/empty")
+	record, playback := testRecordDiagnostics(t)
+	op.ShowDiagnostics = record
 	defer configCleanup()
 	op.PlanRefresh = true
 
@@ -126,8 +128,9 @@ func TestLocal_planNoConfig(t *testing.T) {
 	if run.Result == backend.OperationSuccess {
 		t.Fatal("plan operation succeeded; want failure")
 	}
-	output := b.CLI.(*cli.MockUi).ErrorWriter.String()
+
-	if !strings.Contains(output, "configuration") {
+	output := playback().Err().Error()
+	if !strings.Contains(output, "No configuration files") {
 		t.Fatalf("bad: %s", err)
 	}
 
@@ -733,9 +736,10 @@ func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func
 	_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
 
 	return &backend.Operation{
-		Type:         backend.OperationTypePlan,
+		Type:            backend.OperationTypePlan,
-		ConfigDir:    configDir,
+		ConfigDir:       configDir,
-		ConfigLoader: configLoader,
+		ConfigLoader:    configLoader,
+		ShowDiagnostics: testLogDiagnostics(t),
 	}, configCleanup
 }
 

backend/local/backend_refresh.go

@@ -36,7 +36,7 @@ func (b *Local) opRefresh(
 					"Cannot read state file",
 					fmt.Sprintf("Failed to read %s: %s", b.StatePath, err),
 				))
-				b.ReportResult(runningOp, diags)
+				op.ReportResult(runningOp, diags)
 				return
 			}
 		}
@@ -49,7 +49,7 @@ func (b *Local) opRefresh(
 	tfCtx, _, opState, contextDiags := b.context(op)
 	diags = diags.Append(contextDiags)
 	if contextDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
@@ -58,7 +58,7 @@ func (b *Local) opRefresh(
 	defer func() {
 		err := op.StateLocker.Unlock(nil)
 		if err != nil {
-			b.ShowDiagnostics(err)
+			op.ShowDiagnostics(err)
 			runningOp.Result = backend.OperationFailure
 		}
 	}()
@@ -94,14 +94,14 @@ func (b *Local) opRefresh(
 	runningOp.State = newState
 	diags = diags.Append(refreshDiags)
 	if refreshDiags.HasErrors() {
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 
 	err := statemgr.WriteAndPersist(opState, newState)
 	if err != nil {
 		diags = diags.Append(errwrap.Wrapf("Failed to write state: {{err}}", err))
-		b.ReportResult(runningOp, diags)
+		op.ReportResult(runningOp, diags)
 		return
 	}
 }

backend/local/backend_refresh_test.go

@@ -220,10 +220,11 @@ func testOperationRefresh(t *testing.T, configDir string) (*backend.Operation, f
 	_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
 
 	return &backend.Operation{
-		Type:         backend.OperationTypeRefresh,
+		Type:            backend.OperationTypeRefresh,
-		ConfigDir:    configDir,
+		ConfigDir:       configDir,
-		ConfigLoader: configLoader,
+		ConfigLoader:    configLoader,
-		LockState:    true,
+		LockState:       true,
+		ShowDiagnostics: testLogDiagnostics(t),
 	}, configCleanup
 }
 

backend/local/cli.go

@@ -12,7 +12,6 @@ func (b *Local) CLIInit(opts *backend.CLIOpts) error {
 	b.CLI = opts.CLI
 	b.CLIColor = opts.CLIColor
 	b.Streams = opts.Streams
-	b.ShowDiagnostics = opts.ShowDiagnostics
 	b.ContextOpts = opts.ContextOpts
 	b.OpInput = opts.Input
 	b.OpValidation = opts.Validation

backend/local/testing.go

@@ -34,26 +34,6 @@ func TestLocal(t *testing.T) (*Local, func()) {
 	local.StateWorkspaceDir = filepath.Join(tempDir, "state.tfstate.d")
 	local.ContextOpts = &terraform.ContextOpts{}
 
-	local.ShowDiagnostics = func(vals ...interface{}) {
-		var diags tfdiags.Diagnostics
-		diags = diags.Append(vals...)
-		for _, diag := range diags {
-			// NOTE: Since the caller here is not directly the TestLocal
-			// function, t.Helper doesn't apply and so the log source
-			// isn't correctly shown in the test log output. This seems
-			// unavoidable as long as this is happening so indirectly.
-			desc := diag.Description()
-			if desc.Detail != "" {
-				t.Logf("%s: %s", desc.Summary, desc.Detail)
-			} else {
-				t.Log(desc.Summary)
-			}
-			if local.CLI != nil {
-				local.CLI.Error(desc.Summary)
-			}
-		}
-	}
-
 	cleanup := func() {
 		if err := os.RemoveAll(tempDir); err != nil {
 			t.Fatal("error cleanup up test:", err)
@@ -265,3 +245,43 @@ func assertBackendStateLocked(t *testing.T, b *Local) bool {
 	t.Error("unexpected success locking state")
 	return true
 }
+
+// testRecordDiagnostics allows tests to record and later inspect diagnostics
+// emitted during an Operation. It returns a record function which can be set
+// as the ShowDiagnostics value of an Operation, and a playback function which
+// returns the recorded diagnostics for inspection.
+func testRecordDiagnostics(t *testing.T) (record func(vals ...interface{}), playback func() tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	record = func(vals ...interface{}) {
+		diags = diags.Append(vals...)
+	}
+	playback = func() tfdiags.Diagnostics {
+		diags.Sort()
+		return diags
+	}
+	return
+}
+
+// testLogDiagnostics returns a function which can be used as the
+// ShowDiagnostics value for an Operation, in order to help debugging during
+// tests. Any calls to this function result in test logs.
+func testLogDiagnostics(t *testing.T) func(vals ...interface{}) {
+	return func(vals ...interface{}) {
+		var diags tfdiags.Diagnostics
+		diags = diags.Append(vals...)
+		diags.Sort()
+
+		for _, diag := range diags {
+			// NOTE: Since the caller here is not directly the TestLocal
+			// function, t.Helper doesn't apply and so the log source
+			// isn't correctly shown in the test log output. This seems
+			// unavoidable as long as this is happening so indirectly.
+			desc := diag.Description()
+			if desc.Detail != "" {
+				t.Logf("%s: %s", desc.Summary, desc.Detail)
+			} else {
+				t.Log(desc.Summary)
+			}
+		}
+	}
+}

backend/remote/backend.go

@@ -45,9 +45,6 @@ type Remote struct {
 	CLI      cli.Ui
 	CLIColor *colorstring.Colorize
 
-	// ShowDiagnostics prints diagnostic messages to the UI.
-	ShowDiagnostics func(vals ...interface{})
-
 	// ContextOpts are the base context options to set when initializing a
 	// new Terraform context. Many of these will be overridden or merged by
 	// Operation. See Operation for more details.
@@ -755,7 +752,9 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 
 		r, opErr := f(stopCtx, cancelCtx, op, w)
 		if opErr != nil && opErr != context.Canceled {
-			b.ReportResult(runningOp, opErr)
+			var diags tfdiags.Diagnostics
+			diags = diags.Append(opErr)
+			op.ReportResult(runningOp, diags)
 			return
 		}
 
@@ -768,7 +767,9 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 			// Retrieve the run to get its current status.
 			r, err := b.client.Runs.Read(cancelCtx, r.ID)
 			if err != nil {
-				b.ReportResult(runningOp, generalError("Failed to retrieve run", err))
+				var diags tfdiags.Diagnostics
+				diags = diags.Append(generalError("Failed to retrieve run", err))
+				op.ReportResult(runningOp, diags)
 				return
 			}
 
@@ -777,7 +778,9 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 
 			if opErr == context.Canceled {
 				if err := b.cancel(cancelCtx, op, r); err != nil {
-					b.ReportResult(runningOp, generalError("Failed to retrieve run", err))
+					var diags tfdiags.Diagnostics
+					diags = diags.Append(generalError("Failed to retrieve run", err))
+					op.ReportResult(runningOp, diags)
 					return
 				}
 			}
@@ -831,43 +834,6 @@ func (b *Remote) cancel(cancelCtx context.Context, op *backend.Operation, r *tfe
 	return nil
 }
 
-// ReportResult is a helper for the common chore of setting the status of
-// a running operation and showing any diagnostics produced during that
-// operation.
-//
-// If the given diagnostics contains errors then the operation's result
-// will be set to backend.OperationFailure. It will be set to
-// backend.OperationSuccess otherwise. It will then use b.ShowDiagnostics
-// to show the given diagnostics before returning.
-//
-// Callers should feel free to do each of these operations separately in
-// more complex cases where e.g. diagnostics are interleaved with other
-// output, but terminating immediately after reporting error diagnostics is
-// common and can be expressed concisely via this method.
-func (b *Remote) ReportResult(op *backend.RunningOperation, err error) {
-	var diags tfdiags.Diagnostics
-
-	diags = diags.Append(err)
-	if diags.HasErrors() {
-		op.Result = backend.OperationFailure
-	} else {
-		op.Result = backend.OperationSuccess
-	}
-
-	if b.ShowDiagnostics != nil {
-		b.ShowDiagnostics(diags)
-	} else {
-		// Shouldn't generally happen, but if it does then we'll at least
-		// make some noise in the logs to help us spot it.
-		if len(diags) != 0 {
-			log.Printf(
-				"[ERROR] Remote backend needs to report diagnostics but ShowDiagnostics is not set:\n%s",
-				diags.ErrWithWarnings(),
-			)
-		}
-	}
-}
-
 // IgnoreVersionConflict allows commands to disable the fall-back check that
 // the local Terraform version matches the remote workspace's configured
 // Terraform version. This should be called by commands where this check is

backend/remote/backend_apply_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/terraform/plans/planfile"
 	"github.com/hashicorp/terraform/states/statemgr"
 	"github.com/hashicorp/terraform/terraform"
+	"github.com/hashicorp/terraform/tfdiags"
 	tfversion "github.com/hashicorp/terraform/version"
 	"github.com/mitchellh/cli"
 )
@@ -28,14 +29,26 @@ func testOperationApply(t *testing.T, configDir string) (*backend.Operation, fun
 	_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
 
 	return &backend.Operation{
-		ConfigDir:    configDir,
+		ConfigDir:       configDir,
-		ConfigLoader: configLoader,
+		ConfigLoader:    configLoader,
-		Parallelism:  defaultParallelism,
+		Parallelism:     defaultParallelism,
-		PlanRefresh:  true,
+		PlanRefresh:     true,
-		Type:         backend.OperationTypeApply,
+		ShowDiagnostics: testLogDiagnostics(t),
+		Type:            backend.OperationTypeApply,
 	}, configCleanup
 }
 
+func testOperationApplyWithDiagnostics(t *testing.T, configDir string) (*backend.Operation, func(), func() tfdiags.Diagnostics) {
+	t.Helper()
+
+	op, cleanup := testOperationApply(t, configDir)
+
+	record, playback := testRecordDiagnostics(t)
+	op.ShowDiagnostics = record
+
+	return op, cleanup, playback
+}
+
 func TestRemote_applyBasic(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
@@ -131,7 +144,7 @@ func TestRemote_applyWithoutPermissions(t *testing.T) {
 	}
 	w.Permissions.CanQueueApply = false
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	op.UIOut = b.CLI
@@ -147,7 +160,7 @@ func TestRemote_applyWithoutPermissions(t *testing.T) {
 		t.Fatal("expected apply operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "Insufficient rights to apply changes") {
 		t.Fatalf("expected a permissions error, got: %v", errOutput)
 	}
@@ -170,7 +183,7 @@ func TestRemote_applyWithVCS(t *testing.T) {
 		t.Fatalf("error creating named workspace: %v", err)
 	}
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	op.Workspace = "prod"
@@ -188,7 +201,7 @@ func TestRemote_applyWithVCS(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "not allowed for workspaces with a VCS") {
 		t.Fatalf("expected a VCS error, got: %v", errOutput)
 	}
@@ -198,7 +211,7 @@ func TestRemote_applyWithParallelism(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	op.Parallelism = 3
@@ -214,7 +227,7 @@ func TestRemote_applyWithParallelism(t *testing.T) {
 		t.Fatal("expected apply operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "parallelism values are currently not supported") {
 		t.Fatalf("expected a parallelism error, got: %v", errOutput)
 	}
@@ -224,7 +237,7 @@ func TestRemote_applyWithPlan(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	op.PlanFile = &planfile.Reader{}
@@ -243,7 +256,7 @@ func TestRemote_applyWithPlan(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "saved plan is currently not supported") {
 		t.Fatalf("expected a saved plan error, got: %v", errOutput)
 	}
@@ -253,7 +266,7 @@ func TestRemote_applyWithoutRefresh(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	op.PlanRefresh = false
@@ -269,7 +282,7 @@ func TestRemote_applyWithoutRefresh(t *testing.T) {
 		t.Fatal("expected apply operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "refresh is currently not supported") {
 		t.Fatalf("expected a refresh error, got: %v", errOutput)
 	}
@@ -317,7 +330,7 @@ func TestRemote_applyWithTargetIncompatibleAPIVersion(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	// Set the tfe client's RemoteAPIVersion to an empty string, to mimic
@@ -342,7 +355,7 @@ func TestRemote_applyWithTargetIncompatibleAPIVersion(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "Resource targeting is not supported") {
 		t.Fatalf("expected a targeting error, got: %v", errOutput)
 	}
@@ -352,7 +365,7 @@ func TestRemote_applyWithVariables(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply-variables")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-variables")
 	defer configCleanup()
 
 	op.Variables = testVariables(terraform.ValueFromNamedFile, "foo", "bar")
@@ -368,7 +381,7 @@ func TestRemote_applyWithVariables(t *testing.T) {
 		t.Fatal("expected apply operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "variables are currently not supported") {
 		t.Fatalf("expected a variables error, got: %v", errOutput)
 	}
@@ -378,7 +391,7 @@ func TestRemote_applyNoConfig(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/empty")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/empty")
 	defer configCleanup()
 
 	op.Workspace = backend.DefaultStateName
@@ -396,7 +409,7 @@ func TestRemote_applyNoConfig(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "configuration files found") {
 		t.Fatalf("expected configuration files error, got: %v", errOutput)
 	}
@@ -443,7 +456,7 @@ func TestRemote_applyNoApprove(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 	defer configCleanup()
 
 	input := testInput(t, map[string]string{
@@ -471,7 +484,7 @@ func TestRemote_applyNoApprove(t *testing.T) {
 		t.Fatalf("expected no unused answers, got: %v", input.answers)
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "Apply discarded") {
 		t.Fatalf("expected an apply discarded error, got: %v", errOutput)
 	}
@@ -1042,7 +1055,7 @@ func TestRemote_applyPolicyHardFail(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply-policy-hard-failed")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-policy-hard-failed")
 	defer configCleanup()
 
 	input := testInput(t, map[string]string{
@@ -1070,7 +1083,7 @@ func TestRemote_applyPolicyHardFail(t *testing.T) {
 		t.Fatalf("expected an unused answers, got: %v", input.answers)
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "hard failed") {
 		t.Fatalf("expected a policy check error, got: %v", errOutput)
 	}
@@ -1142,7 +1155,7 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationApply(t, "./testdata/apply-policy-soft-failed")
+	op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-policy-soft-failed")
 	defer configCleanup()
 
 	input := testInput(t, map[string]string{
@@ -1171,7 +1184,7 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) {
 		t.Fatalf("expected an unused answers, got: %v", input.answers)
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "soft failed") {
 		t.Fatalf("expected a policy check error, got: %v", errOutput)
 	}
@@ -1359,7 +1372,7 @@ func TestRemote_applyVersionCheck(t *testing.T) {
 			}
 
 			// RUN: prepare the apply operation and run it
-			op, configCleanup := testOperationApply(t, "./testdata/apply")
+			op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply")
 			defer configCleanup()
 
 			input := testInput(t, map[string]string{
@@ -1384,7 +1397,7 @@ func TestRemote_applyVersionCheck(t *testing.T) {
 				if run.Result != backend.OperationFailure {
 					t.Fatalf("expected run to fail, but result was %#v", run.Result)
 				}
-				errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+				errOutput := playback().Err().Error()
 				if !strings.Contains(errOutput, tc.wantErr) {
 					t.Fatalf("missing error %q\noutput: %s", tc.wantErr, errOutput)
 				}

backend/remote/backend_plan_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/hashicorp/terraform/plans/planfile"
 	"github.com/hashicorp/terraform/states/statemgr"
 	"github.com/hashicorp/terraform/terraform"
+	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/mitchellh/cli"
 )
 
@@ -26,14 +27,26 @@ func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func
 	_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
 
 	return &backend.Operation{
-		ConfigDir:    configDir,
+		ConfigDir:       configDir,
-		ConfigLoader: configLoader,
+		ConfigLoader:    configLoader,
-		Parallelism:  defaultParallelism,
+		Parallelism:     defaultParallelism,
-		PlanRefresh:  true,
+		PlanRefresh:     true,
-		Type:         backend.OperationTypePlan,
+		ShowDiagnostics: testLogDiagnostics(t),
+		Type:            backend.OperationTypePlan,
 	}, configCleanup
 }
 
+func testOperationPlanWithDiagnostics(t *testing.T, configDir string) (*backend.Operation, func(), func() tfdiags.Diagnostics) {
+	t.Helper()
+
+	op, cleanup := testOperationPlan(t, configDir)
+
+	record, playback := testRecordDiagnostics(t)
+	op.ShowDiagnostics = record
+
+	return op, cleanup, playback
+}
+
 func TestRemote_planBasic(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
@@ -148,7 +161,7 @@ func TestRemote_planWithoutPermissions(t *testing.T) {
 	}
 	w.Permissions.CanQueueRun = false
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	op.Workspace = "prod"
@@ -163,7 +176,7 @@ func TestRemote_planWithoutPermissions(t *testing.T) {
 		t.Fatal("expected plan operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "Insufficient rights to generate a plan") {
 		t.Fatalf("expected a permissions error, got: %v", errOutput)
 	}
@@ -173,7 +186,7 @@ func TestRemote_planWithParallelism(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	op.Parallelism = 3
@@ -189,7 +202,7 @@ func TestRemote_planWithParallelism(t *testing.T) {
 		t.Fatal("expected plan operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "parallelism values are currently not supported") {
 		t.Fatalf("expected a parallelism error, got: %v", errOutput)
 	}
@@ -199,7 +212,7 @@ func TestRemote_planWithPlan(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	op.PlanFile = &planfile.Reader{}
@@ -218,7 +231,7 @@ func TestRemote_planWithPlan(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "saved plan is currently not supported") {
 		t.Fatalf("expected a saved plan error, got: %v", errOutput)
 	}
@@ -228,7 +241,7 @@ func TestRemote_planWithPath(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	op.PlanOutPath = "./testdata/plan"
@@ -247,7 +260,7 @@ func TestRemote_planWithPath(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "generated plan is currently not supported") {
 		t.Fatalf("expected a generated plan error, got: %v", errOutput)
 	}
@@ -257,7 +270,7 @@ func TestRemote_planWithoutRefresh(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	op.PlanRefresh = false
@@ -273,7 +286,7 @@ func TestRemote_planWithoutRefresh(t *testing.T) {
 		t.Fatal("expected plan operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "refresh is currently not supported") {
 		t.Fatalf("expected a refresh error, got: %v", errOutput)
 	}
@@ -355,7 +368,7 @@ func TestRemote_planWithTargetIncompatibleAPIVersion(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan")
 	defer configCleanup()
 
 	// Set the tfe client's RemoteAPIVersion to an empty string, to mimic
@@ -380,7 +393,7 @@ func TestRemote_planWithTargetIncompatibleAPIVersion(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "Resource targeting is not supported") {
 		t.Fatalf("expected a targeting error, got: %v", errOutput)
 	}
@@ -390,7 +403,7 @@ func TestRemote_planWithVariables(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan-variables")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-variables")
 	defer configCleanup()
 
 	op.Variables = testVariables(terraform.ValueFromCLIArg, "foo", "bar")
@@ -406,7 +419,7 @@ func TestRemote_planWithVariables(t *testing.T) {
 		t.Fatal("expected plan operation to fail")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "variables are currently not supported") {
 		t.Fatalf("expected a variables error, got: %v", errOutput)
 	}
@@ -416,7 +429,7 @@ func TestRemote_planNoConfig(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/empty")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/empty")
 	defer configCleanup()
 
 	op.Workspace = backend.DefaultStateName
@@ -434,7 +447,7 @@ func TestRemote_planNoConfig(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "configuration files found") {
 		t.Fatalf("expected configuration files error, got: %v", errOutput)
 	}
@@ -875,7 +888,7 @@ func TestRemote_planPolicyHardFail(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan-policy-hard-failed")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-policy-hard-failed")
 	defer configCleanup()
 
 	op.Workspace = backend.DefaultStateName
@@ -893,7 +906,7 @@ func TestRemote_planPolicyHardFail(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "hard failed") {
 		t.Fatalf("expected a policy check error, got: %v", errOutput)
 	}
@@ -914,7 +927,7 @@ func TestRemote_planPolicySoftFail(t *testing.T) {
 	b, bCleanup := testBackendDefault(t)
 	defer bCleanup()
 
-	op, configCleanup := testOperationPlan(t, "./testdata/plan-policy-soft-failed")
+	op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-policy-soft-failed")
 	defer configCleanup()
 
 	op.Workspace = backend.DefaultStateName
@@ -932,7 +945,7 @@ func TestRemote_planPolicySoftFail(t *testing.T) {
 		t.Fatalf("expected plan to be empty")
 	}
 
-	errOutput := b.CLI.(*cli.MockUi).ErrorWriter.String()
+	errOutput := playback().Err().Error()
 	if !strings.Contains(errOutput, "soft failed") {
 		t.Fatalf("expected a policy check error, got: %v", errOutput)
 	}

backend/remote/cli.go

@@ -14,7 +14,6 @@ func (b *Remote) CLIInit(opts *backend.CLIOpts) error {
 
 	b.CLI = opts.CLI
 	b.CLIColor = opts.CLIColor
-	b.ShowDiagnostics = opts.ShowDiagnostics
 	b.ContextOpts = opts.ContextOpts
 
 	return nil

backend/remote/testing.go

@@ -126,13 +126,6 @@ func testBackend(t *testing.T, obj cty.Value) (*Remote, func()) {
 	b.client.Variables = mc.Variables
 	b.client.Workspaces = mc.Workspaces
 
-	b.ShowDiagnostics = func(vals ...interface{}) {
-		var diags tfdiags.Diagnostics
-		for _, diag := range diags.Append(vals...) {
-			b.CLI.Error(diag.Description().Summary)
-		}
-	}
-
 	// Set local to a local test backend.
 	b.local = testLocalBackend(t, b)
 
@@ -163,7 +156,6 @@ func testLocalBackend(t *testing.T, remote *Remote) backend.Enhanced {
 	b := backendLocal.NewWithBackend(remote)
 
 	b.CLI = remote.CLI
-	b.ShowDiagnostics = remote.ShowDiagnostics
 
 	// Add a test provider to the local backend.
 	p := backendLocal.TestLocalProvider(t, b, "null", &terraform.ProviderSchema{
@@ -307,3 +299,43 @@ func testVariables(s terraform.ValueSourceType, vs ...string) map[string]backend
 	}
 	return vars
 }
+
+// testRecordDiagnostics allows tests to record and later inspect diagnostics
+// emitted during an Operation. It returns a record function which can be set
+// as the ShowDiagnostics value of an Operation, and a playback function which
+// returns the recorded diagnostics for inspection.
+func testRecordDiagnostics(t *testing.T) (record func(vals ...interface{}), playback func() tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	record = func(vals ...interface{}) {
+		diags = diags.Append(vals...)
+	}
+	playback = func() tfdiags.Diagnostics {
+		diags.Sort()
+		return diags
+	}
+	return
+}
+
+// testLogDiagnostics returns a function which can be used as the
+// ShowDiagnostics value for an Operation, in order to help debugging during
+// tests. Any calls to this function result in test logs.
+func testLogDiagnostics(t *testing.T) func(vals ...interface{}) {
+	return func(vals ...interface{}) {
+		var diags tfdiags.Diagnostics
+		diags = diags.Append(vals...)
+		diags.Sort()
+
+		for _, diag := range diags {
+			// NOTE: Since the caller here is not directly the TestLocal
+			// function, t.Helper doesn't apply and so the log source
+			// isn't correctly shown in the test log output. This seems
+			// unavoidable as long as this is happening so indirectly.
+			desc := diag.Description()
+			if desc.Detail != "" {
+				t.Logf("%s: %s", desc.Summary, desc.Detail)
+			} else {
+				t.Log(desc.Summary)
+			}
+		}
+	}
+}

command/apply.go

@@ -173,6 +173,7 @@ func (c *ApplyCommand) Run(args []string) int {
 	opReq.Destroy = c.Destroy
 	opReq.PlanFile = planFile
 	opReq.PlanRefresh = refresh
+	opReq.ShowDiagnostics = c.showDiagnostics
 	opReq.Type = backend.OperationTypeApply
 
 	opReq.ConfigLoader, err = c.initConfigLoader()

command/meta_backend.go

@@ -309,7 +309,6 @@ func (m *Meta) backendCLIOpts() (*backend.CLIOpts, error) {
 		CLI:                 m.Ui,
 		CLIColor:            m.Colorize(),
 		Streams:             m.Streams,
-		ShowDiagnostics:     m.showDiagnostics,
 		StatePath:           m.statePath,
 		StateOutPath:        m.stateOutPath,
 		StateBackupPath:     m.backupPath,

command/plan.go

@@ -84,6 +84,7 @@ func (c *PlanCommand) Run(args []string) int {
 	opReq.Destroy = destroy
 	opReq.PlanOutPath = outPath
 	opReq.PlanRefresh = refresh
+	opReq.ShowDiagnostics = c.showDiagnostics
 	opReq.Type = backend.OperationTypePlan
 
 	opReq.ConfigLoader, err = c.initConfigLoader()

command/refresh.go

@@ -75,6 +75,7 @@ func (c *RefreshCommand) Run(args []string) int {
 	// Build the operation
 	opReq := c.Operation(b)
 	opReq.ConfigDir = configPath
+	opReq.ShowDiagnostics = c.showDiagnostics
 	opReq.Type = backend.OperationTypeRefresh
 
 	opReq.ConfigLoader, err = c.initConfigLoader()