User Tests: Use OpenAPI client to check if they exist (#1136)

Gets rid of the manually written checkers Also, added two common code functions to shorten each helper. We can now define a "checker" in 7 lines. This is definitely an improvement from the 30-50 lines of manually written code :)
grafana · Nov 8, 2023 · 872533d · 872533d
1 parent ebde9f8
commit 872533d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 62 deletions.
diff --git a/internal/resources/grafana/common_check_exists_test.go b/internal/resources/grafana/common_check_exists_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/grafana/grafana-openapi-client-go/client/annotations"
 	"github.com/grafana/grafana-openapi-client-go/client/folders"
 	"github.com/grafana/grafana-openapi-client-go/client/teams"
+	"github.com/grafana/grafana-openapi-client-go/client/users"
 	"github.com/grafana/grafana-openapi-client-go/models"
 	"github.com/grafana/terraform-provider-grafana/internal/common"
 	"github.com/grafana/terraform-provider-grafana/internal/resources/grafana"
@@ -25,33 +26,31 @@ var (
 		func(client *goapi.GrafanaHTTPAPI, id string) (*models.Annotation, error) {
 			params := annotations.NewGetAnnotationByIDParams().WithAnnotationID(id)
 			resp, err := client.Annotations.GetAnnotationByID(params, nil)
-			if err != nil {
-				return nil, err
-			}
-			return resp.GetPayload(), nil
+			return payloadOrError(resp, err)
 		},
 	)
 	folderCheckExists = newCheckExistsHelper(
 		func(f *models.Folder) string { return strconv.FormatInt(f.ID, 10) },
 		func(client *goapi.GrafanaHTTPAPI, id string) (*models.Folder, error) {
-			idInt, _ := strconv.ParseInt(id, 10, 64)
-			params := folders.NewGetFolderByIDParams().WithFolderID(idInt)
-			folder, err := client.Folders.GetFolderByID(params, nil)
-			if err != nil {
-				return nil, err
-			}
-			return folder.GetPayload(), nil
+			params := folders.NewGetFolderByIDParams().WithFolderID(mustParseInt64(id))
+			resp, err := client.Folders.GetFolderByID(params, nil)
+			return payloadOrError(resp, err)
 		},
 	)
 	teamCheckExists = newCheckExistsHelper(
 		func(t *models.TeamDTO) string { return strconv.FormatInt(t.ID, 10) },
 		func(client *goapi.GrafanaHTTPAPI, id string) (*models.TeamDTO, error) {
 			params := teams.NewGetTeamByIDParams().WithTeamID(id)
-			team, err := client.Teams.GetTeamByID(params, nil)
-			if err != nil {
-				return nil, err
-			}
-			return team.GetPayload(), nil
+			resp, err := client.Teams.GetTeamByID(params, nil)
+			return payloadOrError(resp, err)
+		},
+	)
+	userCheckExists = newCheckExistsHelper(
+		func(u *models.UserProfileDTO) string { return strconv.FormatInt(u.ID, 10) },
+		func(client *goapi.GrafanaHTTPAPI, id string) (*models.UserProfileDTO, error) {
+			params := users.NewGetUserByIDParams().WithUserID(mustParseInt64(id))
+			resp, err := client.Users.GetUserByID(params, nil)
+			return payloadOrError(resp, err)
 		},
 	)
 )
@@ -64,6 +63,9 @@ type checkExistsHelper[T interface{}] struct {
 	getResourceFunc checkExistsGetResourceFunc[T]
 }
 
+// newCheckExistsHelper creates a test helper that checks if a resource exists or not.
+// The getIDFunc function should return the ID of the resource.
+// The getResourceFunc function should return the resource from the given ID.
 func newCheckExistsHelper[T interface{}](getIDFunc checkExistsGetIDFunc[T], getResourceFunc checkExistsGetResourceFunc[T]) checkExistsHelper[T] {
 	return checkExistsHelper[T]{getIDFunc: getIDFunc, getResourceFunc: getResourceFunc}
 }
@@ -124,3 +126,19 @@ func (h *checkExistsHelper[T]) destroyed(v *T, org *gapi.Org) resource.TestCheck
 		return nil
 	}
 }
+
+func mustParseInt64(s string) int64 {
+	i, err := strconv.ParseInt(s, 10, 64)
+	if err != nil {
+		panic(err)
+	}
+	return i
+}
+
+// payloadOrError returns the error if not nil, or the payload otherwise. This saves 4 lines of code on each helper.
+func payloadOrError[T interface{}, R interface{ GetPayload() *T }](resp R, err error) (*T, error) {
+	if err != nil {
+		return nil, err
+	}
+	return resp.GetPayload(), nil
+}
diff --git a/internal/resources/grafana/data_source_user_test.go b/internal/resources/grafana/data_source_user_test.go
@@ -3,7 +3,7 @@ package grafana_test
 import (
 	"testing"
 
-	gapi "github.com/grafana/grafana-api-golang-client"
+	"github.com/grafana/grafana-openapi-client-go/models"
 	"github.com/grafana/terraform-provider-grafana/internal/common"
 	"github.com/grafana/terraform-provider-grafana/internal/testutils"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -12,9 +12,9 @@ import (
 func TestAccDatasourceUser_basic(t *testing.T) {
 	testutils.CheckOSSTestsEnabled(t)
 
-	var user gapi.User
+	var user models.UserProfileDTO
 	checks := []resource.TestCheckFunc{
-		testAccUserCheckExists("grafana_user.test", &user),
+		userCheckExists.exists("grafana_user.test", &user),
 	}
 	for _, rName := range []string{"from_email", "from_login", "from_id"} {
 		checks = append(checks,
@@ -38,7 +38,7 @@ func TestAccDatasourceUser_basic(t *testing.T) {
 
 	resource.ParallelTest(t, resource.TestCase{
 		ProviderFactories: testutils.ProviderFactories,
-		CheckDestroy:      testAccUserCheckDestroy(&user),
+		CheckDestroy:      userCheckExists.destroyed(&user, nil),
 		Steps: []resource.TestStep{
 			{
 				Config: testutils.TestAccExample(t, "data-sources/grafana_user/data-source.tf"),

diff --git a/internal/resources/grafana/resource_user_test.go b/internal/resources/grafana/resource_user_test.go
@@ -1,30 +1,27 @@
 package grafana_test
 
 import (
-	"fmt"
-	"strconv"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 
-	gapi "github.com/grafana/grafana-api-golang-client"
+	"github.com/grafana/grafana-openapi-client-go/models"
 	"github.com/grafana/terraform-provider-grafana/internal/common"
 	"github.com/grafana/terraform-provider-grafana/internal/testutils"
 )
 
 func TestAccUser_basic(t *testing.T) {
 	testutils.CheckOSSTestsEnabled(t)
 
-	var user gapi.User
+	var user models.UserProfileDTO
 	resource.ParallelTest(t, resource.TestCase{
 		ProviderFactories: testutils.ProviderFactories,
-		CheckDestroy:      testAccUserCheckDestroy(&user),
+		CheckDestroy:      userCheckExists.destroyed(&user, nil),
 		Steps: []resource.TestStep{
 			{
 				Config: testAccUserConfig_basic,
 				Check: resource.ComposeTestCheckFunc(
-					testAccUserCheckExists("grafana_user.test", &user),
+					userCheckExists.exists("grafana_user.test", &user),
 					resource.TestCheckResourceAttr(
 						"grafana_user.test", "email", "terraform-test@localhost",
 					),
@@ -45,7 +42,7 @@ func TestAccUser_basic(t *testing.T) {
 			{
 				Config: testAccUserConfig_update,
 				Check: resource.ComposeTestCheckFunc(
-					testAccUserCheckExists("grafana_user.test", &user),
+					userCheckExists.exists("grafana_user.test", &user),
 					resource.TestCheckResourceAttr(
 						"grafana_user.test", "email", "terraform-test-update@localhost",
 					),
@@ -73,40 +70,6 @@ func TestAccUser_basic(t *testing.T) {
 	})
 }
 
-func testAccUserCheckExists(rn string, a *gapi.User) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		rs, ok := s.RootModule().Resources[rn]
-		if !ok {
-			return fmt.Errorf("resource not found: %s", rn)
-		}
-		if rs.Primary.ID == "" {
-			return fmt.Errorf("resource id not set")
-		}
-		id, err := strconv.ParseInt(rs.Primary.ID, 10, 64)
-		if err != nil {
-			return fmt.Errorf("resource id is malformed")
-		}
-		client := testutils.Provider.Meta().(*common.Client).GrafanaAPI
-		user, err := client.User(id)
-		if err != nil {
-			return fmt.Errorf("error getting data source: %s", err)
-		}
-		*a = user
-		return nil
-	}
-}
-
-func testAccUserCheckDestroy(a *gapi.User) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		client := testutils.Provider.Meta().(*common.Client).GrafanaAPI
-		user, err := client.User(a.ID)
-		if err == nil && user.Email != "" {
-			return fmt.Errorf("user still exists")
-		}
-		return nil
-	}
-}
-
 const testAccUserConfig_basic = `
 resource "grafana_user" "test" {
   email    = "terraform-test@localhost"