review comments

Author: stephybun

helper/resource/query/query_checks.go

@@ -14,41 +14,35 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/querycheck"
 )
 
-func RunQueryChecks(ctx context.Context, t testing.T, query *[]tfjson.LogMsg, queryChecks []querycheck.QueryResultCheck) error {
+func RunQueryChecks(ctx context.Context, t testing.T, query []tfjson.LogMsg, queryChecks []querycheck.QueryResultCheck) error {
 	t.Helper()
 
 	var result []error
 
-	if query == nil || len(*query) == 0 {
-		result = append(result, fmt.Errorf("No query results found"))
+	if query == nil {
+		result = append(result, fmt.Errorf("no query results found"))
 	}
 
 	found := make([]tfjson.ListResourceFoundData, 0)
-	complete := tfjson.ListCompleteData{}
+	summary := tfjson.ListCompleteData{}
 
-	for _, msg := range *query {
+	for _, msg := range query {
 		switch v := msg.(type) {
 		case tfjson.ListResourceFoundMessage:
 			found = append(found, v.ListResourceFound)
 		case tfjson.ListCompleteMessage:
-			complete = v.ListComplete
+			summary = v.ListComplete
 			// TODO diagnostics and errors?
 		default:
-			// ignore other message types
+			continue
 		}
 	}
 
-	// TODO check diagnostics in LogMsg to see if there are any errors we can return here?
-	var err error
-	if len(found) == 0 {
-		return fmt.Errorf("no resources found by query: %+v", err)
-	}
-
 	for _, queryCheck := range queryChecks {
 		resp := querycheck.CheckQueryResponse{}
 		queryCheck.CheckQuery(ctx, querycheck.CheckQueryRequest{
-			Query:          &found,
-			CompletedQuery: &complete,
+			Query:        found,
+			QuerySummary: &summary,
 		}, &resp)
 
 		result = append(result, resp.Error)

helper/resource/query/query_test.go

@@ -64,7 +64,7 @@ func TestQuery(t *testing.T) {
 					}
 				}
 				`,
-				ConfigQueryResultChecks: []querycheck.QueryResultCheck{
+				QueryResultChecks: []querycheck.QueryResultCheck{
 					querycheck.ExpectIdentity("examplecloud_containerette.test", map[string]knownvalue.Check{
 						"id":       knownvalue.StringExact("westeurope/somevalue1"),
 						"location": knownvalue.StringExact("westeurope"),
@@ -110,7 +110,7 @@ func TestQuery(t *testing.T) {
 					}
 				}
 				`,
-				ConfigQueryResultChecks: []querycheck.QueryResultCheck{
+				QueryResultChecks: []querycheck.QueryResultCheck{
 					querycheck.ExpectLength("examplecloud_containerette.test", knownvalue.Int64Exact(3)),
 					querycheck.ExpectLength("examplecloud_containerette.test2", knownvalue.Int64Exact(3)),
 				},
@@ -134,7 +134,7 @@ func TestQuery(t *testing.T) {
 					}
 				}
 				`,
-				ConfigQueryResultChecks: []querycheck.QueryResultCheck{
+				QueryResultChecks: []querycheck.QueryResultCheck{
 					querycheck.ExpectLengthAtLeast("examplecloud_containerette.test", 2),
 					querycheck.ExpectLengthAtLeast("examplecloud_containerette.test2", 1),
 				},

helper/resource/testing.go

@@ -642,9 +642,9 @@ type TestStep struct {
 	// Custom state checks can be created by implementing the [statecheck.StateCheck] interface, or by using a StateCheck implementation from the provided [statecheck] package.
 	ConfigStateChecks []statecheck.StateCheck
 
-	// ConfigQueryResultChecks allow assertions to be made against the query file during a Config test using a query check.
+	// QueryResultChecks allow assertions to be made against a collection of found resources that were returned by a query using a query check.
 	// Custom query checks can be created by implementing the [querycheck.QueryResultCheck] interface, or by using a QueryResultCheck implementation from the provided [querycheck] package.
-	ConfigQueryResultChecks []querycheck.QueryResultCheck
+	QueryResultChecks []querycheck.QueryResultCheck
 
 	// PlanOnly can be set to only run `plan` with this configuration, and not
 	// actually apply it. This is useful for ensuring config changes result in
@@ -861,21 +861,6 @@ type ConfigPlanChecks struct {
 	PostApplyPostRefresh []plancheck.PlanCheck
 }
 
-// ConfigQueryChecks defines the different points in a Config TestStep when query checks can be run.
-type ConfigQueryChecks struct {
-	// PreApply runs all query checks in the slice. This occurs before the apply of a Config test is run. This slice cannot be populated
-	// with TestStep.QueryOnly, as there is no PreApply query run with that flag set. All errors by query checks in this slice are aggregated, reported, and will result in a test failure.
-	PreApply []querycheck.QueryResultCheck
-
-	// PostApplyPreRefresh runs all query checks in the slice. This occurs after the apply and before the refresh of a Config test is run.
-	// All errors by query checks in this slice are aggregated, reported, and will result in a test failure.
-	PostApplyPreRefresh []querycheck.QueryResultCheck
-
-	// PostApplyPostRefresh runs all query checks in the slice. This occurs after the apply and refresh of a Config test are run.
-	// All errors by query checks in this slice are aggregated, reported, and will result in a test failure.
-	PostApplyPostRefresh []querycheck.QueryResultCheck
-}
-
 // ImportPlanChecks defines the different points in an Import TestStep when plan checks can be run.
 type ImportPlanChecks struct {
 	// PreApply runs all plan checks in the slice. This occurs after the plan of an Import test is computed. This slice cannot be populated

helper/resource/testing_new.go

@@ -388,7 +388,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest
 				t.Fatalf("Step %d/%d error running query: %s", stepNumber, len(c.Steps), err)
 			}
 
-			err = query.RunQueryChecks(ctx, t, &queryOut, step.ConfigQueryResultChecks)
+			err = query.RunQueryChecks(ctx, t, queryOut, step.QueryResultChecks)
 			if err != nil {
 				t.Fatalf("Step %d/%d error running query checks: %s", stepNumber, len(c.Steps), err)
 			}

internal/plugintest/working_dir.go

@@ -543,7 +543,7 @@ func (wd *WorkingDir) Query(ctx context.Context) ([]tfjson.LogMsg, error) {
 		}
 
 		if msg.Err != nil {
-			return nil, fmt.Errorf("retrieving next message: %w", msg.Err)
+			return nil, fmt.Errorf("retrieving message: %w", msg.Err)
 		}
 
 		if msg.Msg.Level() == tfjson.Error {

querycheck/contains.go

@@ -17,7 +17,7 @@ type contains struct {
 }
 
 func (c contains) CheckQuery(_ context.Context, req CheckQueryRequest, resp *CheckQueryResponse) {
-	for _, res := range *req.Query {
+	for _, res := range req.Query {
 		if strings.EqualFold(c.check, res.DisplayName) {
 			return
 		}
@@ -27,10 +27,10 @@ func (c contains) CheckQuery(_ context.Context, req CheckQueryRequest, resp *Che
 
 }
 
-// Contains returns a query check that asserts that a resource with a given display name exists within the returned results of the query.
+// ContainsResourceWithName returns a query check that asserts that a resource with a given display name exists within the returned results of the query.
 //
 // This query check can only be used with managed resources that support query. Query is only supported in Terraform v1.14+
-func Contains(resourceAddress string, displayName string) QueryResultCheck {
+func ContainsResourceWithName(resourceAddress string, displayName string) QueryResultCheck {
 	return contains{
 		resourceAddress: resourceAddress,
 		check:           displayName,

querycheck/expect_identity.go

@@ -23,7 +23,7 @@ type expectIdentity struct {
 
 // CheckQuery implements the query check logic.
 func (e expectIdentity) CheckQuery(_ context.Context, req CheckQueryRequest, resp *CheckQueryResponse) {
-	for _, res := range *req.Query {
+	for _, res := range req.Query {
 		var errCollection []error
 
 		if e.listResourceAddress != strings.TrimPrefix(res.Address, "list.") {

querycheck/expect_known_value.go

@@ -22,7 +22,7 @@ type expectKnownValue struct {
 }
 
 func (e expectKnownValue) CheckQuery(_ context.Context, req CheckQueryRequest, resp *CheckQueryResponse) {
-	for _, res := range *req.Query {
+	for _, res := range req.Query {
 		diags := make([]error, 0)
 
 		if e.listResourceAddress == strings.TrimPrefix(res.Address, "list.") && e.resourceName == res.DisplayName {

querycheck/expect_result_length_atleast.go

@@ -17,13 +17,13 @@ type expectLengthAtLeast struct {
 
 // CheckQuery implements the query check logic.
 func (e expectLengthAtLeast) CheckQuery(_ context.Context, req CheckQueryRequest, resp *CheckQueryResponse) {
-	if req.CompletedQuery == nil {
+	if req.QuerySummary == nil {
 		resp.Error = fmt.Errorf("no completed query information available")
 		return
 	}
 
-	if req.CompletedQuery.Total < e.check {
-		resp.Error = fmt.Errorf("Query result of at least length %v - expected but got %v.", e.check, req.CompletedQuery.Total)
+	if req.QuerySummary.Total < e.check {
+		resp.Error = fmt.Errorf("Query result of at least length %v - expected but got %v.", e.check, req.QuerySummary.Total)
 		return
 	}
 }

querycheck/expect_result_length_exact.go

@@ -5,39 +5,33 @@ package querycheck
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
-	"strconv"
-
-	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
 )
 
 var _ QueryResultCheck = expectLength{}
 
 type expectLength struct {
 	resourceAddress string
-	check           knownvalue.Check
+	check           int
 }
 
 // CheckQuery implements the query check logic.
 func (e expectLength) CheckQuery(_ context.Context, req CheckQueryRequest, resp *CheckQueryResponse) {
-	if req.CompletedQuery == nil {
-		resp.Error = fmt.Errorf("no completed query information available")
+	if req.QuerySummary == nil {
+		resp.Error = fmt.Errorf("no query summary information available")
 		return
 	}
 
-	lengthCheck := json.Number(strconv.Itoa(req.CompletedQuery.Total))
-
-	if err := e.check.CheckValue(lengthCheck); err != nil {
-		resp.Error = fmt.Errorf("Query result of length %v - expected but got %v.", e.check, req.CompletedQuery.Total)
+	if e.check != req.QuerySummary.Total {
+		resp.Error = fmt.Errorf("number of found resources %v - expected but got %v.", e.check, req.QuerySummary.Total)
 		return
 	}
 }
 
 // ExpectLength returns a query check that asserts that the length of the query result is exactly the given value.
 //
 // This query check can only be used with managed resources that support query. Query is only supported in Terraform v1.14+
-func ExpectLength(resourceAddress string, length knownvalue.Check) QueryResultCheck {
+func ExpectLength(resourceAddress string, length int) QueryResultCheck {
 	return expectLength{
 		resourceAddress: resourceAddress,
 		check:           length,