ExpectError is ignored for test case where destroy action expected to have error

[IronSavior]

Maybe I'm doing this wrong?

SDK version

{
  "Path": "github.com/hashicorp/terraform-plugin-sdk/v2",
  "Version": "v2.0.2"
}

Relevant provider source code

// In provider resource DELETE function:
_, err := client.Remove(ctx, req)
if err != nil {
    logger.WithError(err).Error("could not remove")
    return diag.FromErr(err) // <-- Code path under test
}

// In test case:
client.On("Remove", mock.Anything, req).Return(nil, &err) // Set up mock client to return error

resource.UnitTest(t, resource.TestCase{
    Providers: testAccProviders,
    Steps: []resource.TestStep{{
        Config:      loadFixtureString("testdata/%s.tf", t.Name()),
        ExpectError: regexp.MustCompile(`.*`), // <-- should match any error at all
        Check: resource.ComposeAggregateTestCheckFunc(
            // irrelevant
        ),
    }},
})

Debug Output

Test output:

    testing_new.go:22: WARNING: destroy failed, so remote objects may still exist and be subject to billing
    testing_new.go:22: failed to destroy: 
        Error: could not remove

Expected Behavior

Test should pass because the delete operation is expected to fail

Actual Behavior

Test fails because the resource is not destroyed.

Steps to Reproduce

Create an apply/destroy test case where the destroy is guaranteed to fail.

References

https://github.com/hashicorp/terraform-plugin-sdk/issues/347 - Except their problem was during resource creation.

[sloloris]

I am hitting a similar issue. I want to test a case where the attempted deletion of a resource with dependencies should fail. The apply does fail with the expected error, but then the test itself errors out due to dangling resources:

    resource_launchdarkly_feature_flag_test.go:1027: Step 3/3 error: Error running apply: exit status 1

        Error: flag "prereq" in project "tx10w6a1r8" cannot be deleted: 400 Bad Request: {"code":"invalid_request","message":"A flag that is a prerequisite of other flags cannot be archived"}

    testing_new.go:70: Error running post-test destroy, there may be dangling resources: exit status 1

        Error: failed to delete flag "prereq" from project "tx10w6a1r8": 404 Not Found: {"message":"Unknown project key tx10w6a1r8"}

Are there any updates on this? Is there a way around this that anyone knows of?

[AniketKariya]

It's not a solution but a workaround. Try adding a step at the end which successfully creates the resources and it should work.

resource.UnitTest(t, resource.TestCase{
    Providers: testAccProviders,
    Steps: []resource.TestStep{{
        Config:      loadFixtureString("testdata/%s.tf", t.Name()),
        ExpectError: regexp.MustCompile(`.*`), // <-- should match any error at all
        Check: resource.ComposeAggregateTestCheckFunc(
            // irrelevant
        ),
                {
                    Config: SomeFuncThatSuccessfullyCreatesResource(resourceArgss),
                },
    }},
})

[matthewcummings]

Hmm. . . @AniketKariya that didn't work for me.

[matthewcummings]

I have a use case where destroys should only succeed/be allowed if the resource specifies force_destroy = true. It looks like it won't work as is:

https://github.com/hashicorp/terraform-plugin-sdk/blob/main/helper/resource/testing_new.go#L24

[matthewcummings]

Hasn't been merged yet but I have a PR to handle this in a way similar to ExpectError: https://github.com/hashicorp/terraform-plugin-sdk/pull/976

[bflad]

Hi folks 👋 The testing framework generally expects tests to always destroy successfully at the end of a TestCase to prevent dangling infrastructure and manual resource cleanup.

Does setting up the testing using a Destroy: true configured TestStep help?

resource.TestCase{
  // ... other fields ...
  Steps: []resource.TestStep{
    {
      Config: "# config that successfully creates resource, but requires forced destruction",
      // ... potentially other fields ...
    },
    {
      Config: "# config that should unsuccessfully destroy resource, potentially same as above Config",
      Destroy: true,
      ExpectError: regexp.MustCompile(/*...*/),
      // ... potentially other fields ...
    },
    {
      Config: "# config that successfully applies forced destruction settings to resource so it can be destroyed by TestCase"
      // ... potentially other fields ...
    },
  },
}

It should also be possible to set Config: "", if that is slightly clearer for signaling the desire to destroy all resources amidst the steps.

[matthewcummings]

@bflad that makes perfect sense, nonetheless it's perfectly normal for a provider to fail when deleting resources, intentionally, for any number of reasons. I spent some time looking into this (existing providers) and I came to the conclusion that people just don't have tests for such cases, which is not ideal.

Destroy: true doesn't help for my use case.

So let's flip this back to you - are you saying that you will not add/allow functionality which can handle (expected) failed deletes? I'm all for simplicity but in my case, being able to validate this behavior is desirable.

[detro]

Hi @matthewcummings and @IronSavior - something I think we need to make sure we check: have you tried to have a TestStep where Destroy: true and ExpectError: regexp is set?

Destroy, under the hood, changes drastically what the TestStep does: instead of terraform plan followed by terraform apply, it will do terraform plan -destroy followed by terraform apply. Effectively exercising the deletion of a resource.

Are you saying that you tested that, and found that in case of error, ExpectError would not actually be matched against it?

[matthewcummings]

Hi @detro yes, I have tried that. It doesn't work, when the post test destroy runs, if there's an error, there's no current way to handle it, see https://github.com/hashicorp/terraform-plugin-sdk/blob/main/helper/resource/testing_new.go#L32

[matthewcummings]

@detro any update on this?

[matthewcummings]

@detro checking again. . . any updates here?

[titaneric]

I have similar issue. I want to let provider user to explicitly set enabled = false to fully destroy the resource since the resource must be stop before it is deleting.

I have traced sdk source code, just like @matthewcummings found. Calling CheckDestory function is after runProviderCommand function calling, so there is no way to catch the error return from runProviderCommand and write custom error handling for it.

Although I have acknowledged that there are some resource will still left on tfstate, I want to execute sweepers to clean up the resource after acceptance test.

https://github.com/hashicorp/terraform-plugin-sdk/blob/ed85ac47867d82e7d1ae802b3b0be02333609ece/helper/resource/testing_new.go#L21-L34

[gmarciani]

Hey there, any update on this?

[TomerHeber]

This is an old thread - but there is a way to make it work: (similar to what was suggested above).

resource.TestCase{
  Steps: []resource.TestStep{
    {
      Config: {config that creates the resource},
      // ... potentially other fields .
    },
    {
      Config: {exactly the same config as above},
      Destroy: true, {this is important}
      ExpectError: {your expected error},
    },

Destroy will be called twice. First call - destroy should return the expected error. Second call - destroy must be successful (mock it accordingly).