Skip to content

Commit

Permalink
helper/resource: verify refresh+plan after each step
Browse files Browse the repository at this point in the history
I forgot to add `Computed: true` when I made the "key_name" field
optional in #1751.

This made the behavior:

 * Name generated in Create and set as ID
 * Follow up plan (without refresh) was nice and empty
 * During refresh, name gets cleared out on Read, causing a bad diff on
   subsequent plans

We can automatically catch bugs like this if we add yet another
verification step to our resource acceptance tests -> a post
Refresh+Plan that we verify is empty.

I left the non-refresh Plan verification in, because it's important that
_both_ of these are empty after an Apply.
  • Loading branch information
phinze committed Apr 30, 2015
1 parent 89b50f7 commit 149e52a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
18 changes: 17 additions & 1 deletion helper/resource/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ func testStep(
}
}

// Verify that Plan is now empty and we don't have a perpetual diff issue
// Now, verify that Plan is now empty and we don't have a perpetual diff issue
// We do this with TWO plans. One without a refresh.
if p, err := ctx.Plan(); err != nil {
return state, fmt.Errorf("Error on follow-up plan: %s", err)
} else {
Expand All @@ -250,6 +251,21 @@ func testStep(
}
}

// And another after a Refresh.
state, err = ctx.Refresh()
if err != nil {
return state, fmt.Errorf(
"Error on follow-up refresh: %s", err)
}
if p, err := ctx.Plan(); err != nil {
return state, fmt.Errorf("Error on second follow-up plan: %s", err)
} else {
if p.Diff != nil && !p.Diff.Empty() {
return state, fmt.Errorf(
"After applying this step and refreshing, the plan was not empty:\n\n%s", p)
}
}

return state, err
}

Expand Down
10 changes: 10 additions & 0 deletions helper/resource/testing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resource
import (
"fmt"
"os"
"sync/atomic"
"testing"

"github.com/hashicorp/terraform/terraform"
Expand All @@ -23,6 +24,15 @@ func TestTest(t *testing.T) {
mp.ApplyReturn = &terraform.InstanceState{
ID: "foo",
}
var refreshCount int32
mp.RefreshFn = func(*terraform.InstanceInfo, *terraform.InstanceState) (*terraform.InstanceState, error) {
atomic.AddInt32(&refreshCount, 1)
if atomic.LoadInt32(&refreshCount) == 1 {
return &terraform.InstanceState{ID: "foo"}, nil
} else {
return nil, nil
}
}

checkDestroy := false
checkStep := false
Expand Down

0 comments on commit 149e52a

Please sign in to comment.