From fcf3f643cef42de8ac789d9b410164e3566ac6f6 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 7 Nov 2018 14:48:55 -0800 Subject: [PATCH] command: Fix TestPlan_shutdown Comments here indicate that this was erroneously returning an error but we accepted it anyway to get the tests passing again after other work. The tests over in the "terraform" package agree that cancelling should be a successful outcome rather than an error. I think that cancelling _should_ actually be an error, since Terraform did not complete the operation it set out to complete, but that's a change we'd need to make cautiously since automation wrapper scripts may be depending on the success-on-cancel behavior. Therefore this just fixes the command package test to agree with the Terraform package tests and adds some FIXME notes to capture the potential that we might want to update this later. --- command/plan_test.go | 12 +++++++----- terraform/hook_stop.go | 4 ++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/command/plan_test.go b/command/plan_test.go index 2af32adf1..b646ce7e4 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -761,16 +761,18 @@ func TestPlan_shutdown(t *testing.T) { "-state=nonexistent.tfstate", testFixturePath("apply-shutdown"), }) - if code != 1 { - // FIXME: we should be able to avoid the error during evaluation - // the early exit isn't caught before the interpolation is evaluated - t.Fatalf("wrong exit code %d; want 1\noutput:\n%s", code, ui.OutputWriter.String()) + if code != 0 { + // FIXME: In retrospect cancellation ought to be an unsuccessful exit + // case, but we need to do that cautiously in case it impacts automation + // wrappers. See the note about this in the terraform.stopHook + // implementation for more. + t.Errorf("wrong exit code %d; want 0\noutput:\n%s", code, ui.OutputWriter.String()) } select { case <-cancelled: default: - t.Fatal("command not cancelled") + t.Error("command not cancelled") } } diff --git a/terraform/hook_stop.go b/terraform/hook_stop.go index 72c004d33..811fb337c 100644 --- a/terraform/hook_stop.go +++ b/terraform/hook_stop.go @@ -76,6 +76,10 @@ func (h *stopHook) PostStateUpdate(new *states.State) (HookAction, error) { func (h *stopHook) hook() (HookAction, error) { if h.Stopped() { + // FIXME: This should really return an error since stopping partway + // through is not a successful run-to-completion, but we'll need to + // introduce that cautiously since existing automation solutions may + // be depending on this behavior. return HookActionHalt, nil }