-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aws/provider: update to using typelist + add empty condition acctest #13334
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha okay hear me out... what if I told you there is this mysterious other acceptance test for provider functionality that probably no one remembers that actually catches a panic bug?
=== CONT TestAccAWSCallerIdentity_basic_panic
------- Stderr: -------
panic: interface conversion: interface {} is nil, not map[string]interface {}
goroutine 137 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.providerConfigure(0xc0012a52d0, 0xc000a26590, 0xa, 0xc0012a52d0, 0xc0000581a0, 0x0, 0x0)
/opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/provider.go:1174 +0x1069
github.com/terraform-providers/terraform-provider-aws/aws.Provider.func1(0xc0012a52d0, 0x0, 0xc000a56660, 0xc0012a52d0, 0x0)
/opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/provider.go:932 +0x58
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Configure(0xc0004cc880, 0xc00163d080, 0x5a4f3c0, 0xc00163cf30)
/opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider.go:275 +0xf6
So! Rather than this weird (I don't even remember how I remembered it) test floating off in some other testing namespace, it would be great to:
- Move that test into
aws/provider_test.go
- Rename it something like
TestAccAWSProvider_AssumeRole_Empty
- Try removing the
aws_caller_identity
bits and ensure it still fails! - Try fixing it and bask in the glory of making all the things better! 🎉
Please let me know if you run into any trouble and thank you for working on this!
(You may also want to update the PR description to not use the |
updated (i think) as suggested @bflad! I'm really not a fan of these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Output from acceptance testing:
--- PASS: TestAccAWSProvider_Region_AwsCommercial (7.41s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_Multiple (8.34s)
--- PASS: TestAccAWSProvider_Region_AwsChina (8.78s)
--- PASS: TestAccAWSProvider_Region_AwsGovCloudUs (8.91s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_One (9.16s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_None (9.18s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_None (9.30s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_One (9.39s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_Multiple (9.60s)
--- PASS: TestAccAWSProvider_Endpoints_Deprecated (9.46s)
--- PASS: TestAccAWSProvider_IgnoreTags_EmptyConfigurationBlock (9.40s)
--- PASS: TestAccAWSProvider_Endpoints (9.51s)
--- PASS: TestAccAWSProvider_AssumeRole_Empty (13.91s)
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Relates #9956
Release note for CHANGELOG:
Output from acceptance testing: