From 872533dd87c3732bcde06708075f1f538c02e468 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Wed, 8 Nov 2023 08:00:05 -0500 Subject: [PATCH] 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/common_check_exists_test.go | 50 +++++++++++++------ .../grafana/data_source_user_test.go | 8 +-- .../resources/grafana/resource_user_test.go | 47 ++--------------- 3 files changed, 43 insertions(+), 62 deletions(-) diff --git a/internal/resources/grafana/common_check_exists_test.go b/internal/resources/grafana/common_check_exists_test.go index 9c80b95e3..85057c65b 100644 --- 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 index 955a5f364..471b45700 100644 --- 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 index 6d883347c..2e0f357f3 100644 --- a/internal/resources/grafana/resource_user_test.go +++ b/internal/resources/grafana/resource_user_test.go @@ -1,14 +1,11 @@ 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" ) @@ -16,15 +13,15 @@ import ( 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"