From 9ee147ca9d868beea94263724517d587ff4010f6 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 29 Apr 2020 17:05:46 -0400 Subject: [PATCH] tests/provider: Introduce shared disappears TestCheckFunc, refactor Backup testing to implement and verify (#12864) * tests/provider: Introduce shared disappears TestCheckFunc, refactor Backup testing to implement and verify A common problem in Terraform Providers is the need to verify that a Terraform Resource will successfully remove itself from the Terraform state when externally deleted. In an effort to test this functionality, the Terraform AWS Provider has implemented testing, commonly named `_disappears` in each resource's acceptance testing, where each test includes a `TestCheckFunc` that manually reimplements the deletion functionality of the resource. This change proposes the introduction of a shared `TestCheckFunc` that can prevent the necessity of manually reimplementing the resource deletion code in a `TestCheckFunc` for each resource. If acceptable, a GitHub issue will be created outlining a refactoring and documentation proposal for the provider codebase. It is worth noting that in the future, the Terraform Plugin SDK's acceptance testing framework could introduce native functionality for this type of common testing, but this shared `TestCheckFunc` is an effort to reduce a current development pain point. Output from acceptance testing: ``` --- PASS: TestAccAwsBackupVault_disappears (13.75s) --- PASS: TestAccAwsBackupPlan_disappears (18.15s) --- PASS: TestAccAwsBackupSelection_disappears (24.02s) ``` * tests/provider: Remove extra colon in testAccCheckResourceDisappears error message Reference: https://github.com/terraform-providers/terraform-provider-aws/pull/12864#pullrequestreview-394967599 --- aws/provider_test.go | 16 ++++++++++++++++ aws/resource_aws_backup_plan_test.go | 16 +--------------- aws/resource_aws_backup_selection_test.go | 21 ++++----------------- aws/resource_aws_backup_vault_test.go | 14 +------------- 4 files changed, 22 insertions(+), 45 deletions(-) diff --git a/aws/provider_test.go b/aws/provider_test.go index 84a84f102184..5c76f99e72e8 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -587,6 +587,22 @@ func testAccAwsRegionProviderFunc(region string, providers *[]*schema.Provider) } } +func testAccCheckResourceDisappears(provider *schema.Provider, resource *schema.Resource, resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + resourceState, ok := s.RootModule().Resources[resourceName] + + if !ok { + return fmt.Errorf("resource not found: %s", resourceName) + } + + if resourceState.Primary.ID == "" { + return fmt.Errorf("resource ID missing: %s", resourceName) + } + + return resource.Delete(resource.Data(resourceState.Primary), provider.Meta()) + } +} + func testAccCheckWithProviders(f func(*terraform.State, *schema.Provider) error, providers *[]*schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { numberOfProviders := len(*providers) diff --git a/aws/resource_aws_backup_plan_test.go b/aws/resource_aws_backup_plan_test.go index 85c461312506..028771c64a5f 100644 --- a/aws/resource_aws_backup_plan_test.go +++ b/aws/resource_aws_backup_plan_test.go @@ -465,7 +465,7 @@ func TestAccAwsBackupPlan_disappears(t *testing.T) { Config: testAccAwsBackupPlanConfig_basic(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAwsBackupPlanExists(resourceName, &plan, &ruleNameMap), - testAccCheckAwsBackupPlanDisappears(&plan), + testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupPlan(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -496,20 +496,6 @@ func testAccCheckAwsBackupPlanDestroy(s *terraform.State) error { return nil } -func testAccCheckAwsBackupPlanDisappears(backupPlan *backup.GetBackupPlanOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).backupconn - - input := &backup.DeleteBackupPlanInput{ - BackupPlanId: backupPlan.BackupPlanId, - } - - _, err := conn.DeleteBackupPlan(input) - - return err - } -} - func testAccCheckAwsBackupPlanExists(name string, plan *backup.GetBackupPlanOutput, ruleNameMap *map[string]string) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).backupconn diff --git a/aws/resource_aws_backup_selection_test.go b/aws/resource_aws_backup_selection_test.go index 59c8bb243926..9d864d6a34ef 100644 --- a/aws/resource_aws_backup_selection_test.go +++ b/aws/resource_aws_backup_selection_test.go @@ -39,6 +39,8 @@ func TestAccAwsBackupSelection_basic(t *testing.T) { func TestAccAwsBackupSelection_disappears(t *testing.T) { var selection1 backup.GetBackupSelectionOutput rInt := acctest.RandInt() + resourceName := "aws_backup_selection.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBackup(t) }, Providers: testAccProviders, @@ -47,8 +49,8 @@ func TestAccAwsBackupSelection_disappears(t *testing.T) { { Config: testAccBackupSelectionConfigBasic(rInt), Check: resource.ComposeTestCheckFunc( - testAccCheckAwsBackupSelectionExists("aws_backup_selection.test", &selection1), - testAccCheckAwsBackupSelectionDisappears(&selection1), + testAccCheckAwsBackupSelectionExists(resourceName, &selection1), + testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupSelection(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -190,21 +192,6 @@ func testAccCheckAwsBackupSelectionExists(name string, selection *backup.GetBack } } -func testAccCheckAwsBackupSelectionDisappears(selection *backup.GetBackupSelectionOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).backupconn - - input := &backup.DeleteBackupSelectionInput{ - BackupPlanId: selection.BackupPlanId, - SelectionId: selection.SelectionId, - } - - _, err := conn.DeleteBackupSelection(input) - - return err - } -} - func testAccCheckAwsBackupSelectionRecreated(t *testing.T, before, after *backup.GetBackupSelectionOutput) resource.TestCheckFunc { return func(s *terraform.State) error { diff --git a/aws/resource_aws_backup_vault_test.go b/aws/resource_aws_backup_vault_test.go index 13e319c3965f..5ce6282c93f1 100644 --- a/aws/resource_aws_backup_vault_test.go +++ b/aws/resource_aws_backup_vault_test.go @@ -124,7 +124,7 @@ func TestAccAwsBackupVault_disappears(t *testing.T) { Config: testAccBackupVaultConfig(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsBackupVaultExists(resourceName, &vault), - testAccCheckAwsBackupVaultDisappears(&vault), + testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupVault(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -177,18 +177,6 @@ func testAccCheckAwsBackupVaultExists(name string, vault *backup.DescribeBackupV } } -func testAccCheckAwsBackupVaultDisappears(vault *backup.DescribeBackupVaultOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).backupconn - params := &backup.DeleteBackupVaultInput{ - BackupVaultName: vault.BackupVaultName, - } - _, err := conn.DeleteBackupVault(params) - - return err - } -} - func testAccPreCheckAWSBackup(t *testing.T) { conn := testAccProvider.Meta().(*AWSClient).backupconn