From f4043b85fc9f7408c18f868cdcd6e3f25167a615 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 21 Feb 2020 08:56:58 -0500 Subject: [PATCH] helper/resource: Return error with TestCheckResourceAttrPair() when comparing self Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/334 Also consolidates the unit testing for the function. --- helper/resource/testing.go | 8 ++ helper/resource/testing_test.go | 137 ++++++++++++++++++++------------ 2 files changed, 93 insertions(+), 52 deletions(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 8fa28d7bf7c..a2baafb7e44 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -1232,6 +1232,14 @@ func TestCheckModuleResourceAttrPair(mpFirst []string, nameFirst string, keyFirs } func testCheckResourceAttrPair(isFirst *terraform.InstanceState, nameFirst string, keyFirst string, isSecond *terraform.InstanceState, nameSecond string, keySecond string) error { + if nameFirst == nameSecond && keyFirst == keySecond { + return fmt.Errorf( + "comparing self: resource %s attribute %s", + nameFirst, + keyFirst, + ) + } + vFirst, okFirst := isFirst.Attributes[keyFirst] vSecond, okSecond := isSecond.Attributes[keySecond] diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 8b344b324ee..798f10f9b78 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -1710,11 +1710,49 @@ func TestCheckNoResourceAttr_empty(t *testing.T) { func TestTestCheckResourceAttrPair(t *testing.T) { tests := map[string]struct { - state *terraform.State - wantErr string + nameFirst string + keyFirst string + nameSecond string + keySecond string + state *terraform.State + wantErr string }{ + "self": { + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.a", + keySecond: "a", + state: &terraform.State{ + Modules: []*terraform.ModuleState{ + { + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test.a": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "a": "boop", + }, + }, + }, + "test.b": { + Primary: &terraform.InstanceState{ + Attributes: map[string]string{ + "b": "boop", + }, + }, + }, + }, + }, + }, + }, + wantErr: `comparing self: resource test.a attribute a`, + }, "exist match": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.b", + keySecond: "b", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1737,10 +1775,14 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }, }, }, - ``, + wantErr: ``, }, "nonexist match": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.b", + keySecond: "b", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1759,10 +1801,14 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }, }, }, - ``, + wantErr: ``, }, "exist nonmatch": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.b", + keySecond: "b", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1785,10 +1831,14 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }, }, }, - `test.a: Attribute 'a' expected "boop", got "beep"`, + wantErr: `test.a: Attribute 'a' expected "boop", got "beep"`, }, "inconsistent exist a": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.b", + keySecond: "b", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1809,10 +1859,14 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }, }, }, - `test.a: Attribute "a" is "beep", but "b" is not set in test.b`, + wantErr: `test.a: Attribute "a" is "beep", but "b" is not set in test.b`, }, "inconsistent exist b": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a", + nameSecond: "test.b", + keySecond: "b", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1833,40 +1887,14 @@ func TestTestCheckResourceAttrPair(t *testing.T) { }, }, }, - `test.a: Attribute "a" not set, but "b" is set in test.b as "boop"`, + wantErr: `test.a: Attribute "a" not set, but "b" is set in test.b as "boop"`, }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - fn := TestCheckResourceAttrPair("test.a", "a", "test.b", "b") - err := fn(test.state) - - if test.wantErr != "" { - if err == nil { - t.Fatalf("succeeded; want error\nwant: %s", test.wantErr) - } - if got, want := err.Error(), test.wantErr; got != want { - t.Fatalf("wrong error\ngot: %s\nwant: %s", got, want) - } - return - } - - if err != nil { - t.Fatalf("failed; want success\ngot: %s", err.Error()) - } - }) - } -} - -func TestTestCheckResourceAttrPairCount(t *testing.T) { - tests := map[string]struct { - state *terraform.State - attr string - wantErr string - }{ "unset and 0 equal list": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a.#", + nameSecond: "test.b", + keySecond: "a.#", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1887,11 +1915,14 @@ func TestTestCheckResourceAttrPairCount(t *testing.T) { }, }, }, - "a.#", - ``, + wantErr: ``, }, "unset and 0 equal map": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a.%", + nameSecond: "test.b", + keySecond: "a.%", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1912,11 +1943,14 @@ func TestTestCheckResourceAttrPairCount(t *testing.T) { }, }, }, - "a.%", - ``, + wantErr: ``, }, "count equal": { - &terraform.State{ + nameFirst: "test.a", + keyFirst: "a.%", + nameSecond: "test.b", + keySecond: "a.%", + state: &terraform.State{ Modules: []*terraform.ModuleState{ { Path: []string{"root"}, @@ -1938,14 +1972,13 @@ func TestTestCheckResourceAttrPairCount(t *testing.T) { }, }, }, - "a.%", - ``, + wantErr: ``, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - fn := TestCheckResourceAttrPair("test.a", test.attr, "test.b", test.attr) + fn := TestCheckResourceAttrPair(test.nameFirst, test.keyFirst, test.nameSecond, test.keySecond) err := fn(test.state) if test.wantErr != "" {