fix: Removes SchemaConfigModeAttr from resources #1961
Conversation
maastha
force-pushed
the
CLOUDP-230250-remove-configmode-cluster
branch
from
5982592 to
c421df1
Compare
| @@ -2,130 +2,141 @@ package cluster_test | |||
|
|
|||
There was a problem hiding this comment.
Previously the file called "resource_cluster_migration_test.go" had tests related to StateUpgrader. That file is now renamed to "resource_cluster_schema_migration_test.go" in this PR.
"resource_cluster_migration_test.go" now contains general migration tests for cluster resource.
| ), | ||
| }, | ||
| mig.TestStepCheckEmptyPlan(config), | ||
| { |
There was a problem hiding this comment.
not against but it looks like adding steps after mig one is like mixing mig and acc test together
There was a problem hiding this comment.
The intention is here is to not only ensure that plans are empty but also that updates and everything work as expected.
| ) | ||
|
|
||
| func TestAccClusterRSClusterMigrateState_empty_advancedConfig(t *testing.T) { | ||
| v0State := map[string]any{ |
There was a problem hiding this comment.
I would add to tests in this file: acc.SkipInUnitTest(t) , so they don't run in unit test, as we're not using resource.ParallelTest ... it won't have TF_ACC variable into account and will always run the tests. (it's true that these tests don't take a lot of time but still better run only when acc)
| "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/cluster" | ||
| ) | ||
|
|
||
| func TestAccClusterRSClusterMigrateState_empty_advancedConfig(t *testing.T) { |
There was a problem hiding this comment.
i think we're using "migration" here in a different way as in the typical migration tests. in fact for instance these tests execute in acc not in mig GH actions because the test names
not a strong opinion but don't know if calling the files "migration" is confusing, maybe i would put these tests together with the regular test file, or maybe just call the file resource_cluster_schema_test
There was a problem hiding this comment.
I think we can rename these to resource_cluster_state_upgrader_test to be more specific, does that address your concerns?
There was a problem hiding this comment.
updated to resource_cluster_state_upgrader_test let me know in case of concerns
There was a problem hiding this comment.
removing "migrate/migration" from the filename works for me, thanks!
internal/service/advancedcluster/resource_advanced_cluster_migration_test.go
Outdated
Show resolved
Hide resolved
maastha
changed the title
Co-authored-by: Agustin Bettati <[email protected]>
| @@ -1537,44 +1537,6 @@ func TestAccClusterRSCluster_basicAWS_PausedToUnpaused(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
| func TestAccClusterRSCluster_withDefaultBiConnectorAndAdvancedConfiguration_maintainsBackwardCompatibility(t *testing.T) { | |||
There was a problem hiding this comment.
moved to _migration_test.go
Description
In mongodbatlas provider v1.12.3 'schema.SchemaConfigModeAttr' was added to collection attributes to support Terraform versions <1.1.5. This setting is not recommended by Hashicorp and breaks CDKTF.
Effectively, this PR reverts changes made in below PR:
#1572
Breaking change: Removing this setting causes cluster and advanced_cluster resources to be incompatible with Terraform versions <1.1.5.
Note: As of 2023-12-31, the minimum supported version of Terraform by us is 1.2.x.
Link to any related issue(s): https://jira.mongodb.org/browse/CLOUDP-230250
Type of change:
Required Checklist:
Further comments