Adding test coverage for changes to ignore data sources in imported state #1077
Conversation
bendbennett
force-pushed
the
bendbennett/issues-1066
branch
from
e477ed9 to
86ba558
Compare
| @@ -1738,43 +1923,41 @@ func composeImportStateCheck(fs ...ImportStateCheckFunc) ImportStateCheckFunc { | |||
| } | |||
| } | |||
|
|
|||
| func testExtractResourceAttrInstanceState(attributeName string, attributeValue *string) ImportStateCheckFunc { | |||
| func testExtractResourceAttrInstanceState(id, attributeName string, attributeValue *string) ImportStateCheckFunc { | |||
There was a problem hiding this comment.
Nit: Instead of fetching via the internal identifier (typically the id attribute, but not necessarily guaranteed) can we instead use the resource address string? e.g. random_string.example Reasons being:
- This is how other testing framework "check" functions generally work
- Resources such as this one may have duplicate resource instances with the same id
- We're looking at making id less special in #1072
There was a problem hiding this comment.
Not sure how we use resource address when we're using a type ImportStateCheckFunc func([]*terraform.InstanceState) error as *terraform.InstanceState just has the ID and Attributes and doesn't appear to have any reference to the resource address.
There was a problem hiding this comment.
Sigh, more trouble than its worth then (e.g. mapping those instance states with the resource addresses). We'll be removing these terraform bits when we take a look at the next iteration of the test framework and this isn't exported, so fine for now.
|
Please note this also needs a changelog entry before merge. |
bflad
added
bug
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
References: #1074
Closes: #1066
Closes: #1074
Changes in this PR are to accompany changes in #1074.@gdavison feel free to pull these changes into your PR and we can merge it all in together.
Tests in this PR fail as expected, require changes in #1074 to pass.I've cherry-picked @gdavison's changes from #1074 into this PR.