From 1a98765ae6c5ffc95b341b3c33ae01b981b5bf56 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 29 Mar 2022 17:05:05 -0400 Subject: [PATCH 1/2] helper/resource: Added error when errantly checking list, map, or set attributes in `TestCheckNoResourceAttr`, `TestCheckResourceAttr`, and `TestCheckResourceAttrSet` Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/885 --- .changelog/pending.txt | 7 + helper/resource/testing.go | 100 +++- helper/resource/testing_test.go | 947 ++++++++++++++++++++++++++++++-- 3 files changed, 990 insertions(+), 64 deletions(-) create mode 100644 .changelog/pending.txt diff --git a/.changelog/pending.txt b/.changelog/pending.txt new file mode 100644 index 00000000000..e9671e2dfc5 --- /dev/null +++ b/.changelog/pending.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +helper/resource: Added error when errantly checking list, map, or set attributes in `TestCheckNoResourceAttr`, `TestCheckResourceAttr`, and `TestCheckResourceAttrSet` +``` + +```release-note:note +helper/resource: False positive checks of list, map, and set attributes with `TestCheckNoResourceAttr` and `TestCheckResourceAttrSet` will now return an error to explain how to accurately check those types of attributes. Some previously passing tests will now fail until the check is correctly updated. +``` diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 2c200c85b2c..79eb30eeac8 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -823,11 +823,33 @@ func TestCheckModuleResourceAttrSet(mp []string, name string, key string) TestCh } func testCheckResourceAttrSet(is *terraform.InstanceState, name string, key string) error { - if val, ok := is.Attributes[key]; !ok || val == "" { - return fmt.Errorf("%s: Attribute '%s' expected to be set", name, key) + val, ok := is.Attributes[key] + + if ok && val != "" { + return nil } - return nil + if _, ok := is.Attributes[key+".#"]; ok { + return fmt.Errorf( + "%s: list or set attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s). Set element value checks should use TestCheckTypeSet functions instead.", + name, + key, + key+".#", + key+".0", + ) + } + + if _, ok := is.Attributes[key+".%"]; ok { + return fmt.Errorf( + "%s: map attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s).", + name, + key, + key+".%", + key+".examplekey", + ) + } + + return fmt.Errorf("%s: Attribute '%s' expected to be set", name, key) } // TestCheckResourceAttr ensures a specific value is stored in state for the @@ -892,23 +914,40 @@ 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 - } + v, ok := is.Attributes[key] - if v, ok := is.Attributes[key]; !ok || v != value { - if emptyCheck && !ok { + if !ok { + // 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. + if value == "0" && (strings.HasSuffix(key, ".#") || strings.HasSuffix(key, ".%")) { return nil } - if !ok { - return fmt.Errorf("%s: Attribute '%s' not found", name, key) + if _, ok := is.Attributes[key+".#"]; ok { + return fmt.Errorf( + "%s: list or set attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s). Set element value checks should use TestCheckTypeSet functions instead.", + name, + key, + key+".#", + key+".0", + ) + } + + if _, ok := is.Attributes[key+".%"]; ok { + return fmt.Errorf( + "%s: map attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s).", + name, + key, + key+".%", + key+".examplekey", + ) } + return fmt.Errorf("%s: Attribute '%s' not found", name, key) + } + + if v != value { return fmt.Errorf( "%s: Attribute '%s' expected %#v, got %#v", name, @@ -916,6 +955,7 @@ func testCheckResourceAttr(is *terraform.InstanceState, name string, key string, value, v) } + return nil } @@ -976,23 +1016,39 @@ func TestCheckModuleNoResourceAttr(mp []string, name string, key string) TestChe } func testCheckNoResourceAttr(is *terraform.InstanceState, name string, key string) error { + v, ok := is.Attributes[key] + // 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" { + if v == "0" && (strings.HasSuffix(key, ".#") || strings.HasSuffix(key, ".%")) { return nil } - if exists { + if ok { return fmt.Errorf("%s: Attribute '%s' found when not expected", name, key) } + if _, ok := is.Attributes[key+".#"]; ok { + return fmt.Errorf( + "%s: list or set attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s). Set element value checks should use TestCheckTypeSet functions instead.", + name, + key, + key+".#", + key+".0", + ) + } + + if _, ok := is.Attributes[key+".%"]; ok { + return fmt.Errorf( + "%s: map attribute '%s' must be checked by element count key (%s) or element value keys (e.g. %s).", + name, + key, + key+".%", + key+".examplekey", + ) + } + return nil } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 403a2bf5204..660471a2c37 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -843,63 +843,786 @@ func mockSweeperFunc(s string) error { return nil } -func TestCheckResourceAttr_empty(t *testing.T) { - s := terraform.NewState() - s.AddModuleState(&terraform.ModuleState{ - Path: []string{"root"}, - Resources: map[string]*terraform.ResourceState{ - "test_resource": { - Primary: &terraform.InstanceState{ - Attributes: map[string]string{ - "empty_list.#": "0", - "empty_map.%": "0", +func TestTestCheckResourceAttr(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + state *terraform.State + key string + value string + expectedError error + }{ + "attribute not found": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{}, + }, + }, + }, + }, + }, + }, + key: "nonexistent", + value: "test-value", + expectedError: fmt.Errorf("Attribute 'nonexistent' not found"), + }, + "bool attribute match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_bool_attribute": "true", + }, + }, + }, + }, + }, + }, + }, + key: "test_bool_attribute", + value: "true", + }, + "bool attribute mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_bool_attribute": "true", + }, + }, + }, + }, + }, + }, + }, + key: "test_bool_attribute", + value: "false", + expectedError: fmt.Errorf("Attribute 'test_bool_attribute' expected \"false\", got \"true\""), + }, + "float attribute match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_float_attribute": "1.2", + }, + }, + }, + }, + }, + }, + }, + key: "test_float_attribute", + value: "1.2", + }, + "float attribute mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_float_attribute": "1.2", + }, + }, + }, + }, + }, + }, + }, + key: "test_float_attribute", + value: "3.4", + expectedError: fmt.Errorf("Attribute 'test_float_attribute' expected \"3.4\", got \"1.2\""), + }, + "integer attribute match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_integer_attribute": "123", + }, + }, + }, + }, + }, + }, + }, + key: "test_integer_attribute", + value: "123", + }, + "integer attribute mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_integer_attribute": "123", + }, + }, + }, + }, + }, + }, + }, + key: "test_integer_attribute", + value: "456", + expectedError: fmt.Errorf("Attribute 'test_integer_attribute' expected \"456\", got \"123\""), + }, + "list attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute", + value: "test-value", + expectedError: fmt.Errorf("list or set attribute 'test_list_attribute' must be checked by element count key (test_list_attribute.#) or element value keys (e.g. test_list_attribute.0)"), + }, + "list attribute element count match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + value: "1", + }, + "list attribute element count mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + value: "2", + expectedError: fmt.Errorf("Attribute 'test_list_attribute.#' expected \"2\", got \"1\""), + }, + "list attribute element count match 0 when empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "0", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + value: "0", + }, + // Special case with .# and value 0 + "list attribute element count match 0 when missing": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{}, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + value: "0", + }, + "list attribute element value match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.0", + value: "test-value", + }, + "list attribute element value mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.0", + value: "not-test-value", + expectedError: fmt.Errorf("Attribute 'test_list_attribute.0' expected \"not-test-value\", got \"test-value\""), + }, + "map attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute", + value: "test-value", + expectedError: fmt.Errorf("map attribute 'test_map_attribute' must be checked by element count key (test_map_attribute.%%) or element value keys (e.g. test_map_attribute.examplekey)"), + }, + "map attribute element count match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + value: "1", + }, + "map attribute element count mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + value: "2", + expectedError: fmt.Errorf("Attribute 'test_map_attribute.%%' expected \"2\", got \"1\""), + }, + "map attribute element count match 0 when empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "0", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + value: "0", + }, + // Special case with .% and value 0 + "map attribute element count match 0 when missing": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{}, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + value: "0", + }, + "map attribute element value match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.testkey1", + value: "test-value-1", + }, + "map attribute element value mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.testkey1", + value: "test-value-2", + expectedError: fmt.Errorf("Attribute 'test_map_attribute.testkey1' expected \"test-value-2\", got \"test-value-1\""), + }, + "set attribute indexing error": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_set_attribute.#": "1", + "test_set_attribute.101.test_string_attribute": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_set_attribute.101.nonexistent", + value: "test-value", + expectedError: fmt.Errorf("likely indexes into TypeSet"), + }, + "string attribute match": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_string_attribute": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_string_attribute", + value: "test-value", + }, + "string attribute mismatch": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_string_attribute": "test-value", + }, + }, + }, + }, }, }, }, + key: "test_string_attribute", + value: "not-test-value", + expectedError: fmt.Errorf("Attribute 'test_string_attribute' expected \"not-test-value\", got \"test-value\""), }, - }) + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := TestCheckResourceAttr("test_resource", testCase.key, testCase.value)(testCase.state) + + if err != nil { + if testCase.expectedError == nil { + t.Fatalf("expected no error, got: %s", err) + } + + if !strings.Contains(err.Error(), testCase.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedError, err) + } + } - 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) + if err == nil && testCase.expectedError != nil { + t.Fatalf("expected error: %s", testCase.expectedError) } }) } } -func TestCheckNoResourceAttr_empty(t *testing.T) { - s := terraform.NewState() - s.AddModuleState(&terraform.ModuleState{ - Path: []string{"root"}, - Resources: map[string]*terraform.ResourceState{ - "test_resource": { - Primary: &terraform.InstanceState{ - Attributes: map[string]string{ - "empty_list.#": "0", - "empty_map.%": "0", +func TestTestCheckNoResourceAttr(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + state *terraform.State + key string + expectedError error + }{ + "attribute not found": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{}, + }, + }, + }, + }, + }, + }, + key: "nonexistent", + }, + "attribute found": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_bool_attribute": "true", + }, + }, + }, + }, + }, + }, + }, + key: "test_bool_attribute", + expectedError: fmt.Errorf("Attribute 'test_bool_attribute' found when not expected"), + }, + "list attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute", + expectedError: fmt.Errorf("list or set attribute 'test_list_attribute' must be checked by element count key (test_list_attribute.#) or element value keys (e.g. test_list_attribute.0)"), + }, + // Special case with .# and value 0 + "list attribute element count match 0 when empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "0", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + }, + "list attribute element count mismatch 0 when non-empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute.#", + expectedError: fmt.Errorf("Attribute 'test_list_attribute.#' found when not expected"), + }, + "map attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute", + expectedError: fmt.Errorf("map attribute 'test_map_attribute' must be checked by element count key (test_map_attribute.%%) or element value keys (e.g. test_map_attribute.examplekey)"), + }, + // Special case with .% and value 0 + "map attribute element count match 0 when empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "0", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + }, + "map attribute element count mismatch 0 when non-empty": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute.%", + expectedError: fmt.Errorf("Attribute 'test_map_attribute.%%' found when not expected"), + }, + "set attribute indexing error": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_set_attribute.#": "1", + "test_set_attribute.101.test_string_attribute": "test-value", + }, + }, + }, + }, }, }, }, + key: "test_set_attribute.101.test_string_attribute", + expectedError: fmt.Errorf("likely indexes into TypeSet"), }, - }) + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := TestCheckNoResourceAttr("test_resource", testCase.key)(testCase.state) + + if err != nil { + if testCase.expectedError == nil { + t.Fatalf("expected no error, got: %s", err) + } - 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) + if !strings.Contains(err.Error(), testCase.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedError, err) + } + } + + if err == nil && testCase.expectedError != nil { + t.Fatalf("expected error: %s", testCase.expectedError) } }) } @@ -1194,3 +1917,143 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }) } } + +func TestTestCheckResourceAttrSet(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + state *terraform.State + key string + expectedError error + }{ + "attribute not found": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{}, + }, + }, + }, + }, + }, + }, + key: "nonexistent", + expectedError: fmt.Errorf("Attribute 'nonexistent' expected to be set"), + }, + "attribute found": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_bool_attribute": "true", + }, + }, + }, + }, + }, + }, + }, + key: "test_bool_attribute", + }, + "list attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_list_attribute.#": "1", + "test_list_attribute.0": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_list_attribute", + expectedError: fmt.Errorf("list or set attribute 'test_list_attribute' must be checked by element count key (test_list_attribute.#) or element value keys (e.g. test_list_attribute.0)"), + }, + "map attribute directly": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_map_attribute.%": "1", + "test_map_attribute.testkey1": "test-value-1", + }, + }, + }, + }, + }, + }, + }, + key: "test_map_attribute", + expectedError: fmt.Errorf("map attribute 'test_map_attribute' must be checked by element count key (test_map_attribute.%%) or element value keys (e.g. test_map_attribute.examplekey)"), + }, + "set attribute indexing error": { + state: &terraform.State{ + IsBinaryDrivenTest: true, // Always true now + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_resource": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "test_set_attribute.#": "1", + "test_set_attribute.101.test_string_attribute": "test-value", + }, + }, + }, + }, + }, + }, + }, + key: "test_set_attribute.101.nonexistent", + expectedError: fmt.Errorf("likely indexes into TypeSet"), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := TestCheckResourceAttrSet("test_resource", testCase.key)(testCase.state) + + if err != nil { + if testCase.expectedError == nil { + t.Fatalf("expected no error, got: %s", err) + } + + if !strings.Contains(err.Error(), testCase.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedError, err) + } + } + + if err == nil && testCase.expectedError != nil { + t.Fatalf("expected error: %s", testCase.expectedError) + } + }) + } +} From 693b8d3628a37b41b141d7dfa09af6894fb2c6da Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 29 Mar 2022 17:07:30 -0400 Subject: [PATCH 2/2] Update CHANGELOG for #920 --- .changelog/{pending.txt => 920.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{pending.txt => 920.txt} (100%) diff --git a/.changelog/pending.txt b/.changelog/920.txt similarity index 100% rename from .changelog/pending.txt rename to .changelog/920.txt