From 6c95dc625961bb7d6ee715331922affdcbacbac0 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 4 Dec 2020 13:05:36 -0500 Subject: [PATCH] provider: Document, standardize, and lint for disappears acceptance testing of parent resources Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16591 --- .semgrep.yml | 13 ++++++++ aws/resource_aws_backup_selection_test.go | 2 +- aws/resource_aws_emr_instance_group_test.go | 2 +- ...ce_aws_lambda_event_source_mapping_test.go | 30 +------------------ ...e_aws_msk_scram_secret_association_test.go | 2 +- aws/resource_aws_network_acl_rule_test.go | 2 +- ...ws_networkfirewall_resource_policy_test.go | 4 +-- aws/resource_aws_s3_access_point_test.go | 2 +- ..._aws_s3_bucket_public_access_block_test.go | 2 +- ...afv2_web_acl_logging_configuration_test.go | 2 +- .../running-and-writing-acceptance-tests.md | 26 ++++++++++++++++ 11 files changed, 49 insertions(+), 38 deletions(-) diff --git a/.semgrep.yml b/.semgrep.yml index 3520fd50fad..6a2728e96ef 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -1,4 +1,17 @@ rules: + - 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|[^_]+_disappears)$" + severity: WARNING + - id: aws-sdk-go-multiple-service-imports languages: [go] message: Resources should not implement multiple AWS service functionality diff --git a/aws/resource_aws_backup_selection_test.go b/aws/resource_aws_backup_selection_test.go index e7030459966..9d6c72f5a2c 100644 --- a/aws/resource_aws_backup_selection_test.go +++ b/aws/resource_aws_backup_selection_test.go @@ -59,7 +59,7 @@ func TestAccAwsBackupSelection_disappears(t *testing.T) { }) } -func TestAccAwsBackupSelection_backupPlanDisappears(t *testing.T) { +func TestAccAwsBackupSelection_disappears_BackupPlan(t *testing.T) { var selection1 backup.GetBackupSelectionOutput resourceName := "aws_backup_selection.test" backupPlanResourceName := "aws_backup_plan.test" diff --git a/aws/resource_aws_emr_instance_group_test.go b/aws/resource_aws_emr_instance_group_test.go index 340f765cfdc..ab797cf7347 100644 --- a/aws/resource_aws_emr_instance_group_test.go +++ b/aws/resource_aws_emr_instance_group_test.go @@ -202,7 +202,7 @@ func TestAccAWSEMRInstanceGroup_InstanceCount(t *testing.T) { } // Regression test for https://github.com/hashicorp/terraform-provider-aws/issues/1355 -func TestAccAWSEMRInstanceGroup_EmrClusterDisappears(t *testing.T) { +func TestAccAWSEMRInstanceGroup_disappears_EmrCluster(t *testing.T) { var cluster emr.Cluster var ig emr.InstanceGroup rInt := acctest.RandInt() diff --git a/aws/resource_aws_lambda_event_source_mapping_test.go b/aws/resource_aws_lambda_event_source_mapping_test.go index 11eb857de2f..f4e7c7d65af 100644 --- a/aws/resource_aws_lambda_event_source_mapping_test.go +++ b/aws/resource_aws_lambda_event_source_mapping_test.go @@ -246,35 +246,7 @@ func TestAccAWSLambdaEventSourceMapping_SQSBatchWindow(t *testing.T) { }) } -func TestAccAWSLambdaEventSourceMapping_kinesis_disappears(t *testing.T) { - var conf lambda.EventSourceMappingConfiguration - - rString := acctest.RandString(8) - roleName := fmt.Sprintf("tf_acc_role_lambda_esm_import_%s", rString) - policyName := fmt.Sprintf("tf_acc_policy_lambda_esm_import_%s", rString) - attName := fmt.Sprintf("tf_acc_att_lambda_esm_import_%s", rString) - streamName := fmt.Sprintf("tf_acc_stream_lambda_esm_import_%s", rString) - funcName := fmt.Sprintf("tf_acc_lambda_esm_import_%s", rString) - uFuncName := fmt.Sprintf("tf_acc_lambda_esm_import_updated_%s", rString) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckLambdaEventSourceMappingDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAWSLambdaEventSourceMappingConfig_kinesis(roleName, policyName, attName, streamName, funcName, uFuncName), - Check: resource.ComposeTestCheckFunc( - testAccCheckAwsLambdaEventSourceMappingExists("aws_lambda_event_source_mapping.lambda_event_source_mapping_test", &conf), - testAccCheckAWSLambdaEventSourceMappingDisappears(&conf), - ), - ExpectNonEmptyPlan: true, - }, - }, - }) -} - -func TestAccAWSLambdaEventSourceMapping_sqsDisappears(t *testing.T) { +func TestAccAWSLambdaEventSourceMapping_disappears(t *testing.T) { var conf lambda.EventSourceMappingConfiguration rString := acctest.RandString(8) diff --git a/aws/resource_aws_msk_scram_secret_association_test.go b/aws/resource_aws_msk_scram_secret_association_test.go index e73e5d2047a..0847e305a2e 100644 --- a/aws/resource_aws_msk_scram_secret_association_test.go +++ b/aws/resource_aws_msk_scram_secret_association_test.go @@ -109,7 +109,7 @@ func TestAccAwsMskScramSecretAssociation_disappears(t *testing.T) { }) } -func TestAccAwsMskScramSecretAssociation_clusterDisappears(t *testing.T) { +func TestAccAwsMskScramSecretAssociation_disappears_Cluster(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_msk_scram_secret_association.test" clusterResourceName := "aws_msk_cluster.test" diff --git a/aws/resource_aws_network_acl_rule_test.go b/aws/resource_aws_network_acl_rule_test.go index ea56cdc6650..9823023ed00 100644 --- a/aws/resource_aws_network_acl_rule_test.go +++ b/aws/resource_aws_network_acl_rule_test.go @@ -68,7 +68,7 @@ func TestAccAWSNetworkAclRule_disappears(t *testing.T) { }) } -func TestAccAWSNetworkAclRule_ingressEgressSameNumberDisappears(t *testing.T) { +func TestAccAWSNetworkAclRule_disappears_IngressEgressSameNumber(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, diff --git a/aws/resource_aws_networkfirewall_resource_policy_test.go b/aws/resource_aws_networkfirewall_resource_policy_test.go index 80b5c776a14..3eee0d8de0f 100644 --- a/aws/resource_aws_networkfirewall_resource_policy_test.go +++ b/aws/resource_aws_networkfirewall_resource_policy_test.go @@ -101,7 +101,7 @@ func TestAccAwsNetworkFirewallResourcePolicy_disappears(t *testing.T) { }) } -func TestAccAwsNetworkFirewallResourcePolicy_firewallPolicy_disappears(t *testing.T) { +func TestAccAwsNetworkFirewallResourcePolicy_disappears_FirewallPolicy(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_networkfirewall_resource_policy.test" @@ -122,7 +122,7 @@ func TestAccAwsNetworkFirewallResourcePolicy_firewallPolicy_disappears(t *testin }) } -func TestAccAwsNetworkFirewallResourcePolicy_ruleGroup_disappears(t *testing.T) { +func TestAccAwsNetworkFirewallResourcePolicy_disappears_RuleGroup(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_networkfirewall_resource_policy.test" diff --git a/aws/resource_aws_s3_access_point_test.go b/aws/resource_aws_s3_access_point_test.go index 8dd9b040955..9dc1917e132 100644 --- a/aws/resource_aws_s3_access_point_test.go +++ b/aws/resource_aws_s3_access_point_test.go @@ -141,7 +141,7 @@ func TestAccAWSS3AccessPoint_disappears(t *testing.T) { }) } -func TestAccAWSS3AccessPoint_bucketDisappears(t *testing.T) { +func TestAccAWSS3AccessPoint_disappears_Bucket(t *testing.T) { var v s3control.GetAccessPointOutput bucketName := acctest.RandomWithPrefix("tf-acc-test") accessPointName := acctest.RandomWithPrefix("tf-acc-test") diff --git a/aws/resource_aws_s3_bucket_public_access_block_test.go b/aws/resource_aws_s3_bucket_public_access_block_test.go index 2dfafeab6aa..b065203e0fd 100644 --- a/aws/resource_aws_s3_bucket_public_access_block_test.go +++ b/aws/resource_aws_s3_bucket_public_access_block_test.go @@ -67,7 +67,7 @@ func TestAccAWSS3BucketPublicAccessBlock_disappears(t *testing.T) { }) } -func TestAccAWSS3BucketPublicAccessBlock_bucketDisappears(t *testing.T) { +func TestAccAWSS3BucketPublicAccessBlock_disappears_Bucket(t *testing.T) { var config s3.PublicAccessBlockConfiguration name := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt()) resourceName := "aws_s3_bucket_public_access_block.bucket" diff --git a/aws/resource_aws_wafv2_web_acl_logging_configuration_test.go b/aws/resource_aws_wafv2_web_acl_logging_configuration_test.go index 5a1768b75d5..bce42080a91 100644 --- a/aws/resource_aws_wafv2_web_acl_logging_configuration_test.go +++ b/aws/resource_aws_wafv2_web_acl_logging_configuration_test.go @@ -201,7 +201,7 @@ func TestAccAwsWafv2WebACLLoggingConfiguration_disappears(t *testing.T) { }) } -func TestAccAwsWafv2WebACLLoggingConfiguration_webACLDisappears(t *testing.T) { +func TestAccAwsWafv2WebACLLoggingConfiguration_disappears_WebAcl(t *testing.T) { var v wafv2.LoggingConfiguration rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_wafv2_web_acl_logging_configuration.test" diff --git a/docs/contributing/running-and-writing-acceptance-tests.md b/docs/contributing/running-and-writing-acceptance-tests.md index ad833ab99fc..b8ccab736c1 100644 --- a/docs/contributing/running-and-writing-acceptance-tests.md +++ b/docs/contributing/running-and-writing-acceptance-tests.md @@ -639,6 +639,32 @@ if err != nil { } ``` +For children resources that are encapsulated by a parent resource, it is also preferable to verify that removing the parent resource will not generate an error either. These are typically named `TestAccAws{SERVICE}{THING}_disappears_{PARENT}`, e.g. `TestAccAwsRoute53ZoneAssociation_disappears_Vpc` + +```go +func TestAccAwsExampleChildThing_disappears_ParentThing(t *testing.T) { + rName := acctest.RandomWithPrefix("tf-acc-test") + parentResourceName := "aws_example_parent_thing.test" + resourceName := "aws_example_child_thing.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsExampleChildThingDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAwsExampleThingConfigName(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsExampleThingExists(resourceName), + testAccCheckResourceDisappears(testAccProvider, resourceAwsExampleParentThing(), parentResourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} +``` + #### Per Attribute Acceptance Tests These are typically named `TestAccAws{SERVICE}{THING}_{ATTRIBUTE}`, e.g. `TestAccAwsCloudWatchDashboard_Name`