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.
This commit is contained in:
Mitchell Hashimoto 2016-04-21 08:32:57 -07:00
parent 5cd60aa26c
commit e0da21d381
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
2 changed files with 10 additions and 60 deletions

View File

@ -66,15 +66,12 @@ type TestCase struct {
// refreshed with only an ID to result in the same attributes. // refreshed with only an ID to result in the same attributes.
// This validates completeness of Refresh. // 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 // IDRefreshName is the name of the resource to check. This will
// default to the first non-nil primary resource in the state. // default to the first non-nil primary resource in the state.
// //
// IDRefreshIgnore is a list of configuration keys that will be ignored. // IDRefreshIgnore is a list of configuration keys that will be ignored.
DisableIDRefresh bool IDRefreshName string
IDRefreshName string IDRefreshIgnore []string
IDRefreshIgnore []string
} }
// TestStep is a single apply sequence of a test, done within the // 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 // Go through each step and run it
var idRefreshCheck *terraform.ResourceState var idRefreshCheck *terraform.ResourceState
idRefresh := c.IDRefreshName != ""
errored := false errored := false
for i, step := range c.Steps { for i, step := range c.Steps {
var err error 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 // If we've never checked an id-only refresh and our state isn't
// empty, find the first resource and test it. // 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 // Find the first non-nil resource in the state
for _, m := range state.Modules { for _, m := range state.Modules {
if len(m.Resources) > 0 { if len(m.Resources) > 0 {
if c.IDRefreshName != "" { if v, ok := m.Resources[c.IDRefreshName]; ok {
if v, ok := m.Resources[c.IDRefreshName]; ok { idRefreshCheck = v
idRefreshCheck = v
}
break
} }
for _, v := range m.Resources { break
if v != nil && v.Primary != nil {
idRefreshCheck = v
break
}
}
} }
} }
@ -219,7 +208,7 @@ func Test(t TestT, c TestCase) {
} }
// If we never checked an id-only refresh, it is a failure. // 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 { if !errored && len(c.Steps) > 0 && idRefreshCheck == nil {
t.Error("ID-only refresh check never ran.") t.Error("ID-only refresh check never ran.")
} }

View File

@ -125,6 +125,7 @@ func TestTest_idRefresh(t *testing.T) {
mt := new(mockT) mt := new(mockT)
Test(mt, TestCase{ Test(mt, TestCase{
IDRefreshName: "test_instance.foo",
Providers: map[string]terraform.ResourceProvider{ Providers: map[string]terraform.ResourceProvider{
"test": mp, "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) { func TestTest_idRefreshFail(t *testing.T) {
// Refresh count should be 3: // Refresh count should be 3:
// 1.) initial Ref/Plan/Apply // 1.) initial Ref/Plan/Apply
@ -278,6 +238,7 @@ func TestTest_idRefreshFail(t *testing.T) {
mt := new(mockT) mt := new(mockT)
Test(mt, TestCase{ Test(mt, TestCase{
IDRefreshName: "test_instance.foo",
Providers: map[string]terraform.ResourceProvider{ Providers: map[string]terraform.ResourceProvider{
"test": mp, "test": mp,
}, },