-
Notifications
You must be signed in to change notification settings - Fork 233
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
Adding RefreshState test step #1070
Changes from all commits
f9a2ca4
204d99e
468b72d
a5fad91
e17526a
51342f5
2f726cb
0f220c8
4be309c
034a8b7
bc221f7
830445e
9ec2d62
5be3380
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:enhancement | ||
helper/resource: Added `TestStep` type `RefreshState` field, which enables a step that refreshes state without an explicit apply or configuration changes | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
package resource | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/davecgh/go-spew/spew" | ||
tfjson "github.com/hashicorp/terraform-json" | ||
"github.com/mitchellh/go-testing-interface" | ||
|
||
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/plugintest" | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform" | ||
) | ||
|
||
func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir, step TestStep, providers *providerFactories) error { | ||
t.Helper() | ||
|
||
spewConf := spew.NewDefaultConfig() | ||
spewConf.SortKeys = true | ||
|
||
var err error | ||
// Explicitly ensure prior state exists before refresh. | ||
err = runProviderCommand(ctx, t, func() error { | ||
bendbennett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_, err = getState(ctx, t, wd) | ||
if err != nil { | ||
return err | ||
} | ||
return nil | ||
}, wd, providers) | ||
if err != nil { | ||
t.Fatalf("Error getting state: %s", err) | ||
} | ||
|
||
err = runProviderCommand(ctx, t, func() error { | ||
return wd.Refresh(ctx) | ||
}, wd, providers) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var refreshState *terraform.State | ||
err = runProviderCommand(ctx, t, func() error { | ||
refreshState, err = getState(ctx, t, wd) | ||
if err != nil { | ||
return err | ||
} | ||
return nil | ||
}, wd, providers) | ||
if err != nil { | ||
t.Fatalf("Error getting state: %s", err) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should refresh There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have added a plan following the check and the check against |
||
// Go through the refreshed state and verify | ||
if step.Check != nil { | ||
logging.HelperResourceDebug(ctx, "Calling TestStep Check for RefreshState") | ||
|
||
if err := step.Check(refreshState); err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
logging.HelperResourceDebug(ctx, "Called TestStep Check for RefreshState") | ||
} | ||
|
||
// do a plan | ||
err = runProviderCommand(ctx, t, func() error { | ||
return wd.CreatePlan(ctx) | ||
}, wd, providers) | ||
if err != nil { | ||
return fmt.Errorf("Error running post-apply plan: %w", err) | ||
} | ||
|
||
var plan *tfjson.Plan | ||
err = runProviderCommand(ctx, t, func() error { | ||
var err error | ||
plan, err = wd.SavedPlan(ctx) | ||
return err | ||
}, wd, providers) | ||
if err != nil { | ||
return fmt.Errorf("Error retrieving post-apply plan: %w", err) | ||
} | ||
|
||
if !planIsEmpty(plan) && !step.ExpectNonEmptyPlan { | ||
var stdout string | ||
err = runProviderCommand(ctx, t, func() error { | ||
var err error | ||
stdout, err = wd.SavedPlanRawStdout(ctx) | ||
return err | ||
}, wd, providers) | ||
if err != nil { | ||
return fmt.Errorf("Error retrieving formatted plan output: %w", err) | ||
} | ||
return fmt.Errorf("After refreshing state during this test step, a followup plan was not empty.\nstdout:\n\n%s", stdout) | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider what website documentation we can add around this. It'd also be good to denote here and in the website that its current intention is refresh, (potentially) plan, and optionally running checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field should also document that it cannot be the first
TestStep
and any other potential restrictions or contradictory definitions (such asImportState
+RefreshState
for example)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the documentation. In terms of the website docs, where do you think they should be added? The information around
ImportState
is in/plugin/sdkv2/resources
. As we're just discussing testing here should be add a new section or add/amend an area of the website docs that already exist?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think https://www.terraform.io/plugin/sdkv2/testing/acceptance-tests/teststep is okay enough for now. Ideally that page would probably be broken up by test mode, but adding information in the current information architecture should be okay. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have added a couple of lines. Let me know if you think this needs to be expanded.