Fix migration to terraform-plugin-testing causes panic in existing te…

…st suite (#1167)

* helper/resource: Deprecating NotFoundError, UnexpectedStateError, TimeoutError, StateRefreshFunc, StateChangeConf, RetryFunc, RetryContext, Retry, RetryError, RetryableError and NonRetryableError

Equivalent types and functions are now available in the helper/retry package (#1166)

* Deprecating helper/resource PrefixedUniqueId() and `UniqueId()` (#1167)

* Responding to code review feedback (#1167)

* Expanding aliases to include helper/id

* Expanding contents of changie notes and adding enhancement entries

* Updating documentation to refer to import of helper/retry

.changes/unreleased/ENHANCEMENTS-20230310-132324.yaml

@@ -0,0 +1,7 @@
+kind: ENHANCEMENTS
+body: 'helper/id: New `helper/id` package added. `resource.PrefixedUniqueId()` and
+  `resource.UniqueId()` are deprecated, `helper/id` should be used instead. `helper/resource`
+  now contains aliases to the migrated code'
+time: 2023-03-10T13:23:24.648529Z
+custom:
+  Issue: "1167"

.changes/unreleased/ENHANCEMENTS-20230310-132539.yaml

@@ -0,0 +1,7 @@
+kind: ENHANCEMENTS
+body: 'helper/retry: New `helper/retry` package added. `resource.RetryContext()`,
+  `resource.StateChangeConf`, and associated `*Error` types are deprecated, `helper/retry`
+  should be used instead. `helper/resource now contains aliases to the migrated code'
+time: 2023-03-10T13:25:39.920985Z
+custom:
+  Issue: "1167"

.changes/unreleased/NOTES-20230310-131553.yaml

@@ -0,0 +1,6 @@
+kind: NOTES
+body: 'helper/resource: Deprecated `PrefixedUniqueId()` and `UniqueId()`. Use the
+  `helper/id` package instead. These deprecations are to assist in migrating to terraform-plugin-testing'
+time: 2023-03-10T13:15:53.740137Z
+custom:
+  Issue: "1167"

.changes/unreleased/NOTES-20230310-131709.yaml

@@ -0,0 +1,7 @@
+kind: NOTES
+body: 'helper/resource: Deprecated `RetryContext()`, `StateChangeConf`, and associated
+  `*Error` types. Use the `helper/retry` package instead. These deprecations are to
+  assist in migrating to terraform-plugin-testing'
+time: 2023-03-10T13:17:09.544612Z
+custom:
+  Issue: "1167"

helper/resource/id.go → helper/id/id.go

@@ -1,4 +1,4 @@
-package resource
+package id
 
 import (
 	"fmt"

helper/resource/id_test.go → helper/id/id_test.go

@@ -1,4 +1,4 @@
-package resource
+package id
 
 import (
 	"regexp"

helper/resource/aliases.go

@@ -0,0 +1,142 @@
+package resource
+
+import (
+	"context"
+	"time"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
+)
+
+// Deprecated: Use helper/id package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+const UniqueIdPrefix = id.UniqueIdPrefix
+
+// Helper for a resource to generate a unique identifier w/ default prefix
+//
+// Deprecated: Use helper/id package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func UniqueId() string {
+	return id.UniqueId()
+}
+
+// Deprecated: Use helper/id package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+const UniqueIDSuffixLength = id.UniqueIDSuffixLength
+
+// Helper for a resource to generate a unique identifier w/ given prefix
+//
+// After the prefix, the ID consists of an incrementing 26 digit value (to match
+// previous timestamp output).  After the prefix, the ID consists of a timestamp
+// and an incrementing 8 hex digit value The timestamp means that multiple IDs
+// created with the same prefix will sort in the order of their creation, even
+// across multiple terraform executions, as long as the clock is not turned back
+// between calls, and as long as any given terraform execution generates fewer
+// than 4 billion IDs.
+//
+// Deprecated: Use helper/id package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func PrefixedUniqueId(prefix string) string {
+	return id.PrefixedUniqueId(prefix)
+}
+
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type NotFoundError retry.NotFoundError
+
+// UnexpectedStateError is returned when Refresh returns a state that's neither in Target nor Pending
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type UnexpectedStateError retry.UnexpectedStateError
+
+// TimeoutError is returned when WaitForState times out
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type TimeoutError retry.TimeoutError
+
+// StateRefreshFunc is a function type used for StateChangeConf that is
+// responsible for refreshing the item being watched for a state change.
+//
+// It returns three results. `result` is any object that will be returned
+// as the final object after waiting for state change. This allows you to
+// return the final updated object, for example an EC2 instance after refreshing
+// it. A nil result represents not found.
+//
+// `state` is the latest state of that object. And `err` is any error that
+// may have happened while refreshing the state.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type StateRefreshFunc retry.StateRefreshFunc
+
+// StateChangeConf is the configuration struct used for `WaitForState`.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type StateChangeConf retry.StateChangeConf
+
+// RetryFunc is the function retried until it succeeds.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type RetryFunc retry.RetryFunc
+
+// RetryContext is a basic wrapper around StateChangeConf that will just retry
+// a function until it no longer returns an error.
+//
+// Cancellation from the passed in context will propagate through to the
+// underlying StateChangeConf
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func RetryContext(ctx context.Context, timeout time.Duration, f RetryFunc) error {
+	return retry.RetryContext(ctx, timeout, retry.RetryFunc(f))
+}
+
+// Retry is a basic wrapper around StateChangeConf that will just retry
+// a function until it no longer returns an error.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func Retry(timeout time.Duration, f RetryFunc) error {
+	return retry.Retry(timeout, retry.RetryFunc(f))
+}
+
+// RetryError is the required return type of RetryFunc. It forces client code
+// to choose whether or not a given error is retryable.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+type RetryError retry.RetryError
+
+// RetryableError is a helper to create a RetryError that's retryable from a
+// given error. To prevent logic errors, will return an error when passed a
+// nil error.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func RetryableError(err error) *RetryError {
+	r := retry.RetryableError(err)
+
+	return &RetryError{
+		Err:       r.Err,
+		Retryable: r.Retryable,
+	}
+}
+
+// NonRetryableError is a helper to create a RetryError that's _not_ retryable
+// from a given error. To prevent logic errors, will return an error when
+// passed a nil error.
+//
+// Deprecated: Use helper/retry package instead. This is required for migrating acceptance
+// testing to terraform-plugin-testing.
+func NonRetryableError(err error) *RetryError {
+	r := retry.NonRetryableError(err)
+
+	return &RetryError{
+		Err:       r.Err,
+		Retryable: r.Retryable,
+	}
+}

helper/resource/testcase_providers_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )

helper/resource/error.go → helper/retry/error.go

@@ -1,4 +1,4 @@
-package resource
+package retry
 
 import (
 	"fmt"

helper/resource/state.go → helper/retry/state.go

@@ -1,4 +1,4 @@
-package resource
+package retry
 
 import (
 	"context"

helper/resource/state_test.go → helper/retry/state_test.go

@@ -1,4 +1,4 @@
-package resource
+package retry
 
 import (
 	"context"

helper/resource/wait.go → helper/retry/wait.go

@@ -1,4 +1,4 @@
-package resource
+package retry
 
 import (
 	"context"

helper/resource/wait_test.go → helper/retry/wait_test.go

@@ -1,4 +1,4 @@
-package resource
+package retry
 
 import (
 	"context"

website/docs/plugin/sdkv2/guides/v2-upgrade-guide.mdx

@@ -30,7 +30,7 @@ Upgrade topics:
 * [Louder `helper/schema.ResourceData.Set` Errors in Testing](#louder-helper-schema-resourcedata-set-errors-in-testing)
 * [More Consistent Usage of `github.com/mitchellh/go-testing-interface`](#more-consistent-usage-of-github-com-mitchellh-go-testing-interface)
 * [Corrected Results of `helper/acctest.RandIntRange`](#corrected-results-of-helper-acctest-randintrange)
-* [Clearer Handling of `nil` for `helper/resource.NonRetryableError` and `helper/resource.RetryableError`](#clearer-handling-of-nil-for-helper-resource-nonretryableerror-and-helper-resource-retryableerror)
+* [Clearer Handling of `nil` for `helper/retry.RetryableError` and `helper/retry.RetryableError`](#clearer-handling-of-nil-for-helper-resource-nonretryableerror-and-helper-resource-retryableerror)
 * [More Robust Handling of `helper/schema.TypeSet` Hashes](#more-robust-handling-of-helper-schema-typeset-hashes)
 * [Stronger Validation for `helper/schema.Schema.Computed` Fields](#stronger-validation-for-helper-schema-schema-computed-fields)
 * [More Robust Validation of `helper/schema.TypeMap` `Elem`s](#more-robust-validation-of-helper-schema-typemap-elems)
@@ -287,13 +287,13 @@ integer between the arguments. It has been corrected to return an integer
 between the arguments. Any usage reliant on the old, buggy behavior should be
 updated.
 
-## Clearer Handling of `nil` for `helper/resource.NonRetryableError` and `helper/resource.RetryableError`
+## Clearer Handling of `nil` for `helper/retry.RetryableError` and `helper/retry.RetryableError`
 
-Previously, passing a `nil` error to `helper/resource.RetryableError` and
-`helper/resource.NonRetryableError` could lead to subtle bugs and even crashes.
+Previously, passing a `nil` error to `helper/retry.RetryableError` and
+`helper/retry.RetryableError` could lead to subtle bugs and even crashes.
 It will now return an error. Providers should check that the error is not `nil`
-before calling `helper/resource.NonRetryableError` or
-`helper/resource.RetryableError`.
+before calling `helper/retry.RetryableError` or
+`helper/retry.RetryableError`.
 
 ## More Robust Handling of `helper/schema.TypeSet` Hashes
 

website/docs/plugin/sdkv2/resources/retries-and-customizable-timeouts.mdx

@@ -76,9 +76,9 @@ If a CRUD function timeout is exceeded, the SDK will automatically return a `con
 The retry helper takes a timeout and a retry function.
 
 - The **timeout** value specifies the maximum time Terraform will invoke the retry function. You can retrieve the timeout from the `*schema.ResourceData` struct by passing the timeout key (`schema.TimeoutCreate`) to the `Timeout` method.
-- The **retry function** returns either a `resource.NonRetryableError` for unexpected errors/states or a `resource.RetryableError` for expected errrors/states. If the function returns a `resource.RetryableError`, it will re-run the function.
+- The **retry function**, which is available by importing `github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry`, returns either a `retry.NonRetryableError` for unexpected errors/states or a `retry.RetryableError` for expected errrors/states. If the function returns a `retry.RetryableError`, it will re-run the function.
 
-In the context of a `CREATE` function, once the backend responds with the desired state, invoke the `READ` function. If `READ` errors, return that error wrapped with `resource.NonRetryableError`. Otherwise, return `nil` (no error) from the retry function.
+In the context of a `CREATE` function, once the backend responds with the desired state, invoke the `READ` function. If `READ` errors, return that error wrapped with `retry.NonRetryableError`. Otherwise, return `nil` (no error) from the retry function.
 
 ```go
 func resourceExampleInstanceCreate(d *schema.ResourceData, meta any) error {
@@ -90,20 +90,20 @@ func resourceExampleInstanceCreate(d *schema.ResourceData, meta any) error {
         return fmt.Errorf("Error creating instance: %s", err)
     }
 
-    return resource.Retry(d.Timeout(schema.TimeoutCreate) - time.Minute, func() *resource.RetryError {
+    return retry.Retry(d.Timeout(schema.TimeoutCreate) - time.Minute, func() *retry.RetryError {
         resp, err := client.DescribeInstance(name)
 
         if err != nil {
-            return resource.NonRetryableError(fmt.Errorf("Error describing instance: %s", err))
+            return retry.NonRetryableError(fmt.Errorf("Error describing instance: %s", err))
         }
 
         if resp.Status != "CREATED" {
-            return resource.RetryableError(fmt.Errorf("Expected instance to be created but was in state %s", resp.Status))
+            return retry.RetryableError(fmt.Errorf("Expected instance to be created but was in state %s", resp.Status))
         }
 
         err = resourceExampleInstanceRead(d, meta)
         if err != nil {
-            return resource.NonRetryableError(err)
+            return retry.NonRetryableError(err)
         } else {
             return nil
         }
@@ -115,17 +115,17 @@ func resourceExampleInstanceCreate(d *schema.ResourceData, meta any) error {
 
 ## StateChangeConf
 
-`resource.Retry` is useful for simple scenarios, particularly when the API response is either success or failure, but sometimes handling an APIs latency or eventual consistency requires more fine tuning. `resource.Retry` is in fact a wrapper for a another helper: `resource.StateChangeConf`.
+`retry.Retry` is useful for simple scenarios, particularly when the API response is either success or failure, but sometimes handling an APIs latency or eventual consistency requires more fine tuning. `retry.Retry` is in fact a wrapper for a another helper: `retry.StateChangeConf`.
 
-Use `resource.StateChangeConf` when your resource has multiple states to progress though, you require fine grained control of retry and delay timing, or you want to ensure a minimum number of occurrences of a target state is reached (this is very common when dealing with eventually consistent APIs, where a response can reply back with an old state between calls before becoming consistent).
+Use `retry.StateChangeConf` when your resource has multiple states to progress though, you require fine grained control of retry and delay timing, or you want to ensure a minimum number of occurrences of a target state is reached (this is very common when dealing with eventually consistent APIs, where a response can reply back with an old state between calls before becoming consistent).
 
 ```go
 func resourceExampleInstanceCreate(d *schema.ResourceData, meta any) error {
     name := d.Get("name").(string)
     client := meta.(*ExampleClient)
     resp, err := client.CreateInstance(name)
 
-    createStateConf := &resource.StateChangeConf{
+    createStateConf := &retry.StateChangeConf{
         Pending: []string{
             client.ExampleInstanceStateRequesting,
             client.ExampleInstanceStatePending,