Skip to content

Conversation

@YakDriver
Copy link
Member

@YakDriver YakDriver commented Oct 29, 2025

Description

NOTE: The Use modern Go commit adds about 400 lines of modern Go cleanup. I can split that from these changes if you prefer.

This PR adds ActionChecks - a new testing capability that validates progress messages from Terraform actions during test execution.

New ActionChecks framework following the same patterns as existing StateChecks and PlanChecks:

  • TestCheckProgressMessageContains() - verify messages contain expected content
  • TestCheckProgressMessageCount() - verify expected number of messages
  • TestCheckProgressMessageSequence() - verify messages appear in expected order

Provider protocol interception to capture action progress messages in real-time:

  • Wraps both tfprotov5 and tfprotov6 providers during test execution
  • Intercepts InvokeAction calls to capture resp.SendProgress() messages
  • Thread-safe progress message storage and retrieval

Integration with TestStep:

  • New ActionChecks []actioncheck.ActionCheck field in TestStep
  • Automatic progress capture when ActionChecks are present
  • Validation runs after test step completion

Usage example:

resource.TestStep{
    Config: actionConfig,
    ActionChecks: []actioncheck.ActionCheck{
        resource.TestCheckProgressMessageContains("aws_lambda_invoke.test", "invoked successfully"),
        resource.TestCheckProgressMessageCount("aws_lambda_invoke.test", 2),
    },
}

This enables testing of action implementations like AWS Lambda invoke, CloudFront invalidation, etc. to verify they produce expected progress output for users.

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

@YakDriver YakDriver requested a review from a team as a code owner October 29, 2025 18:01
Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @YakDriver, I like the overall concept of using interceptors to capture progress messages. I have a couple of comments regarding overall design and some of the functionality.

Comment on lines +2185 to +2215
func TestCheckProgressMessageContains(actionName, expectedContent string) actioncheck.ActionCheck {
return actioncheck.ExpectProgressMessageContains(actionName, expectedContent)
}

// TestCheckProgressMessageCount returns an ActionCheck that verifies the expected number of progress messages.
//
// Example usage:
//
// resource.TestStep{
// Config: testConfig,
// ActionChecks: []actioncheck.ActionCheck{
// resource.TestCheckProgressMessageCount("aws_lambda_invoke.test", 2),
// },
// }
func TestCheckProgressMessageCount(actionName string, expectedCount int) actioncheck.ActionCheck {
return actioncheck.ExpectProgressCount(actionName, expectedCount)
}

// TestCheckProgressMessageSequence returns an ActionCheck that verifies progress messages appear in the expected sequence.
//
// Example usage:
//
// resource.TestStep{
// Config: testConfig,
// ActionChecks: []actioncheck.ActionCheck{
// resource.TestCheckProgressMessageSequence("aws_lambda_invoke.test", []string{"Invoking", "completed successfully"}),
// },
// }
func TestCheckProgressMessageSequence(actionName string, expectedSequence []string) actioncheck.ActionCheck {
return actioncheck.ExpectProgressSequence(actionName, expectedSequence)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For newer checks, we would prefer the functions to be in the same file where it's struct is defined for readability. This also helps distinguish the checks by package name:

 actioncheck.TestCheckProgressMessageCount("framework_modify_file_action", 2),

// resource.TestCheckProgressMessageContains("aws_lambda_invoke.test", "Lambda function logs:"),
// },
// }
func TestCheckProgressMessageContains(actionName, expectedContent string) actioncheck.ActionCheck {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func TestCheckProgressMessageContains(actionName, expectedContent string) actioncheck.ActionCheck {
func ExpectProgressMessageContains(actionName, expectedContent string) actioncheck.ActionCheck {

// resource.TestCheckProgressMessageCount("aws_lambda_invoke.test", 2),
// },
// }
func TestCheckProgressMessageCount(actionName string, expectedCount int) actioncheck.ActionCheck {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func TestCheckProgressMessageCount(actionName string, expectedCount int) actioncheck.ActionCheck {
func ExpectProgressMessageCount(actionName string, expectedCount int) actioncheck.ActionCheck {

// resource.TestCheckProgressMessageSequence("aws_lambda_invoke.test", []string{"Invoking", "completed successfully"}),
// },
// }
func TestCheckProgressMessageSequence(actionName string, expectedSequence []string) actioncheck.ActionCheck {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func TestCheckProgressMessageSequence(actionName string, expectedSequence []string) actioncheck.ActionCheck {
func ExpectProgressMessageSequence(actionName string, expectedSequence []string) actioncheck.ActionCheck {

`,
ActionChecks: []actioncheck.ActionCheck{
// Check that the action produces a message containing log output
resource.TestCheckProgressMessageContains("aws_lambda_invoke.test", "Lambda function logs:"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
resource.TestCheckProgressMessageContains("aws_lambda_invoke.test", "Lambda function logs:"),
resource.TestCheckProgressMessageContains("aws_lambda_invoke", "Lambda function logs:"),

The examples as written do not work because the provider does not have access to the alias name so you can only reference messages by action type name.

resource.TestCheckProgressMessageCount("aws_lambda_invoke.test", 2),

// Check that messages appear in the expected sequence
resource.TestCheckProgressMessageSequence("aws_lambda_invoke.test", []string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
resource.TestCheckProgressMessageSequence("aws_lambda_invoke.test", []string{
resource.TestCheckProgressMessageSequence("aws_lambda_invoke", []string{

func runActionChecks(ctx context.Context, actionChecks []actioncheck.ActionCheck, wd *plugintest.WorkingDir) error {
progressCapture := wd.GetProgressCapture()

// IMPORTANT: Unlike StateChecks and PlanChecks which retrieve persisted data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really like this comment clarifying the source of the progress messages!

Comment on lines +21 to +23
if req.ActionName != e.actionName {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, if the provider developer mistypes the action name in the check or if the expected action never produces any progress messages, the check will still pass without an error.

I think that we need to pass the entire map of messages from ProcessCapture into the CheckActionRequest so that each action check can throw an error if the action type name doesn't exist in the map.

)

// ActionCheck defines an interface for implementing test logic that checks action progress messages.
type ActionCheck interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wonder if the interface name should make it clear that these checks assert against intercepted provider messages rather than a standard Terraform artifact. Maybe there could be a future where there is a terraform-produced artifact for Actions and we would like a different interface for those types of checks? I don't have a strong opinion on this right now 🤔

t.Helper()

// Enable progress capture if action checks are defined - do this early before any provider operations
var progressCaptureEnabled bool
Copy link
Contributor

@SBGoods SBGoods Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like having this flag for process capture here, it helps reduce the performance cost of the interceptors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants