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.
This commit is contained in:
Martin Atkins 2018-11-07 14:48:55 -08:00
parent 0ea8aa6fe5
commit fcf3f643ce
2 changed files with 11 additions and 5 deletions

View File

@ -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")
}
}

View File

@ -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
}