From e0da21d381f039a27b7a46e14ebee0cb2f4a4abe Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Apr 2016 08:32:57 -0700 Subject: [PATCH] helper/resource: make id-only check opt-in As I've been working through the resources, I'm finding that a lot are going to need some serious work. Given we have hundreds, I think it might be prudent to make this opt-in for now and we can revisit automatic/opt-out at some future point. Importability will likely be opt-in it appears so this will match up with that. --- helper/resource/testing.go | 27 ++++++--------------- helper/resource/testing_test.go | 43 ++------------------------------- 2 files changed, 10 insertions(+), 60 deletions(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 3d10158d5..30a6922f6 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -66,15 +66,12 @@ type TestCase struct { // refreshed with only an ID to result in the same attributes. // This validates completeness of Refresh. // - // DisableIDRefresh, if true, willl not do the id-only refresh test. - // // IDRefreshName is the name of the resource to check. This will // default to the first non-nil primary resource in the state. // // IDRefreshIgnore is a list of configuration keys that will be ignored. - DisableIDRefresh bool - IDRefreshName string - IDRefreshIgnore []string + IDRefreshName string + IDRefreshIgnore []string } // TestStep is a single apply sequence of a test, done within the @@ -163,6 +160,7 @@ func Test(t TestT, c TestCase) { // Go through each step and run it var idRefreshCheck *terraform.ResourceState + idRefresh := c.IDRefreshName != "" errored := false for i, step := range c.Steps { var err error @@ -177,24 +175,15 @@ func Test(t TestT, c TestCase) { // If we've never checked an id-only refresh and our state isn't // empty, find the first resource and test it. - if !c.DisableIDRefresh && idRefreshCheck == nil && !state.Empty() { + if idRefresh && idRefreshCheck == nil && !state.Empty() { // Find the first non-nil resource in the state for _, m := range state.Modules { if len(m.Resources) > 0 { - if c.IDRefreshName != "" { - if v, ok := m.Resources[c.IDRefreshName]; ok { - idRefreshCheck = v - } - - break + if v, ok := m.Resources[c.IDRefreshName]; ok { + idRefreshCheck = v } - for _, v := range m.Resources { - if v != nil && v.Primary != nil { - idRefreshCheck = v - break - } - } + break } } @@ -219,7 +208,7 @@ func Test(t TestT, c TestCase) { } // If we never checked an id-only refresh, it is a failure. - if !c.DisableIDRefresh { + if idRefresh { if !errored && len(c.Steps) > 0 && idRefreshCheck == nil { t.Error("ID-only refresh check never ran.") } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 761d01e19..edb11b7b6 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -125,6 +125,7 @@ func TestTest_idRefresh(t *testing.T) { mt := new(mockT) Test(mt, TestCase{ + IDRefreshName: "test_instance.foo", Providers: map[string]terraform.ResourceProvider{ "test": mp, }, @@ -197,47 +198,6 @@ func TestTest_idRefreshCustomName(t *testing.T) { } } -func TestTest_idRefreshDisable(t *testing.T) { - mp := testProvider() - mp.DiffReturn = nil - - mp.ApplyFn = func( - info *terraform.InstanceInfo, - state *terraform.InstanceState, - diff *terraform.InstanceDiff) (*terraform.InstanceState, error) { - if !diff.Destroy { - return &terraform.InstanceState{ - ID: "foo", - }, nil - } - - return nil, nil - } - - var refreshCount int32 - mp.RefreshFn = func(*terraform.InstanceInfo, *terraform.InstanceState) (*terraform.InstanceState, error) { - atomic.AddInt32(&refreshCount, 1) - return &terraform.InstanceState{ID: "foo"}, nil - } - - mt := new(mockT) - Test(mt, TestCase{ - DisableIDRefresh: true, - Providers: map[string]terraform.ResourceProvider{ - "test": mp, - }, - Steps: []TestStep{ - TestStep{ - Config: testConfigStr, - }, - }, - }) - - if mt.failed() { - t.Fatal("test failed") - } -} - func TestTest_idRefreshFail(t *testing.T) { // Refresh count should be 3: // 1.) initial Ref/Plan/Apply @@ -278,6 +238,7 @@ func TestTest_idRefreshFail(t *testing.T) { mt := new(mockT) Test(mt, TestCase{ + IDRefreshName: "test_instance.foo", Providers: map[string]terraform.ResourceProvider{ "test": mp, },