Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error returned from CheckDestroy is ignored #571

Closed
lsierant opened this issue Sep 10, 2020 · 1 comment · Fixed by #581
Closed

Error returned from CheckDestroy is ignored #571

lsierant opened this issue Sep 10, 2020 · 1 comment · Fixed by #581
Assignees
Labels
bug Something isn't working

Comments

@lsierant
Copy link

lsierant commented Sep 10, 2020

SDK version

v2.0.1

Relevant provider source code

In helper/resource/testing_new.go result from runPostTestDestroy is ignored:

func runNewTest(t testing.T, c TestCase, helper *tftest.Helper) {
// ...
    if !stateIsEmpty(statePreDestroy) {
        runPostTestDestroy(t, c, wd, c.ProviderFactories)
    }

runPostTestDestroy invokes CheckDestroy and bubbles up the error.

Expected Behavior

When CheckDestroy function returns an error, test should fail. Error returned from CheckDestroy indicates that some resources still exist or there was a problem deleting it.

Actual Behavior

Test is passing despite returning error from CheckDestroy.

Steps to Reproduce

  1. Return an error in any CheckDestroy function.
  2. Tests still will pass.

References

Change was introduced in April in #356 and #393. Fix is trivial:

    if !stateIsEmpty(statePreDestroy) {
        if err := runPostTestDestroy(t, c, wd, c.ProviderFactories); err != nil {
            t.Fatal(err)
        }
    }
@lsierant lsierant added the bug Something isn't working label Sep 10, 2020
@paddycarver paddycarver self-assigned this Sep 12, 2020
paddycarver added a commit that referenced this issue Sep 12, 2020
We were silently discarding the error instead of surfacing it when there
was an error returned from our post-test destroy code responsible for
tearing down infrastructure. Let's tell the user so they know
infrastructure may be dangling, and can see what went wrong and ideally
fix it.

Fixes #571.
paddycarver added a commit that referenced this issue Sep 15, 2020
We were silently discarding the error instead of surfacing it when there
was an error returned from our post-test destroy code responsible for
tearing down infrastructure. Let's tell the user so they know
infrastructure may be dangling, and can see what went wrong and ideally
fix it.

Fixes #571.
@ghost
Copy link

ghost commented Oct 16, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants