search

hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.81k stars 9.16k forks source link

Document, Standardize, and Potentially Lint for Parent Resource Disappears Test Naming #16591

Closed bflad closed 3 years ago

bflad commented 3 years ago

Community Note

Description

We currently document disappears testing for the simplest case (the resource itself) in the Contributing Guide, however there is not documentation for how to handle disappears testing designed for children resources where the test is meant to verify that removing the parent resource occurs in a recreation plan rather than an error, e.g. for verifying VPC deletion for a Route 53 Zone Association

func TestAccAWSRoute53ZoneAssociation_disappears_VPC(t *testing.T) {
    resourceName := "aws_route53_zone_association.foobar"
    vpcResourceName := "aws_vpc.bar"

    resource.ParallelTest(t, resource.TestCase{
        PreCheck:     func() { testAccPreCheck(t) },
        ErrorCheck:   testAccErrorCheckSkipRoute53(t),
        Providers:    testAccProviders,
        CheckDestroy: testAccCheckRoute53ZoneAssociationDestroy,
        Steps: []resource.TestStep{
            {
                Config: testAccRoute53ZoneAssociationConfig,
                Check: resource.ComposeTestCheckFunc(
                    testAccCheckRoute53ZoneAssociationExists(resourceName),
                    testAccCheckResourceDisappears(testAccProvider, resourceAwsVpc(), vpcResourceName),
                ),
                ExpectNonEmptyPlan: true,
            },
        },
    })
}

There are also two current styles for naming these tests, where the first should be preferred due to higher prevalence:

$ ag --stats 'func TestAcc.*_disappears_' aws/*_test.go
...
29 matches
25 files contained matches
...
$ ag --stats 'func TestAcc.*_[^_]+Disappears' aws/*_test.go
...
9 matches
9 files contained matches
...

We can document the preferred style and example implementation in the Contributing Guide. We may also be able to create a semgrep rule that reports these.

Affected Resource(s)

References

bflad commented 3 years ago

Example semgrep rule:

  - id: acceptance-test-naming-parent-disappears
    languages: [go]
    message: Prefer naming acceptance tests with _disappears_Parent suffix
    paths:
      include:
        - 'aws/*_test.go'
    patterns:
      - pattern: func $FUNCNAME(t *testing.T) { ... }
      - metavariable-regex:
          metavariable: "$FUNCNAME"
          regex: "^TestAcc[^_]+_[a-zA-Z]+[dD]isappears$"
    severity: WARNING
running 1 rules...
aws/resource_aws_backup_selection_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
62:func TestAccAwsBackupSelection_backupPlanDisappears(t *testing.T) {
63: var selection1 backup.GetBackupSelectionOutput
64: resourceName := "aws_backup_selection.test"
65: backupPlanResourceName := "aws_backup_plan.test"
66: rName := acctest.RandomWithPrefix("tf-acc-test")
67:
68: resource.ParallelTest(t, resource.TestCase{
69:     PreCheck:     func() { testAccPreCheck(t); testAccPreCheckAWSBackup(t) },
70:     Providers:    testAccProviders,
71:     CheckDestroy: testAccCheckAwsBackupSelectionDestroy,
72:     Steps: []resource.TestStep{
73:         {
74:             Config: testAccBackupSelectionConfigBasic(rName),
75:             Check: resource.ComposeTestCheckFunc(
76:                 testAccCheckAwsBackupSelectionExists(resourceName, &selection1),
77:                 testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupSelection(), resourceName),
78:                 testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupPlan(), backupPlanResourceName),
79:             ),
80:             ExpectNonEmptyPlan: true,
81:         },
82:     },
83: })
84:}

aws/resource_aws_emr_instance_group_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
205:func TestAccAWSEMRInstanceGroup_EmrClusterDisappears(t *testing.T) {
206:    var cluster emr.Cluster
207:    var ig emr.InstanceGroup
208:    rInt := acctest.RandInt()
209:    emrClusterResourceName := "aws_emr_cluster.tf-test-cluster"
210:    resourceName := "aws_emr_instance_group.task"
211:
212:    resource.ParallelTest(t, resource.TestCase{
213:        PreCheck:     func() { testAccPreCheck(t) },
214:        Providers:    testAccProviders,
215:        CheckDestroy: testAccCheckAWSEmrInstanceGroupDestroy,
216:        Steps: []resource.TestStep{
217:            {
218:                Config: testAccAWSEmrInstanceGroupConfig_basic(rInt),
219:                Check: resource.ComposeTestCheckFunc(
220:                    testAccCheckAWSEmrClusterExists(emrClusterResourceName, &cluster),
221:                    testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
222:                    testAccCheckAWSEmrClusterDisappears(&cluster),
223:                ),
224:                ExpectNonEmptyPlan: true,
225:            },
226:        },
227:    })
228:}

aws/resource_aws_lambda_event_source_mapping_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
277:func TestAccAWSLambdaEventSourceMapping_sqsDisappears(t *testing.T) {
278:    var conf lambda.EventSourceMappingConfiguration
279:
280:    rString := acctest.RandString(8)
281:    roleName := fmt.Sprintf("tf_acc_role_lambda_sqs_import_%s", rString)
282:    policyName := fmt.Sprintf("tf_acc_policy_lambda_sqs_import_%s", rString)
283:    attName := fmt.Sprintf("tf_acc_att_lambda_sqs_import_%s", rString)
284:    streamName := fmt.Sprintf("tf_acc_stream_lambda_sqs_import_%s", rString)
285:    funcName := fmt.Sprintf("tf_acc_lambda_sqs_import_%s", rString)
286:    uFuncName := fmt.Sprintf("tf_acc_lambda_sqs_import_updated_%s", rString)
287:
288:    resource.ParallelTest(t, resource.TestCase{
289:        PreCheck:     func() { testAccPreCheck(t) },
290:        Providers:    testAccProviders,
291:        CheckDestroy: testAccCheckLambdaEventSourceMappingDestroy,
292:        Steps: []resource.TestStep{
293:            {
294:                Config: testAccAWSLambdaEventSourceMappingConfig_sqs(roleName, policyName, attName, streamName, funcName, uFuncName),
295:                Check: resource.ComposeTestCheckFunc(
296:                    testAccCheckAwsLambdaEventSourceMappingExists("aws_lambda_event_source_mapping.lambda_event_source_mapping_test", &conf),
297:                    testAccCheckAWSLambdaEventSourceMappingDisappears(&conf),
298:                ),
299:                ExpectNonEmptyPlan: true,
300:            },
301:        },
302:    })
303:}

aws/resource_aws_msk_scram_secret_association_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
112:func TestAccAwsMskScramSecretAssociation_clusterDisappears(t *testing.T) {
113:    rName := acctest.RandomWithPrefix("tf-acc-test")
114:    resourceName := "aws_msk_scram_secret_association.test"
115:    clusterResourceName := "aws_msk_cluster.test"
116:
117:    resource.ParallelTest(t, resource.TestCase{
118:        PreCheck:     func() { testAccPreCheck(t); testAccPreCheckAWSMsk(t) },
119:        Providers:    testAccProviders,
120:        CheckDestroy: testAccCheckMskScramSecretAssociationDestroy,
121:        Steps: []resource.TestStep{
122:            {
123:                Config: testAccMskScramSecretAssociation_basic(rName, 1),
124:                Check: resource.ComposeTestCheckFunc(
125:                    testAccCheckMskScramSecretAssociationExists(resourceName),
126:                    testAccCheckResourceDisappears(testAccProvider, resourceAwsMskCluster(), clusterResourceName),
127:                ),
128:                ExpectNonEmptyPlan: true,
129:            },
130:        },
131:    })
132:}

aws/resource_aws_network_acl_rule_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
71:func TestAccAWSNetworkAclRule_ingressEgressSameNumberDisappears(t *testing.T) {
72: resource.ParallelTest(t, resource.TestCase{
73:     PreCheck:     func() { testAccPreCheck(t) },
74:     Providers:    testAccProviders,
75:     CheckDestroy: testAccCheckAWSNetworkAclRuleDestroy,
76:     Steps: []resource.TestStep{
77:         {
78:             Config: testAccAWSNetworkAclRuleIngressEgressSameNumberMissing,
79:             Check: resource.ComposeTestCheckFunc(
80:                 testAccCheckAWSNetworkAclRuleExists("aws_network_acl_rule.baz"),
81:                 testAccCheckAWSNetworkAclRuleDelete("aws_network_acl_rule.baz"),
82:             ),
83:             ExpectNonEmptyPlan: true,
84:         },
85:     },
86: })
87:}

aws/resource_aws_s3_access_point_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
144:func TestAccAWSS3AccessPoint_bucketDisappears(t *testing.T) {
145:    var v s3control.GetAccessPointOutput
146:    bucketName := acctest.RandomWithPrefix("tf-acc-test")
147:    accessPointName := acctest.RandomWithPrefix("tf-acc-test")
148:    resourceName := "aws_s3_access_point.test"
149:    bucketResourceName := "aws_s3_bucket.test"
150:
151:    resource.ParallelTest(t, resource.TestCase{
152:        PreCheck:     func() { testAccPreCheck(t) },
153:        Providers:    testAccProviders,
154:        CheckDestroy: testAccCheckAWSS3AccessPointDestroy,
155:        Steps: []resource.TestStep{
156:            {
157:                Config: testAccAWSS3AccessPointConfig_basic(bucketName, accessPointName),
158:                Check: resource.ComposeTestCheckFunc(
159:                    testAccCheckAWSS3AccessPointExists(resourceName, &v),
160:                    testAccCheckAWSS3DestroyBucket(bucketResourceName),
161:                ),
162:                ExpectNonEmptyPlan: true,
163:            },
164:        },
165:    })
166:}

aws/resource_aws_s3_bucket_public_access_block_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
70:func TestAccAWSS3BucketPublicAccessBlock_bucketDisappears(t *testing.T) {
71: var config s3.PublicAccessBlockConfiguration
72: name := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt())
73: resourceName := "aws_s3_bucket_public_access_block.bucket"
74: bucketResourceName := "aws_s3_bucket.bucket"
75:
76: resource.ParallelTest(t, resource.TestCase{
77:     PreCheck:     func() { testAccPreCheck(t) },
78:     Providers:    testAccProviders,
79:     CheckDestroy: testAccCheckAWSS3BucketDestroy,
80:     Steps: []resource.TestStep{
81:         {
82:             Config: testAccAWSS3BucketPublicAccessBlockConfig(name, "false", "false", "false", "false"),
83:             Check: resource.ComposeTestCheckFunc(
84:                 testAccCheckAWSS3BucketPublicAccessBlockExists(resourceName, &config),
85:                 testAccCheckAWSS3DestroyBucket(bucketResourceName),
86:             ),
87:             ExpectNonEmptyPlan: true,
88:         },
89:     },
90: })
91:}

aws/resource_aws_wafv2_web_acl_logging_configuration_test.go
severity:warning rule:acceptance-test-naming-parent-disappears: Prefer naming acceptance tests with _disappears_Parent suffix
204:func TestAccAwsWafv2WebACLLoggingConfiguration_webACLDisappears(t *testing.T) {
205:    var v wafv2.LoggingConfiguration
206:    rName := acctest.RandomWithPrefix("tf-acc-test")
207:    resourceName := "aws_wafv2_web_acl_logging_configuration.test"
208:    webACLResourceName := "aws_wafv2_web_acl.test"
209:
210:    resource.ParallelTest(t, resource.TestCase{
211:        PreCheck:     func() { testAccPreCheck(t); testAccPreCheckAWSWafv2ScopeRegional(t) },
212:        Providers:    testAccProviders,
213:        CheckDestroy: testAccCheckAwsWafv2WebACLLoggingConfigurationDestroy,
214:        Steps: []resource.TestStep{
215:            {
216:                Config: testAccAwsWafv2WebACLLoggingConfiguration_basic(rName),
217:                Check: resource.ComposeTestCheckFunc(
218:                    testAccCheckAwsWafv2WebACLLoggingConfigurationExists(resourceName, &v),
219:                    testAccCheckResourceDisappears(testAccProvider, resourceAwsWafv2WebACL(), webACLResourceName),
220:                ),
221:                ExpectNonEmptyPlan: true,
222:            },
223:        },
224:    })
225:}
ghost commented 3 years ago

This has been released in version 3.21.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost commented 3 years ago

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!