backend/local: TestLocal_plan_context_error to fail terraform.NewContext
The original intent of this test was to verify that we properly release the state lock if terraform.NewContext fails. This was in response to a bug in an earlier version of Terraform where that wasn't true. In the recent refactoring that made terraform.NewContext no longer responsible for provider constraint/checksum verification, this test began testing a failed plan operation instead, which left the error return path from terraform.NewContext untested. An invalid parallelism value is the one remaining case where terraform.NewContext can return an error, so as a localized fix for this test I've switched it to just intentionally set an invalid parallelism value. This is still not ideal because it's still testing an implementation detail, but I've at least left a comment inline to try to be clearer about what the goal is here so that we can respond in a more appropriate way if future changes cause this test to fail again. In the long run I'd like to move this last remaining check out to be the responsibility of the CLI layer, with terraform.NewContext either just assuming the value correct or panicking when it isn't, but the handling of this CLI option is currently rather awkwardly spread across the command and backend packages so we'll save that refactoring for a later date.
This commit is contained in:
parent
718fa3895f
commit
332ea1f233
|
@ -119,9 +119,24 @@ func TestLocal_plan_context_error(t *testing.T) {
|
||||||
b, cleanup := TestLocal(t)
|
b, cleanup := TestLocal(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
|
// This is an intentionally-invalid value to make terraform.NewContext fail
|
||||||
|
// when b.Operation calls it.
|
||||||
|
// NOTE: This test was originally using a provider initialization failure
|
||||||
|
// as its forced error condition, but terraform.NewContext is no longer
|
||||||
|
// responsible for checking that. Invalid parallelism is the last situation
|
||||||
|
// where terraform.NewContext can return error diagnostics, and arguably
|
||||||
|
// we should be validating this argument at the UI layer anyway, so perhaps
|
||||||
|
// in future we'll make terraform.NewContext never return errors and then
|
||||||
|
// this test will become redundant, because its purpose is specifically
|
||||||
|
// to test that we properly unlock the state if terraform.NewContext
|
||||||
|
// returns an error.
|
||||||
|
if b.ContextOpts == nil {
|
||||||
|
b.ContextOpts = &terraform.ContextOpts{}
|
||||||
|
}
|
||||||
|
b.ContextOpts.Parallelism = -1
|
||||||
|
|
||||||
op, configCleanup, done := testOperationPlan(t, "./testdata/plan")
|
op, configCleanup, done := testOperationPlan(t, "./testdata/plan")
|
||||||
defer configCleanup()
|
defer configCleanup()
|
||||||
op.PlanRefresh = true
|
|
||||||
|
|
||||||
// we coerce a failure in Context() by omitting the provider schema
|
// we coerce a failure in Context() by omitting the provider schema
|
||||||
run, err := b.Operation(context.Background(), op)
|
run, err := b.Operation(context.Background(), op)
|
||||||
|
@ -136,7 +151,7 @@ func TestLocal_plan_context_error(t *testing.T) {
|
||||||
// the backend should be unlocked after a run
|
// the backend should be unlocked after a run
|
||||||
assertBackendStateUnlocked(t, b)
|
assertBackendStateUnlocked(t, b)
|
||||||
|
|
||||||
if got, want := done(t).Stderr(), "failed to read schema for test_instance.foo in registry.terraform.io/hashicorp/test"; !strings.Contains(got, want) {
|
if got, want := done(t).Stderr(), "Error: Invalid parallelism value"; !strings.Contains(got, want) {
|
||||||
t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want)
|
t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue