Skip to content

Commit

Permalink
fix: Fix small issues (#2287)
Browse files Browse the repository at this point in the history
* Fix resource monitor dereference

* Handle query change for dynamic table

* Fix after review
  • Loading branch information
sfc-gh-asawicki committed Dec 20, 2023
1 parent b19fe8b commit 4e2613c
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/resources/dynamic_table.go
Expand Up @@ -65,6 +65,7 @@ var dynamicTableShema = map[string]*schema.Schema{
"query": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Description: "Specifies the query to use to populate the dynamic table.",
DiffSuppressFunc: DiffSuppressStatement,
},
Expand Down
48 changes: 48 additions & 0 deletions pkg/resources/dynamic_table_acceptance_test.go
Expand Up @@ -236,6 +236,54 @@ func TestAcc_DynamicTable_issue2134(t *testing.T) {
})
}

// TestAcc_DynamicTable_issue2276 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2276 issue.
func TestAcc_DynamicTable_issue2276(t *testing.T) {
dynamicTableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
tableName := dynamicTableName + "_table"
query := fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)
newQuery := fmt.Sprintf(`select "data" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)
m := func() map[string]config.Variable {
return map[string]config.Variable{
"name": config.StringVariable(dynamicTableName),
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"warehouse": config.StringVariable(acc.TestWarehouseName),
"query": config.StringVariable(query),
"comment": config.StringVariable("Terraform acceptance test for GH issue 2276"),
"table_name": config.StringVariable(tableName),
}
}
m2 := m()
m2["query"] = config.StringVariable(newQuery)

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: testAccCheckDynamicTableDestroy,
Steps: []resource.TestStep{
{
ConfigDirectory: config.TestStepDirectory(),
ConfigVariables: m(),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "name", dynamicTableName),
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "query", query),
),
},
{
ConfigDirectory: acc.ConfigurationSameAsStepN(1),
ConfigVariables: m2,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "name", dynamicTableName),
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "query", newQuery),
),
},
},
})
}

func testAccCheckDynamicTableDestroy(s *terraform.State) error {
db := acc.TestAccProvider.Meta().(*sql.DB)
client := sdk.NewClientFromDB(db)
Expand Down
4 changes: 3 additions & 1 deletion pkg/resources/resource_monitor.go
Expand Up @@ -327,7 +327,9 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error {
for _, name := range userNames {
users = append(users, sdk.NotifiedUser{Name: name})
}
opts.NotifyUsers.Users = users
opts.NotifyUsers = &sdk.NotifyUsers{
Users: users,
}
}

if d.HasChange("credit_quota") {
Expand Down
31 changes: 31 additions & 0 deletions pkg/resources/resource_monitor_acceptance_test.go
Expand Up @@ -4,12 +4,14 @@ import (
"encoding/json"
"fmt"
"os"
"regexp"
"strings"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/stretchr/testify/require"
)

func TestAcc_ResourceMonitor(t *testing.T) {
Expand Down Expand Up @@ -216,6 +218,35 @@ resource "snowflake_resource_monitor" "test" {
`, accName)
}

// TestAcc_ResourceMonitor_issue2167 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2167 issue.
// Second step is purposely error, because tests TestAcc_ResourceMonitorUpdateNotifyUsers and TestAcc_ResourceMonitorNotifyUsers are still skipped.
// It can be fixed with them.
func TestAcc_ResourceMonitor_issue2167(t *testing.T) {
name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
configNoUsers, err := resourceMonitorNotifyUsersConfig(name, []string{})
require.NoError(t, err)
config, err := resourceMonitorNotifyUsersConfig(name, []string{"does not matter"})
require.NoError(t, err)

resource.Test(t, resource.TestCase{
Providers: acc.TestAccProviders(),
PreCheck: func() { acc.TestAccPreCheck(t) },
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Config: configNoUsers,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name),
),
},
{
Config: config,
ExpectError: regexp.MustCompile(`.*exactly one of AlterResourceMonitorOptions fields \[Set NotifyUsers Triggers] must be set.*`),
},
},
})
}

func TestAcc_ResourceMonitorNotifyUsers(t *testing.T) {
userEnv := os.Getenv("RESOURCE_MONITOR_NOTIFY_USERS_TEST")
if userEnv == "" {
Expand Down
27 changes: 27 additions & 0 deletions pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/test.tf
@@ -0,0 +1,27 @@
resource "snowflake_table" "t" {
database = var.database
schema = var.schema
name = var.table_name
change_tracking = true
column {
name = "id"
type = "NUMBER(38,0)"
}
column {
name = "data"
type = "VARCHAR(16)"
}
}

resource "snowflake_dynamic_table" "dt" {
depends_on = [snowflake_table.t]
name = var.name
database = var.database
schema = var.schema
target_lag {
maximum_duration = "2 minutes"
}
warehouse = var.warehouse
query = var.query
comment = var.comment
}
@@ -0,0 +1,27 @@
variable "name" {
type = string
}

variable "database" {
type = string
}

variable "schema" {
type = string
}

variable "warehouse" {
type = string
}

variable "query" {
type = string
}

variable "comment" {
type = string
}

variable "table_name" {
type = string
}

0 comments on commit 4e2613c

Please sign in to comment.