From c63040c73729a4f9e607292ede5660dfe1eca522 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 9 Jan 2019 13:01:17 -0500 Subject: [PATCH 1/2] have TestCheckResourceAttr accept missing counts Missing containers were often erroneously kept in the state, but since the addition of the new provider shims, they can often be correctly eliminated. There are however many tests that check for a "0" count in the flatmap state when there shouldn't be a key at all. This addition looks for a container count key and "0" pair, and allows for the key to be missing. There may be some tests negatively effected by this which were legitimately checking for empty containers, but those were also not reliably detected, and there should be much fewer tests involved. --- helper/resource/testing.go | 12 ++++++++++++ helper/resource/testing_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 00da8a92e..073eb5d6f 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -971,7 +971,19 @@ func TestCheckModuleResourceAttr(mp []string, name string, key string, value str } func testCheckResourceAttr(is *terraform.InstanceState, name string, key string, value string) error { + // Empty containers may be elided from the state. + // If the intent here is to check for an empty container, allow the key to + // also be non-existent. + emptyCheck := false + if value == "0" && (strings.HasSuffix(key, ".#") || strings.HasSuffix(key, ".%")) { + emptyCheck = true + } + if v, ok := is.Attributes[key]; !ok || v != value { + if emptyCheck && !ok { + return nil + } + if !ok { return fmt.Errorf("%s: Attribute '%s' not found", name, key) } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 59d2f17b4..bf7369f63 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -1113,3 +1113,34 @@ resource "test_instance" "foo" {} const testConfigStrProvider = ` provider "test" {} ` + +func TestCheckResourceAttr_empty(t *testing.T) { + s := terraform.NewState() + s.AddModuleState(&terraform.ModuleState{ + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": &terraform.ResourceState{ + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "empty_list.#": "0", + "empty_map.%": "0", + }, + }, + }, + }, + }) + + for _, key := range []string{ + "empty_list.#", + "empty_map.%", + "missing_list.#", + "missing_map.%", + } { + t.Run(key, func(t *testing.T) { + check := TestCheckResourceAttr("test_resource", key, "0") + if err := check(s); err != nil { + t.Fatal(err) + } + }) + } +} From 7973872524d4b2466e0cd6bcd22210b78399eba6 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 9 Jan 2019 13:09:02 -0500 Subject: [PATCH 2/2] allow TestCheckNoResourceAttr for empty containers Stricter type handling in the new shims may add empty containers into the state where they were previously elided. Since the detection of missing and empty containers in the legacy state was never reliable, allow TestCheckNoResourceAttr to succeed if the key is a container count index, and the value is "0" --- helper/resource/testing.go | 15 ++++++++++++++- helper/resource/testing_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 073eb5d6f..83ef7713c 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -1026,7 +1026,20 @@ func TestCheckModuleNoResourceAttr(mp []string, name string, key string) TestChe } func testCheckNoResourceAttr(is *terraform.InstanceState, name string, key string) error { - if _, ok := is.Attributes[key]; ok { + // Empty containers may sometimes be included in the state. + // If the intent here is to check for an empty container, allow the value to + // also be "0". + emptyCheck := false + if strings.HasSuffix(key, ".#") || strings.HasSuffix(key, ".%") { + emptyCheck = true + } + + val, exists := is.Attributes[key] + if emptyCheck && val == "0" { + return nil + } + + if exists { return fmt.Errorf("%s: Attribute '%s' found when not expected", name, key) } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index bf7369f63..885bfabf9 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -1144,3 +1144,34 @@ func TestCheckResourceAttr_empty(t *testing.T) { }) } } + +func TestCheckNoResourceAttr_empty(t *testing.T) { + s := terraform.NewState() + s.AddModuleState(&terraform.ModuleState{ + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": &terraform.ResourceState{ + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "empty_list.#": "0", + "empty_map.%": "0", + }, + }, + }, + }, + }) + + for _, key := range []string{ + "empty_list.#", + "empty_map.%", + "missing_list.#", + "missing_map.%", + } { + t.Run(key, func(t *testing.T) { + check := TestCheckNoResourceAttr("test_resource", key) + if err := check(s); err != nil { + t.Fatal(err) + } + }) + } +}