From 149e52ad1f06b2f128bb0d42ad832ab01e613b86 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Thu, 30 Apr 2015 10:13:32 -0500
Subject: [PATCH] helper/resource: verify refresh+plan after each step

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.
---
 helper/resource/testing.go      | 18 +++++++++++++++++-
 helper/resource/testing_test.go | 10 ++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/helper/resource/testing.go b/helper/resource/testing.go
index d8a2b253a..d153f93dc 100644
--- a/helper/resource/testing.go
+++ b/helper/resource/testing.go
@@ -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 {
@@ -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
 }
 
diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go
index 2f7ed0517..a6c944eac 100644
--- a/helper/resource/testing_test.go
+++ b/helper/resource/testing_test.go
@@ -3,6 +3,7 @@ package resource
 import (
 	"fmt"
 	"os"
+	"sync/atomic"
 	"testing"
 
 	"github.com/hashicorp/terraform/terraform"
@@ -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