Migrate Separate ImportState Tests Into All Applicable Tests

[bflad]

Description

An older common practice with acceptance testing was creating a separate acceptance test, that performed the ImportState testing:

func TestAccExampleThing_basic(t *testing.T) { // not flagged!
    resource.ParallelTest(t, resource.TestCase{
        PreCheck:     func() { testAccPreCheck(t) },
        Providers:    testAccProviders,
        CheckDestroy: testAccCheckExampleThingDestroy,
        Steps: []resource.TestStep{
            {
                Config: testAccExampleThingConfig(),
                Check: resource.ComposeTestCheckFunc(
                    resource.TestCheckResourceAttrSet("example_thing.test", "attr1"),
                ),
            },
        },
    })
}

func TestAccExampleThing_importBasic(t *testing.T) { // flagged!
    resource.ParallelTest(t, resource.TestCase{
        PreCheck:     func() { testAccPreCheck(t) },
        Providers:    testAccProviders,
        CheckDestroy: testAccCheckExampleThingDestroy,
        Steps: []resource.TestStep{
            {
                Config: testAccExampleThingConfig(),
                Check: resource.ComposeTestCheckFunc(
                    resource.TestCheckResourceAttrSet("example_thing.test", "attr1"),
                ),
            },
            {
                ResourceName:      "example_thing.test",
                ImportState:       true,
                ImportStateVerify: true,
            },
        },
    })
}

This testing setup creates duplicate test infrastructure for little benefit (only an additional API call) and can miss other permutations of Terraform configurations that do not successfully import. It is generally preferred to just add the ImportState testing to all existing applicable tests (except tests like _disappears tests, which the infrastructure won't be importable).

In the above example, its preferable to just have:

func TestAccExampleThing_basic(t *testing.T) {
    resource.ParallelTest(t, resource.TestCase{
        PreCheck:     func() { testAccPreCheck(t) },
        Providers:    testAccProviders,
        CheckDestroy: testAccCheckExampleThingDestroy,
        Steps: []resource.TestStep{
            {
                Config: testAccExampleThingConfig(),
                Check: resource.ComposeTestCheckFunc(
                    resource.TestCheckResourceAttrSet("example_thing.test", "attr1"),
                ),
            },
            {
                ResourceName:      "example_thing.test",
                ImportState:       true,
                ImportStateVerify: true,
            },
        },
    })
}

Many of these in the Terraform AWS Provider can be found by enabling the tfproviderlint AT002 check, since the acceptance test function naming scheme included _import or _Import. Some resources may require updates to fix ImportStateVerify issues. There are also some false positive test names that we can have the linter ignore via: //lintignore:AT002

Definition of Done

• In GNUmakefile, add -AT002 to tfproviderlint command under lint target and have TravisCI testing pass

[ghost]

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!