From 7c0ec0107fa8d04b29edbeb4026b6388173b266e Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Thu, 25 Feb 2021 10:02:23 -0500 Subject: [PATCH] backend: Replace ShowDiagnostics with view.Diagnostics --- backend/backend.go | 11 +- backend/cli.go | 4 - backend/local/backend_apply.go | 6 +- backend/local/backend_apply_test.go | 33 +++--- backend/local/backend_plan.go | 6 +- backend/local/backend_plan_test.go | 28 ++--- backend/local/backend_refresh.go | 2 +- backend/local/backend_refresh_test.go | 24 ++--- backend/local/testing.go | 41 ------- backend/remote/backend_apply_test.go | 150 ++++++++++++++------------ backend/remote/backend_plan_test.go | 139 +++++++++++++----------- backend/remote/testing.go | 40 ------- command/apply.go | 11 -- command/plan.go | 6 -- command/refresh.go | 6 -- 15 files changed, 208 insertions(+), 299 deletions(-) diff --git a/backend/backend.go b/backend/backend.go index 6b801d104..801e5c465 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -216,9 +216,6 @@ type Operation struct { UIIn terraform.UIInput UIOut terraform.UIOutput - // ShowDiagnostics prints diagnostic messages to the UI. - ShowDiagnostics func(vals ...interface{}) - // StateLocker is used to lock the state while providing UI feedback to the // user. This will be replaced by the Backend to update the context. // @@ -253,7 +250,7 @@ func (o *Operation) Config() (*configs.Config, tfdiags.Diagnostics) { // // 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 +// backend.OperationSuccess otherwise. It will then use o.View.Diagnostics // to show the given diagnostics before returning. // // Callers should feel free to do each of these operations separately in @@ -266,14 +263,14 @@ func (o *Operation) ReportResult(op *RunningOperation, diags tfdiags.Diagnostics } else { op.Result = OperationSuccess } - if o.ShowDiagnostics != nil { - o.ShowDiagnostics(diags) + if o.View != nil { + o.View.Diagnostics(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", + "[ERROR] Backend needs to report diagnostics but View is not set:\n%s", diags.ErrWithWarnings(), ) } diff --git a/backend/cli.go b/backend/cli.go index 80313c39c..454473b2a 100644 --- a/backend/cli.go +++ b/backend/cli.go @@ -56,10 +56,6 @@ type CLIOpts struct { // for tailoring the output to fit the attached terminal, for example. Streams *terminal.Streams - // ShowDiagnostics is a function that will format and print diagnostic - // messages to the UI. - ShowDiagnostics func(vals ...interface{}) - // StatePath is the local path where state is read from. // // StateOutPath is the local path where the state will be written. diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 4baba83fb..1817dcb40 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -53,7 +53,7 @@ func (b *Local) opApply( defer func() { diags := op.StateLocker.Unlock() if diags.HasErrors() { - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) runningOp.Result = backend.OperationFailure } }() @@ -101,7 +101,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 { - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) diags = nil // reset so we won't show the same diagnostics again later } @@ -168,7 +168,7 @@ func (b *Local) opApply( // 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. - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) } // backupStateForError is called in a scenario where we're unable to persist the diff --git a/backend/local/backend_apply_test.go b/backend/local/backend_apply_test.go index 88df036f9..d220f13a3 100644 --- a/backend/local/backend_apply_test.go +++ b/backend/local/backend_apply_test.go @@ -101,8 +101,8 @@ func TestLocal_applyEmptyDir(t *testing.T) { // the backend should be unlocked after a run assertBackendStateUnlocked(t, b) - if errOutput := done(t).Stderr(); errOutput != "" { - t.Fatalf("unexpected error output:\n%s", errOutput) + if got, want := done(t).Stderr(), "Error: No configuration files"; !strings.Contains(got, want) { + t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want) } } @@ -165,7 +165,7 @@ func TestLocal_applyError(t *testing.T) { ami := r.Config.GetAttr("ami").AsString() if !errored && ami == "error" { errored = true - diags = diags.Append(errors.New("error")) + diags = diags.Append(errors.New("ami error")) return providers.ApplyResourceChangeResponse{ Diagnostics: diags, } @@ -201,8 +201,8 @@ test_instance.foo: // the backend should be unlocked after a run assertBackendStateUnlocked(t, b) - if errOutput := done(t).Stderr(); errOutput != "" { - t.Fatalf("unexpected error output:\n%s", errOutput) + if got, want := done(t).Stderr(), "Error: ami error"; !strings.Contains(got, want) { + t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want) } } @@ -229,9 +229,6 @@ func TestLocal_applyBackendFail(t *testing.T) { op, configCleanup, done := testOperationApply(t, wd+"/testdata/apply") defer configCleanup() - record, playback := testRecordDiagnostics(t) - op.ShowDiagnostics = record - b.Backend = &backendWithFailingState{} run, err := b.Operation(context.Background(), op) @@ -239,11 +236,14 @@ func TestLocal_applyBackendFail(t *testing.T) { t.Fatalf("bad: %s", err) } <-run.Done() + + output := done(t) + if run.Result == backend.OperationSuccess { t.Fatalf("apply succeeded; want error") } - diagErr := playback().Err().Error() + diagErr := output.Stderr() if !strings.Contains(diagErr, "Error saving state: fake failure") { t.Fatalf("missing \"fake failure\" message in diags:\n%s", diagErr) } @@ -259,10 +259,6 @@ test_instance.foo: // the backend should be unlocked after a run assertBackendStateUnlocked(t, b) - - if errOutput := done(t).Stderr(); errOutput != "" { - t.Fatalf("unexpected error output:\n%s", errOutput) - } } func TestLocal_applyRefreshFalse(t *testing.T) { @@ -320,12 +316,11 @@ func testOperationApply(t *testing.T, configDir string) (*backend.Operation, fun view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) return &backend.Operation{ - Type: backend.OperationTypeApply, - ConfigDir: configDir, - ConfigLoader: configLoader, - ShowDiagnostics: testLogDiagnostics(t), - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypeApply, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, }, configCleanup, done } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index abcc20daa..f4d46dd37 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -64,7 +64,7 @@ func (b *Local) opPlan( defer func() { diags := op.StateLocker.Unlock() if diags.HasErrors() { - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) runningOp.Result = backend.OperationFailure } }() @@ -135,7 +135,7 @@ func (b *Local) opPlan( op.View.PlanNoChanges() // Even if there are no changes, there still could be some warnings - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) return } @@ -145,7 +145,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. - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) op.View.PlanNextStep(op.PlanOutPath) } diff --git a/backend/local/backend_plan_test.go b/backend/local/backend_plan_test.go index a01f284cc..66ccb9e0a 100644 --- a/backend/local/backend_plan_test.go +++ b/backend/local/backend_plan_test.go @@ -90,8 +90,6 @@ func TestLocal_planNoConfig(t *testing.T) { TestLocalProvider(t, b, "test", &terraform.ProviderSchema{}) op, configCleanup, done := testOperationPlan(t, "./testdata/empty") - record, playback := testRecordDiagnostics(t) - op.ShowDiagnostics = record defer configCleanup() op.PlanRefresh = true @@ -101,21 +99,18 @@ func TestLocal_planNoConfig(t *testing.T) { } <-run.Done() + output := done(t) + if run.Result == backend.OperationSuccess { t.Fatal("plan operation succeeded; want failure") } - output := playback().Err().Error() - if !strings.Contains(output, "No configuration files") { - t.Fatalf("bad: %s", err) + if stderr := output.Stderr(); !strings.Contains(stderr, "No configuration files") { + t.Fatalf("bad: %s", stderr) } // the backend should be unlocked after a run assertBackendStateUnlocked(t, b) - - if errOutput := done(t).Stderr(); errOutput != "" { - t.Fatalf("unexpected error output:\n%s", errOutput) - } } // This test validates the state lacking behavior when the inner call to @@ -141,8 +136,8 @@ func TestLocal_plan_context_error(t *testing.T) { // the backend should be unlocked after a run assertBackendStateUnlocked(t, b) - if errOutput := done(t).Stderr(); errOutput != "" { - t.Fatalf("unexpected error output:\n%s", errOutput) + if got, want := done(t).Stderr(), "Error: Could not load plugin"; !strings.Contains(got, want) { + t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want) } } @@ -723,12 +718,11 @@ func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) return &backend.Operation{ - Type: backend.OperationTypePlan, - ConfigDir: configDir, - ConfigLoader: configLoader, - ShowDiagnostics: testLogDiagnostics(t), - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypePlan, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, }, configCleanup, done } diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index ec3f9bde1..54ecc07b0 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -57,7 +57,7 @@ func (b *Local) opRefresh( defer func() { diags := op.StateLocker.Unlock() if diags.HasErrors() { - op.ShowDiagnostics(diags) + op.View.Diagnostics(diags) runningOp.Result = backend.OperationFailure } }() diff --git a/backend/local/backend_refresh_test.go b/backend/local/backend_refresh_test.go index 91c8087b4..bef5c5d01 100644 --- a/backend/local/backend_refresh_test.go +++ b/backend/local/backend_refresh_test.go @@ -238,10 +238,6 @@ func TestLocal_refreshEmptyState(t *testing.T) { op, configCleanup, done := testOperationRefresh(t, "./testdata/refresh") defer configCleanup() - defer done(t) - - record, playback := testRecordDiagnostics(t) - op.ShowDiagnostics = record run, err := b.Operation(context.Background(), op) if err != nil { @@ -249,11 +245,12 @@ func TestLocal_refreshEmptyState(t *testing.T) { } <-run.Done() - diags := playback() - if diags.HasErrors() { - t.Fatalf("expected only warning diags, got errors: %s", diags.Err()) + output := done(t) + + if stderr := output.Stderr(); stderr != "" { + t.Fatalf("expected only warning diags, got errors: %s", stderr) } - if got, want := diags.ErrWithWarnings().Error(), "Empty or non-existent state"; !strings.Contains(got, want) { + if got, want := output.Stdout(), "Warning: Empty or non-existent state"; !strings.Contains(got, want) { t.Errorf("wrong diags\n got: %s\nwant: %s", got, want) } @@ -270,12 +267,11 @@ func testOperationRefresh(t *testing.T, configDir string) (*backend.Operation, f view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) return &backend.Operation{ - Type: backend.OperationTypeRefresh, - ConfigDir: configDir, - ConfigLoader: configLoader, - ShowDiagnostics: testLogDiagnostics(t), - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypeRefresh, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, }, configCleanup, done } diff --git a/backend/local/testing.go b/backend/local/testing.go index 941bf3c13..87e57fc32 100644 --- a/backend/local/testing.go +++ b/backend/local/testing.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states/statemgr" "github.com/hashicorp/terraform/terraform" - "github.com/hashicorp/terraform/tfdiags" ) // TestLocal returns a configured Local struct with temporary paths and @@ -245,43 +244,3 @@ 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) - } - } - } -} diff --git a/backend/remote/backend_apply_test.go b/backend/remote/backend_apply_test.go index fae4b476b..8a419a573 100644 --- a/backend/remote/backend_apply_test.go +++ b/backend/remote/backend_apply_test.go @@ -22,53 +22,44 @@ 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" ) -func testOperationApply(t *testing.T, configDir string) (*backend.Operation, func()) { +func testOperationApply(t *testing.T, configDir string) (*backend.Operation, func(), func(*testing.T) *terminal.TestOutput) { t.Helper() return testOperationApplyWithTimeout(t, configDir, 0) } -func testOperationApplyWithTimeout(t *testing.T, configDir string, timeout time.Duration) (*backend.Operation, func()) { +func testOperationApplyWithTimeout(t *testing.T, configDir string, timeout time.Duration) (*backend.Operation, func(), func(*testing.T) *terminal.TestOutput) { t.Helper() _, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir) - streams, _ := terminal.StreamsForTesting(t) - view := views.NewStateLocker(arguments.ViewHuman, views.NewView(streams)) + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + stateLockerView := views.NewStateLocker(arguments.ViewHuman, view) + operationView := views.NewOperation(arguments.ViewHuman, false, view) return &backend.Operation{ - ConfigDir: configDir, - ConfigLoader: configLoader, - Parallelism: defaultParallelism, - PlanRefresh: true, - ShowDiagnostics: testLogDiagnostics(t), - StateLocker: clistate.NewLocker(timeout, view), - 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 + ConfigDir: configDir, + ConfigLoader: configLoader, + Parallelism: defaultParallelism, + PlanRefresh: true, + StateLocker: clistate.NewLocker(timeout, stateLockerView), + Type: backend.OperationTypeApply, + View: operationView, + }, configCleanup, done } func TestRemote_applyBasic(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -117,8 +108,9 @@ func TestRemote_applyCanceled(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -158,7 +150,7 @@ func TestRemote_applyWithoutPermissions(t *testing.T) { } w.Permissions.CanQueueApply = false - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() op.UIOut = b.CLI @@ -170,11 +162,12 @@ func TestRemote_applyWithoutPermissions(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "Insufficient rights to apply changes") { t.Fatalf("expected a permissions error, got: %v", errOutput) } @@ -197,7 +190,7 @@ func TestRemote_applyWithVCS(t *testing.T) { t.Fatalf("error creating named workspace: %v", err) } - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() op.Workspace = "prod" @@ -208,6 +201,7 @@ func TestRemote_applyWithVCS(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -215,7 +209,7 @@ func TestRemote_applyWithVCS(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "not allowed for workspaces with a VCS") { t.Fatalf("expected a VCS error, got: %v", errOutput) } @@ -225,7 +219,7 @@ func TestRemote_applyWithParallelism(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() op.Parallelism = 3 @@ -237,11 +231,12 @@ func TestRemote_applyWithParallelism(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "parallelism values are currently not supported") { t.Fatalf("expected a parallelism error, got: %v", errOutput) } @@ -251,7 +246,7 @@ func TestRemote_applyWithPlan(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() op.PlanFile = &planfile.Reader{} @@ -263,6 +258,7 @@ func TestRemote_applyWithPlan(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -270,7 +266,7 @@ func TestRemote_applyWithPlan(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "saved plan is currently not supported") { t.Fatalf("expected a saved plan error, got: %v", errOutput) } @@ -280,7 +276,7 @@ func TestRemote_applyWithoutRefresh(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() op.PlanRefresh = false @@ -292,11 +288,12 @@ func TestRemote_applyWithoutRefresh(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "refresh is currently not supported") { t.Fatalf("expected a refresh error, got: %v", errOutput) } @@ -306,8 +303,9 @@ func TestRemote_applyWithTarget(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) addr, _ := addrs.ParseAbsResourceStr("null_resource.foo") @@ -344,7 +342,7 @@ func TestRemote_applyWithTargetIncompatibleAPIVersion(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() // Set the tfe client's RemoteAPIVersion to an empty string, to mimic @@ -362,6 +360,7 @@ func TestRemote_applyWithTargetIncompatibleAPIVersion(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -369,7 +368,7 @@ func TestRemote_applyWithTargetIncompatibleAPIVersion(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "Resource targeting is not supported") { t.Fatalf("expected a targeting error, got: %v", errOutput) } @@ -379,7 +378,7 @@ func TestRemote_applyWithVariables(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-variables") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-variables") defer configCleanup() op.Variables = testVariables(terraform.ValueFromNamedFile, "foo", "bar") @@ -391,11 +390,12 @@ func TestRemote_applyWithVariables(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "variables are currently not supported") { t.Fatalf("expected a variables error, got: %v", errOutput) } @@ -405,7 +405,7 @@ func TestRemote_applyNoConfig(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/empty") + op, configCleanup, done := testOperationApply(t, "./testdata/empty") defer configCleanup() op.Workspace = backend.DefaultStateName @@ -416,6 +416,7 @@ func TestRemote_applyNoConfig(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -423,7 +424,7 @@ func TestRemote_applyNoConfig(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "configuration files found") { t.Fatalf("expected configuration files error, got: %v", errOutput) } @@ -439,8 +440,9 @@ func TestRemote_applyNoChanges(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply-no-changes") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-no-changes") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -470,7 +472,7 @@ func TestRemote_applyNoApprove(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() input := testInput(t, map[string]string{ @@ -487,6 +489,7 @@ func TestRemote_applyNoApprove(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -498,7 +501,7 @@ func TestRemote_applyNoApprove(t *testing.T) { t.Fatalf("expected no unused answers, got: %v", input.answers) } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "Apply discarded") { t.Fatalf("expected an apply discarded error, got: %v", errOutput) } @@ -508,8 +511,9 @@ func TestRemote_applyAutoApprove(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "no", @@ -553,8 +557,9 @@ func TestRemote_applyApprovedExternally(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "wait-for-external-update", @@ -628,8 +633,9 @@ func TestRemote_applyDiscardedExternally(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "wait-for-external-update", @@ -716,8 +722,9 @@ func TestRemote_applyWithAutoApply(t *testing.T) { t.Fatalf("error creating named workspace: %v", err) } - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -767,8 +774,9 @@ func TestRemote_applyForceLocal(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -829,8 +837,9 @@ func TestRemote_applyWorkspaceWithoutOperations(t *testing.T) { t.Fatalf("error creating named workspace: %v", err) } - op, configCleanup := testOperationApply(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -900,8 +909,9 @@ func TestRemote_applyLockTimeout(t *testing.T) { t.Fatalf("error creating pending run: %v", err) } - op, configCleanup := testOperationApplyWithTimeout(t, "./testdata/apply", 50*time.Millisecond) + op, configCleanup, done := testOperationApplyWithTimeout(t, "./testdata/apply", 50*time.Millisecond) defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "cancel": "yes", @@ -950,8 +960,9 @@ func TestRemote_applyDestroy(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply-destroy") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-destroy") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -999,8 +1010,9 @@ func TestRemote_applyDestroyNoConfig(t *testing.T) { "approve": "yes", }) - op, configCleanup := testOperationApply(t, "./testdata/empty") + op, configCleanup, done := testOperationApply(t, "./testdata/empty") defer configCleanup() + defer done(t) op.Destroy = true op.UIIn = input @@ -1029,8 +1041,9 @@ func TestRemote_applyPolicyPass(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply-policy-passed") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-policy-passed") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -1076,7 +1089,7 @@ func TestRemote_applyPolicyHardFail(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-policy-hard-failed") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-policy-hard-failed") defer configCleanup() input := testInput(t, map[string]string{ @@ -1093,6 +1106,7 @@ func TestRemote_applyPolicyHardFail(t *testing.T) { } <-run.Done() + viewOutput := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -1104,7 +1118,7 @@ func TestRemote_applyPolicyHardFail(t *testing.T) { t.Fatalf("expected an unused answers, got: %v", input.answers) } - errOutput := playback().Err().Error() + errOutput := viewOutput.Stderr() if !strings.Contains(errOutput, "hard failed") { t.Fatalf("expected a policy check error, got: %v", errOutput) } @@ -1128,8 +1142,9 @@ func TestRemote_applyPolicySoftFail(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply-policy-soft-failed") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-policy-soft-failed") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "override": "override", @@ -1176,7 +1191,7 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply-policy-soft-failed") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-policy-soft-failed") defer configCleanup() input := testInput(t, map[string]string{ @@ -1194,6 +1209,7 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { } <-run.Done() + viewOutput := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected apply operation to fail") } @@ -1205,7 +1221,7 @@ func TestRemote_applyPolicySoftFailAutoApprove(t *testing.T) { t.Fatalf("expected an unused answers, got: %v", input.answers) } - errOutput := playback().Err().Error() + errOutput := viewOutput.Stderr() if !strings.Contains(errOutput, "soft failed") { t.Fatalf("expected a policy check error, got: %v", errOutput) } @@ -1242,8 +1258,9 @@ func TestRemote_applyPolicySoftFailAutoApply(t *testing.T) { t.Fatalf("error creating named workspace: %v", err) } - op, configCleanup := testOperationApply(t, "./testdata/apply-policy-soft-failed") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-policy-soft-failed") defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "override": "override", @@ -1290,8 +1307,9 @@ func TestRemote_applyWithRemoteError(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationApply(t, "./testdata/apply-with-error") + op, configCleanup, done := testOperationApply(t, "./testdata/apply-with-error") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -1393,13 +1411,12 @@ func TestRemote_applyVersionCheck(t *testing.T) { } // RUN: prepare the apply operation and run it - op, configCleanup, playback := testOperationApplyWithDiagnostics(t, "./testdata/apply") + op, configCleanup, done := testOperationApply(t, "./testdata/apply") defer configCleanup() streams, done := terminal.StreamsForTesting(t) view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) op.View = view - defer done(t) input := testInput(t, map[string]string{ "approve": "yes", @@ -1416,6 +1433,7 @@ func TestRemote_applyVersionCheck(t *testing.T) { // RUN: wait for completion <-run.Done() + output := done(t) if tc.wantErr != "" { // ASSERT: if the test case wants an error, check for failure @@ -1423,7 +1441,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 := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, tc.wantErr) { t.Fatalf("missing error %q\noutput: %s", tc.wantErr, errOutput) } diff --git a/backend/remote/backend_plan_test.go b/backend/remote/backend_plan_test.go index 54925fb32..ac33ce76e 100644 --- a/backend/remote/backend_plan_test.go +++ b/backend/remote/backend_plan_test.go @@ -21,52 +21,43 @@ 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" ) -func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func()) { +func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func(), func(*testing.T) *terminal.TestOutput) { t.Helper() return testOperationPlanWithTimeout(t, configDir, 0) } -func testOperationPlanWithTimeout(t *testing.T, configDir string, timeout time.Duration) (*backend.Operation, func()) { +func testOperationPlanWithTimeout(t *testing.T, configDir string, timeout time.Duration) (*backend.Operation, func(), func(*testing.T) *terminal.TestOutput) { t.Helper() _, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir) - streams, _ := terminal.StreamsForTesting(t) - view := views.NewStateLocker(arguments.ViewHuman, views.NewView(streams)) + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + stateLockerView := views.NewStateLocker(arguments.ViewHuman, view) + operationView := views.NewOperation(arguments.ViewHuman, false, view) return &backend.Operation{ - ConfigDir: configDir, - ConfigLoader: configLoader, - Parallelism: defaultParallelism, - PlanRefresh: true, - ShowDiagnostics: testLogDiagnostics(t), - StateLocker: clistate.NewLocker(timeout, view), - 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 + ConfigDir: configDir, + ConfigLoader: configLoader, + Parallelism: defaultParallelism, + PlanRefresh: true, + StateLocker: clistate.NewLocker(timeout, stateLockerView), + Type: backend.OperationTypePlan, + View: operationView, + }, configCleanup, done } func TestRemote_planBasic(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -102,8 +93,9 @@ func TestRemote_planCanceled(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -131,8 +123,9 @@ func TestRemote_planLongLine(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan-long-line") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-long-line") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -175,7 +168,7 @@ func TestRemote_planWithoutPermissions(t *testing.T) { } w.Permissions.CanQueueRun = false - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() op.Workspace = "prod" @@ -186,11 +179,12 @@ func TestRemote_planWithoutPermissions(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "Insufficient rights to generate a plan") { t.Fatalf("expected a permissions error, got: %v", errOutput) } @@ -200,7 +194,7 @@ func TestRemote_planWithParallelism(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() op.Parallelism = 3 @@ -212,11 +206,12 @@ func TestRemote_planWithParallelism(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "parallelism values are currently not supported") { t.Fatalf("expected a parallelism error, got: %v", errOutput) } @@ -226,7 +221,7 @@ func TestRemote_planWithPlan(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() op.PlanFile = &planfile.Reader{} @@ -238,6 +233,7 @@ func TestRemote_planWithPlan(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -245,7 +241,7 @@ func TestRemote_planWithPlan(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "saved plan is currently not supported") { t.Fatalf("expected a saved plan error, got: %v", errOutput) } @@ -255,7 +251,7 @@ func TestRemote_planWithPath(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() op.PlanOutPath = "./testdata/plan" @@ -267,6 +263,7 @@ func TestRemote_planWithPath(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -274,7 +271,7 @@ func TestRemote_planWithPath(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "generated plan is currently not supported") { t.Fatalf("expected a generated plan error, got: %v", errOutput) } @@ -284,7 +281,7 @@ func TestRemote_planWithoutRefresh(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() op.PlanRefresh = false @@ -296,11 +293,12 @@ func TestRemote_planWithoutRefresh(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "refresh is currently not supported") { t.Fatalf("expected a refresh error, got: %v", errOutput) } @@ -333,8 +331,9 @@ func TestRemote_planWithTarget(t *testing.T) { } } - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) addr, _ := addrs.ParseAbsResourceStr("null_resource.foo") @@ -382,7 +381,7 @@ func TestRemote_planWithTargetIncompatibleAPIVersion(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() // Set the tfe client's RemoteAPIVersion to an empty string, to mimic @@ -400,6 +399,7 @@ func TestRemote_planWithTargetIncompatibleAPIVersion(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -407,7 +407,7 @@ func TestRemote_planWithTargetIncompatibleAPIVersion(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "Resource targeting is not supported") { t.Fatalf("expected a targeting error, got: %v", errOutput) } @@ -417,7 +417,7 @@ func TestRemote_planWithVariables(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-variables") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-variables") defer configCleanup() op.Variables = testVariables(terraform.ValueFromCLIArg, "foo", "bar") @@ -429,11 +429,12 @@ func TestRemote_planWithVariables(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "variables are currently not supported") { t.Fatalf("expected a variables error, got: %v", errOutput) } @@ -443,7 +444,7 @@ func TestRemote_planNoConfig(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/empty") + op, configCleanup, done := testOperationPlan(t, "./testdata/empty") defer configCleanup() op.Workspace = backend.DefaultStateName @@ -454,6 +455,7 @@ func TestRemote_planNoConfig(t *testing.T) { } <-run.Done() + output := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -461,7 +463,7 @@ func TestRemote_planNoConfig(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := output.Stderr() if !strings.Contains(errOutput, "configuration files found") { t.Fatalf("expected configuration files error, got: %v", errOutput) } @@ -471,8 +473,9 @@ func TestRemote_planNoChanges(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan-no-changes") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-no-changes") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -509,8 +512,9 @@ func TestRemote_planForceLocal(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -544,8 +548,9 @@ func TestRemote_planWithoutOperationsEntitlement(t *testing.T) { b, bCleanup := testBackendNoOperations(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -593,8 +598,9 @@ func TestRemote_planWorkspaceWithoutOperations(t *testing.T) { t.Fatalf("error creating named workspace: %v", err) } - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = "no-operations" @@ -651,8 +657,9 @@ func TestRemote_planLockTimeout(t *testing.T) { t.Fatalf("error creating pending run: %v", err) } - op, configCleanup := testOperationPlanWithTimeout(t, "./testdata/plan", 50) + op, configCleanup, done := testOperationPlanWithTimeout(t, "./testdata/plan", 50) defer configCleanup() + defer done(t) input := testInput(t, map[string]string{ "cancel": "yes", @@ -698,8 +705,9 @@ func TestRemote_planDestroy(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Destroy = true op.Workspace = backend.DefaultStateName @@ -722,8 +730,9 @@ func TestRemote_planDestroyNoConfig(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/empty") + op, configCleanup, done := testOperationPlan(t, "./testdata/empty") defer configCleanup() + defer done(t) op.Destroy = true op.Workspace = backend.DefaultStateName @@ -756,8 +765,9 @@ func TestRemote_planWithWorkingDirectory(t *testing.T) { t.Fatalf("error configuring working directory: %v", err) } - op, configCleanup := testOperationPlan(t, "./testdata/plan-with-working-directory/terraform") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-with-working-directory/terraform") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -814,8 +824,9 @@ func TestRemote_planWithWorkingDirectoryFromCurrentPath(t *testing.T) { // For this test we need to give our current directory instead of the // full path to the configuration as we already changed directories. - op, configCleanup := testOperationPlan(t, ".") + op, configCleanup, done := testOperationPlan(t, ".") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -845,8 +856,9 @@ func TestRemote_planCostEstimation(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan-cost-estimation") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-cost-estimation") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -879,8 +891,9 @@ func TestRemote_planPolicyPass(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan-policy-passed") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-policy-passed") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -913,7 +926,7 @@ func TestRemote_planPolicyHardFail(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-policy-hard-failed") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-policy-hard-failed") defer configCleanup() op.Workspace = backend.DefaultStateName @@ -924,6 +937,7 @@ func TestRemote_planPolicyHardFail(t *testing.T) { } <-run.Done() + viewOutput := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -931,7 +945,7 @@ func TestRemote_planPolicyHardFail(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := viewOutput.Stderr() if !strings.Contains(errOutput, "hard failed") { t.Fatalf("expected a policy check error, got: %v", errOutput) } @@ -952,7 +966,7 @@ func TestRemote_planPolicySoftFail(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup, playback := testOperationPlanWithDiagnostics(t, "./testdata/plan-policy-soft-failed") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-policy-soft-failed") defer configCleanup() op.Workspace = backend.DefaultStateName @@ -963,6 +977,7 @@ func TestRemote_planPolicySoftFail(t *testing.T) { } <-run.Done() + viewOutput := done(t) if run.Result == backend.OperationSuccess { t.Fatal("expected plan operation to fail") } @@ -970,7 +985,7 @@ func TestRemote_planPolicySoftFail(t *testing.T) { t.Fatalf("expected plan to be empty") } - errOutput := playback().Err().Error() + errOutput := viewOutput.Stderr() if !strings.Contains(errOutput, "soft failed") { t.Fatalf("expected a policy check error, got: %v", errOutput) } @@ -991,8 +1006,9 @@ func TestRemote_planWithRemoteError(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan-with-error") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan-with-error") defer configCleanup() + defer done(t) op.Workspace = backend.DefaultStateName @@ -1022,8 +1038,9 @@ func TestRemote_planOtherError(t *testing.T) { b, bCleanup := testBackendDefault(t) defer bCleanup() - op, configCleanup := testOperationPlan(t, "./testdata/plan") + op, configCleanup, done := testOperationPlan(t, "./testdata/plan") defer configCleanup() + defer done(t) op.Workspace = "network-error" // custom error response in backend_mock.go diff --git a/backend/remote/testing.go b/backend/remote/testing.go index 035cdff88..82e201e32 100644 --- a/backend/remote/testing.go +++ b/backend/remote/testing.go @@ -297,43 +297,3 @@ 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) - } - } - } -} diff --git a/command/apply.go b/command/apply.go index d0892f760..250884ad2 100644 --- a/command/apply.go +++ b/command/apply.go @@ -252,17 +252,6 @@ func (c *ApplyCommand) OperationRequest( opReq.Type = backend.OperationTypeApply opReq.View = view.Operation() - // FIXME: To allow errors to be easily rendered, the showDiagnostics method - // accepts ...interface{}. The backend no longer needs this, as only - // tfdiags.Diagnostics values are used. Once we have migrated plan and refresh - // to use views, we can remove ShowDiagnostics and update the backend code to - // call view.Diagnostics() instead. - opReq.ShowDiagnostics = func(vals ...interface{}) { - var diags tfdiags.Diagnostics - diags = diags.Append(vals...) - view.Diagnostics(diags) - } - var err error opReq.ConfigLoader, err = c.initConfigLoader() if err != nil { diff --git a/command/plan.go b/command/plan.go index 8bb82588d..1e9aadf38 100644 --- a/command/plan.go +++ b/command/plan.go @@ -147,12 +147,6 @@ func (c *PlanCommand) OperationRequest( opReq.Targets = args.Targets opReq.Type = backend.OperationTypePlan opReq.View = view.Operation() - // FIXME: this shim is needed until the remote backend is migrated to views - opReq.ShowDiagnostics = func(vals ...interface{}) { - var diags tfdiags.Diagnostics - diags = diags.Append(vals...) - view.Diagnostics(diags) - } var err error opReq.ConfigLoader, err = c.initConfigLoader() diff --git a/command/refresh.go b/command/refresh.go index 3fa42833f..abff46601 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -135,12 +135,6 @@ func (c *RefreshCommand) OperationRequest(be backend.Enhanced, view views.Refres opReq.Targets = args.Targets opReq.Type = backend.OperationTypeRefresh opReq.View = view.Operation() - // FIXME: this shim is needed until the remote backend is migrated to views - opReq.ShowDiagnostics = func(vals ...interface{}) { - var diags tfdiags.Diagnostics - diags = diags.Append(vals...) - view.Diagnostics(diags) - } var err error opReq.ConfigLoader, err = c.initConfigLoader()