Skip to content

Commit

Permalink
Fix after failing tests
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jcieslak committed Jun 14, 2024
1 parent 91d0d50 commit 71db0fa
Show file tree
Hide file tree
Showing 16 changed files with 68 additions and 102 deletions.
10 changes: 0 additions & 10 deletions pkg/acceptance/helpers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"os"
"regexp"
"strings"
"testing"
Expand Down Expand Up @@ -82,12 +81,3 @@ func AssertErrorContainsPartsFunc(t *testing.T, parts []string) resource.ErrorCh
return nil
}
}

// TfAccFunc wraps a function and executes it whenever TF_ACC environment variable is set.
// Its main purpose is to run setup code before the actual terraform acceptance test.
func TfAccFunc(t *testing.T, f func()) {
t.Helper()
if os.Getenv(resource.EnvTfAcc) != "" {
f()
}
}
22 changes: 0 additions & 22 deletions pkg/acceptance/helpers/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package helpers
import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -58,23 +56,3 @@ func TestMatchAllStringsInOrderNonOverlapping(t *testing.T) {
})
}
}

func TestTfAccFunc(t *testing.T) {
t.Run("TF_ACC enabled", func(t *testing.T) {
t.Setenv("TF_ACC", "true")
value := new(bool)
TfAccFunc(t, func() {
*value = true
})
assert.True(t, *value)
})

t.Run("TF_ACC disabled", func(t *testing.T) {
t.Setenv("TF_ACC", "")
value := new(bool)
TfAccFunc(t, func() {
*value = true
})
assert.False(t, *value)
})
}
2 changes: 2 additions & 0 deletions pkg/acceptance/testenvs/testing_environment_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package testenvs

import (
"fmt"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"

Check failure on line 5 in pkg/acceptance/testenvs/testing_environment_variables.go

View workflow job for this annotation

GitHub Actions / reviewdog

[golangci] reported by reviewdog 🐶 File is not `gofumpt`-ed (gofumpt) Raw Output: pkg/acceptance/testenvs/testing_environment_variables.go:5: File is not `gofumpt`-ed (gofumpt) "github.com/hashicorp/terraform-plugin-testing/helper/resource"
"os"
"testing"
)
Expand All @@ -27,6 +28,7 @@ const (
SkipManagedAccountTest env = "TEST_SF_TF_SKIP_MANAGED_ACCOUNT_TEST"
SkipSamlIntegrationTest env = "TEST_SF_TF_SKIP_SAML_INTEGRATION_TEST"

EnableAcceptance env = resource.EnvTfAcc
EnableSweep env = "TEST_SF_TF_ENABLE_SWEEP"
ConfigureClientOnce env = "SF_TF_ACC_TEST_CONFIGURE_CLIENT_ONCE"
TestObjectsSuffix env = "TEST_SF_TF_TEST_OBJECT_SUFFIX"
Expand Down
6 changes: 4 additions & 2 deletions pkg/datasources/databases_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func TestAcc_Databases_Complete(t *testing.T) {
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.kind", "STANDARD"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_transient", "false"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_default", "false"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_current", "true"),
// Commenting as this value depends on the currently used database, which is different when running as a single test and multiple tests (e.g., on CI)
// resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_current", "true"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.origin", ""),
resource.TestCheckResourceAttrSet("data.snowflake_databases.test", "databases.0.owner"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.comment", comment),
Expand Down Expand Up @@ -75,7 +76,8 @@ func TestAcc_Databases_Complete(t *testing.T) {
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.kind", "STANDARD"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_transient", "false"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_default", "false"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_current", "false"),
// Commenting for the same reason as above
// resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.is_current", "false"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.origin", ""),
resource.TestCheckResourceAttrSet("data.snowflake_databases.test", "databases.0.owner"),
resource.TestCheckResourceAttr("data.snowflake_databases.test", "databases.0.comment", comment),
Expand Down
3 changes: 2 additions & 1 deletion pkg/resources/custom_diffs.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ func ParameterValueComputedIf(key string, parameters []*sdk.Parameter, objectPar
// For cases where currently set value (in the config) is equal to the parameter, but not set on the right level.
// The parameter is set somewhere higher in the hierarchy, and we need to "forcefully" set the value to
// perform the actual set on Snowflake (and set the parameter on the correct level).
if ok && !configValue.IsNull() && parameter.Level != objectParameterLevel {
// TODO:
if ok && !configValue.IsNull() && parameter.Level != objectParameterLevel && parameter.Value == valueToString(d.Get(key)) {
return d.SetNewComputed(key)
}

Expand Down
27 changes: 11 additions & 16 deletions pkg/resources/database_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resources_test

import (
"fmt"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
"strconv"
"testing"

Expand Down Expand Up @@ -276,7 +277,6 @@ func TestAcc_Database_ComputedValues(t *testing.T) {
resource.TestCheckResourceAttr("snowflake_database.test", "external_volume", externalVolumeId.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "catalog", catalogId.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "replace_invalid_characters", "true"),
resource.TestCheckResourceAttr("snowflake_database.test", "default_ddl_collation", "en_US"),
resource.TestCheckResourceAttr("snowflake_database.test", "storage_serialization_policy", string(sdk.StorageSerializationPolicyCompatible)),
resource.TestCheckResourceAttr("snowflake_database.test", "log_level", string(sdk.LogLevelInfo)),
resource.TestCheckResourceAttr("snowflake_database.test", "trace_level", string(sdk.TraceLevelOnEvent)),
Expand Down Expand Up @@ -1105,13 +1105,12 @@ resource "snowflake_database" "test" {
}

func TestAcc_Database_UpgradeFromShare(t *testing.T) {
_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)

id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
secondaryClientLocator := acc.SecondaryClient(t).GetAccountLocator()

shareExternalId := sdk.Pointer(sdk.NewExternalObjectIdentifierFromFullyQualifiedName(""))
helpers.TfAccFunc(t, func() {
*shareExternalId = createShareableDatabase(t)
})
shareExternalId := createShareableDatabase(t)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
Expand Down Expand Up @@ -1153,7 +1152,7 @@ func TestAcc_Database_UpgradeFromShare(t *testing.T) {
})
}

func databaseStateUpgraderFromShareOld(id sdk.AccountObjectIdentifier, secondaryClientLocator string, externalShare *sdk.ExternalObjectIdentifier) string {
func databaseStateUpgraderFromShareOld(id sdk.AccountObjectIdentifier, secondaryClientLocator string, externalShare sdk.ExternalObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
Expand All @@ -1176,17 +1175,13 @@ resource "snowflake_database" "test" {
}

func TestAcc_Database_UpgradeFromReplica(t *testing.T) {
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)

// This couldn't be done in any other way (except creating this setup with terraform configuration), because primaryDatabaseId is part of the configuration which is "static".
primaryDatabaseId := sdk.Pointer(sdk.NewExternalObjectIdentifierFromFullyQualifiedName(""))
helpers.TfAccFunc(t, func() {
_, externalId, databaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{
acc.TestClient().Account.GetAccountIdentifier(t),
})
t.Cleanup(databaseCleanup)
*primaryDatabaseId = externalId
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
_, primaryDatabaseId, databaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{
acc.TestClient().Account.GetAccountIdentifier(t),
})
t.Cleanup(databaseCleanup)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
Expand All @@ -1202,7 +1197,7 @@ func TestAcc_Database_UpgradeFromReplica(t *testing.T) {
Source: "Snowflake-Labs/snowflake",
},
},
Config: databaseStateUpgraderFromReplicaOld(id, *primaryDatabaseId),
Config: databaseStateUpgraderFromReplicaOld(id, primaryDatabaseId),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_database.test", "id", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "name", id.Name()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1004,14 +1004,14 @@ func TestAcc_GrantPrivilegesToAccountRole_ImportedPrivileges(t *testing.T) {
sharedDatabaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
sharedDatabaseName := sharedDatabaseId.Name()
shareId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
shareName := shareId.Name()
roleName := acc.TestClient().Ids.Alpha()
secondaryAccountName := acc.SecondaryTestClient().Context.CurrentAccount(t)
configVariables := config.Variables{
"role_name": config.StringVariable(roleName),
"shared_database_name": config.StringVariable(sharedDatabaseName),
"share_name": config.StringVariable(shareName),
"account_name": config.StringVariable(secondaryAccountName),
"external_share_name": config.StringVariable(sdk.NewExternalObjectIdentifier(
acc.SecondaryTestClient().Account.GetAccountIdentifier(t),
shareId,
).FullyQualifiedName()),
"privileges": config.ListVariable(
config.StringVariable(sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
Expand Down
8 changes: 4 additions & 4 deletions pkg/resources/grant_privileges_to_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,14 +1081,14 @@ func TestAcc_GrantPrivilegesToRole_ImportedPrivileges(t *testing.T) {
sharedDatabaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
sharedDatabaseName := sharedDatabaseId.Name()
shareId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
shareName := shareId.Name()
roleName := acc.TestClient().Ids.Alpha()
secondaryAccountName := acc.SecondaryTestClient().Context.CurrentAccount(t)
configVariables := config.Variables{
"role_name": config.StringVariable(roleName),
"shared_database_name": config.StringVariable(sharedDatabaseName),
"share_name": config.StringVariable(shareName),
"account_name": config.StringVariable(secondaryAccountName),
"external_share_name": config.StringVariable(sdk.NewExternalObjectIdentifier(
acc.SecondaryTestClient().Account.GetAccountIdentifier(t),
shareId,
).FullyQualifiedName()),
"privileges": config.ListVariable(
config.StringVariable(sdk.AccountObjectPrivilegeImportedPrivileges.String()),
),
Expand Down
46 changes: 30 additions & 16 deletions pkg/resources/secondary_database_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,13 +326,13 @@ func TestAcc_CreateSecondaryDatabase_complete(t *testing.T) {
ConfigVariables: unsetConfigVariables,
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_SecondaryDatabase/complete-optionals-unset"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "name", newId.Name()),
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "name", id.Name()),
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "is_transient", "false"),
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "as_replica_of", externalPrimaryId.FullyQualifiedName()),
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "comment", newComment),
resource.TestCheckResourceAttr("snowflake_secondary_database.test", "comment", ""),

resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "data_retention_time_in_days.0.value", accountDataRetentionTimeInDays),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "max_data_extension_time_in_days.0.value", accountMaxDataExtensionTimeInDays),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "data_retention_time_in_days", accountDataRetentionTimeInDays),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "max_data_extension_time_in_days", accountMaxDataExtensionTimeInDays),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "external_volume", accountExternalVolume),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "catalog", accountCatalog),
resource.TestCheckResourceAttrPtr("snowflake_secondary_database.test", "replace_invalid_characters", accountReplaceInvalidCharacters),
Expand Down Expand Up @@ -399,27 +399,41 @@ func TestAcc_CreateSecondaryDatabase_DataRetentionTimeInDays(t *testing.T) {
accountDataRetentionTimeInDays, err := acc.Client(t).Parameters.ShowAccountParameter(context.Background(), sdk.AccountParameterDataRetentionTimeInDays)
require.NoError(t, err)

externalVolumeId, externalVolumeCleanup := acc.TestClient().ExternalVolume.Create(t)
t.Cleanup(externalVolumeCleanup)

catalogId, catalogCleanup := acc.TestClient().CatalogIntegration.Create(t)
t.Cleanup(catalogCleanup)

configVariables := func(
id sdk.AccountObjectIdentifier,
primaryDatabaseName sdk.ExternalObjectIdentifier,
dataRetentionTimeInDays *int,
) config.Variables {
variables := config.Variables{
"name": config.StringVariable(id.Name()),
"as_replica_of": config.StringVariable(primaryDatabaseName.FullyQualifiedName()),
"transient": config.BoolVariable(false),
"external_volume": config.StringVariable(""),
"catalog": config.StringVariable(""),
"replace_invalid_characters": config.StringVariable("false"),
"default_ddl_collation": config.StringVariable(""),
"storage_serialization_policy": config.StringVariable("OPTIMIZED"),
"log_level": config.StringVariable("OFF"),
"trace_level": config.StringVariable("OFF"),
"comment": config.StringVariable(""),
"name": config.StringVariable(id.Name()),
"as_replica_of": config.StringVariable(primaryDatabaseName.FullyQualifiedName()),
"transient": config.BoolVariable(false),
"comment": config.StringVariable(""),

"max_data_extension_time_in_days": config.IntegerVariable(10),
"external_volume": config.StringVariable(externalVolumeId.Name()),
"catalog": config.StringVariable(catalogId.Name()),
"replace_invalid_characters": config.BoolVariable(true),
"default_ddl_collation": config.StringVariable("en_US"),
"storage_serialization_policy": config.StringVariable("OPTIMIZED"),
"log_level": config.StringVariable("OFF"),
"trace_level": config.StringVariable("OFF"),
"suspend_task_after_num_failures": config.IntegerVariable(10),
"task_auto_retry_attempts": config.IntegerVariable(10),
"user_task_managed_initial_warehouse_size": config.StringVariable(string(sdk.WarehouseSizeSmall)),
"user_task_timeout_ms": config.IntegerVariable(120000),
"user_task_minimum_trigger_interval_in_seconds": config.IntegerVariable(120),
"quoted_identifiers_ignore_case": config.BoolVariable(true),
"enable_console_output": config.BoolVariable(true),
}
if dataRetentionTimeInDays != nil {
variables["data_retention_time_in_days"] = config.IntegerVariable(*dataRetentionTimeInDays)
variables["max_data_extension_time_in_days"] = config.IntegerVariable(10)
}
return variables
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
resource "snowflake_database" "db" {
resource "snowflake_database_old" "db" {
name = var.db
comment = "test comment"
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
resource "snowflake_database" "test" {
resource "snowflake_database_old" "test" {
name = var.database
data_retention_time_in_days = var.database_data_retention_time
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
resource "snowflake_database" "test" {
resource "snowflake_database_old" "test" {
name = var.database
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
resource "snowflake_database" "test" {
resource "snowflake_shared_database" "test" {
name = var.shared_database_name
data_retention_time_in_days = 0
from_share = {
provider = var.account_name
share = var.share_name
}
from_share = var.external_share_name
}

resource "snowflake_role" "test" {
name = var.role_name
}

resource "snowflake_grant_privileges_to_account_role" "test" {
depends_on = [snowflake_database.test, snowflake_role.test]
depends_on = [snowflake_shared_database.test, snowflake_role.test]
account_role_name = "\"${var.role_name}\""
privileges = var.privileges
on_account_object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ variable "shared_database_name" {
type = string
}

variable "share_name" {
type = string
}

variable "account_name" {
variable "external_share_name" {
type = string
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
resource "snowflake_database" "test" {
resource "snowflake_shared_database" "test" {
name = var.shared_database_name
data_retention_time_in_days = 0
from_share = {
provider = var.account_name
share = var.share_name
}
from_share = var.external_share_name
}

resource "snowflake_role" "test" {
name = var.role_name
}

resource "snowflake_grant_privileges_to_role" "test" {
depends_on = [snowflake_database.test, snowflake_role.test]
depends_on = [snowflake_shared_database.test, snowflake_role.test]
role_name = "\"${var.role_name}\""
privileges = var.privileges
on_account_object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ variable "shared_database_name" {
type = string
}

variable "share_name" {
type = string
}

variable "account_name" {
variable "external_share_name" {
type = string
}

0 comments on commit 71db0fa

Please sign in to comment.