Skip to content

test: Updates importIgnoredFields function to handle dynamic replication specs #2977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 20, 2025
Merged

test: Updates importIgnoredFields function to handle dynamic replication specs #2977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24b0e97
test: Updates importIgnoredFields function to handle dynamic replicat…
EspenAlbert Jan 17, 2025
a3c5dc5
improve naming, reverse condition
lantoli Jan 17, 2025
2426d2e
ignore all changing attributes
lantoli Jan 17, 2025
08b40ee
clarify tests
lantoli Jan 19, 2025
781aa1a
refactor acc.TestStepImportCluster
lantoli Jan 19, 2025
29fe715
revert unrelated changes
lantoli Jan 19, 2025
f64edbb
ignore all replication_specs
lantoli Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions internal/service/advancedcluster/resource_advanced_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func replicaSetMultiCloudTestCase(t *testing.T, isAcc bool) resource.TestCase {
Config: configReplicaSetMultiCloud(t, isAcc, orgID, projectName, clusterNameUpdated),
Check: checkReplicaSetMultiCloud(isAcc, clusterNameUpdated, 3),
},
acc.TestStepImportCluster(resourceName, "replication_specs", "retain_backups_enabled"),
acc.TestStepImportCluster(resourceName),
},
}
}
Expand Down Expand Up @@ -253,7 +253,7 @@ func singleShardedMultiCloudTestCase(t *testing.T, isAcc bool) resource.TestCase
Config: configShardedOldSchemaMultiCloud(t, isAcc, projectID, clusterNameUpdated, 1, "M10", nil),
Check: checkShardedOldSchemaMultiCloud(isAcc, clusterNameUpdated, 1, "M10", true, nil),
},
acc.TestStepImportCluster(resourceName, "replication_specs"),
acc.TestStepImportCluster(resourceName),
},
}
}
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestAccClusterAdvancedCluster_unpausedToPaused(t *testing.T) {
Config: configSingleProviderPaused(t, true, projectID, clusterName, true, anotherInstanceSize),
ExpectError: regexp.MustCompile("CANNOT_UPDATE_PAUSED_CLUSTER"),
},
acc.TestStepImportCluster(resourceName, "replication_specs"),
acc.TestStepImportCluster(resourceName),
},
})
}
Expand Down Expand Up @@ -313,7 +313,7 @@ func TestAccClusterAdvancedCluster_pausedToUnpaused(t *testing.T) {
{
Config: configSingleProviderPaused(t, true, projectID, clusterName, false, instanceSize),
},
acc.TestStepImportCluster(resourceName, "replication_specs"),
acc.TestStepImportCluster(resourceName),
},
})
}
Expand Down Expand Up @@ -1190,7 +1190,7 @@ func TestAccMockableAdvancedCluster_replicasetAdvConfigUpdate(t *testing.T) {
Config: configBasicReplicaset(t, projectID, clusterName, fullUpdate),
Check: checksUpdate,
},
acc.TestStepImportCluster(resourceName, importIgnoredFields()...),
acc.TestStepImportCluster(resourceName),
},
})
}
Expand Down Expand Up @@ -1233,7 +1233,7 @@ func TestAccMockableAdvancedCluster_shardedAddAnalyticsAndAutoScaling(t *testing
Config: configSharded(t, projectID, clusterName, true),
Check: checksUpdated,
},
acc.TestStepImportCluster(resourceName, importIgnoredFields()...),
acc.TestStepImportCluster(resourceName),
},
})
}
Expand Down Expand Up @@ -2781,14 +2781,3 @@ func configFCVPinning(t *testing.T, orgID, projectName, clusterName string, pinn

`, orgID, projectName, clusterName, mongoDBMajorVersion, pinnedFCVAttr)) + dataSourcesTFNewSchema
}

func importIgnoredFields() []string {
if config.AdvancedClusterV2Schema() {
return []string{}
}
return []string{
"replication_specs.0.region_configs.0.read_only_specs",
"replication_specs.0.region_configs.0.analytics_specs",
"replication_specs.0.region_configs.0.electable_specs.0.ebs_volume_type",
}
}
15 changes: 13 additions & 2 deletions internal/testutil/acc/advanced_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/config"
"go.mongodb.org/atlas-sdk/v20241113004/admin"
)

Expand Down Expand Up @@ -42,14 +43,24 @@ var (
}
)

func TestStepImportCluster(resourceName string, ignoredFields ...string) resource.TestStep {
func TestStepImportCluster(resourceName string, ignorePrefixFields ...string) resource.TestStep {
ignorePrefixFields = append(ignorePrefixFields,
"retain_backups_enabled", // This field is TF specific and not returned by Atlas, so Import can't fill it in.
)

// auto_scaling & specs (electable_specs, read_only_specs, etc.) are only set in state in SDKv2 if present in the definition.
// However, as import doesn't have a previous state to compare with, import will always fill them.
// This will make these fields differ in the state, although the plan change won't be shown to the user as they're computed values.
if !config.AdvancedClusterV2Schema() {
ignorePrefixFields = append(ignorePrefixFields, "replication_specs")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite broad. But I like that you could use the TestStepImportCluster instead of the extra helper

Copy link
Member

@lantoli lantoli Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EspenAlbert it is for SDKv2 tests, but reality is that any spec that is not in the definition file must be included, so at then it's like playing to exclude the elements that are not in the definition file.

however it's more restrict for TPF now, because some tests had "replication_specs" for SDKv2 and TPF, not this is only for SDKv2 tests.

}
return resource.TestStep{
ResourceName: resourceName,
ImportStateIdFunc: ImportStateIDFuncProjectIDClusterName(resourceName, "project_id", "name"),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIdentifierAttribute: "name",
ImportStateVerifyIgnore: ignoredFields,
ImportStateVerifyIgnore: ignorePrefixFields,
}
}

Expand Down
Loading