Skip to content

test: Updates all test workflows to use advanced_cluster TPF implementation #3580

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 11 commits into from
Aug 14, 2025
Merged

test: Updates all test workflows to use advanced_cluster TPF implementation #3580

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8dd6255
fix
maastha Aug 11, 2025
a9f8fb5
fix
maastha Aug 11, 2025
177581e
fix
maastha Aug 11, 2025
e9d1239
nit
maastha Aug 11, 2025
f419e76
nit
maastha Aug 11, 2025
dc53484
init
maastha Aug 12, 2025
95c3428
revert
maastha Aug 12, 2025
1062349
fmt
maastha Aug 12, 2025
1bf0f23
nit
maastha Aug 12, 2025
0bcb43e
nit
maastha Aug 14, 2025
4358b36
pr cmt
maastha Aug 14, 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
11 changes: 3 additions & 8 deletions .github/workflows/acceptance-tests-runner.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
type: string
required: false
test_group:
description: 'Test group to run, e.g. advanced_cluster, empty for all'
description: 'Test group to run, e.g. advanced_cluster_tpf, empty for all'
Copy link
Member

@lantoli lantoli Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what about renaming all occurences in this file from advanced_cluster_tpf to advanced_cluster as that's the only one now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought that would be done later as part of the cleanup when we finally delete advancedcluster? but no strong opinion, will update

Copy link
Member

Choose a reason for hiding this comment

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

ah, ok, the cleanup was more about the service folders but i see your point here, feel free to keep it as it is if you prefer that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated now

Copy link
Collaborator

Choose a reason for hiding this comment

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

curious about the context: why do we still need to specify the _tpf?

Copy link
Member

Choose a reason for hiding this comment

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

i made a similar comment, we can remove tpf now that it's the only test group

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

type: string
required: false
test_name:
Expand All @@ -26,7 +26,7 @@ on:
type: string
required: false
reduced_tests:
description: 'If run only minimum tests for advanced_cluster, e.g. in PRs'
description: 'If run only minimum tests for advanced_cluster_tpf, e.g. in PRs'
type: boolean
required: false
default: false
Expand Down Expand Up @@ -241,7 +241,6 @@ jobs:
env:
mustTrigger: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && inputs.test_group == '' ) }}
outputs: # ensure resources are sorted alphabetically
advanced_cluster: ${{ steps.filter.outputs.advanced_cluster == 'true' || env.mustTrigger == 'true' }}
advanced_cluster_tpf: ${{ steps.filter.outputs.advanced_cluster_tpf == 'true' || env.mustTrigger == 'true' }}
assume_role: ${{ steps.filter.outputs.assume_role == 'true' || env.mustTrigger == 'true' }}
autogen: ${{ steps.filter.outputs.autogen == 'true' || env.mustTrigger == 'true' }}
Expand Down Expand Up @@ -274,9 +273,6 @@ jobs:
if: ${{ inputs.test_group == '' && env.mustTrigger == 'false' }}
with:
filters: |
advanced_cluster:
- 'internal/service/advancedcluster/!(*_test).go' # matches any adv_cluster file change except test files
- 'internal/service/advancedclustertpf/common*.go'
advanced_cluster_tpf:
- 'internal/service/advancedclustertpf/*.go'
- 'internal/service/advancedcluster/*_test.go'
Copy link
Member

@lantoli lantoli Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

is there any test in adv_cluster (without tpf) or we can delete filter path 'internal/service/advancedcluster/*_test.go' ?

we're only running package ./internal/service/advancedclustertpf so I think it's ok to delete the filter path

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed 'internal/service/advancedcluster/*_test.go' ty!

Expand Down Expand Up @@ -408,7 +404,6 @@ jobs:
HTTP_MOCKER_CAPTURE: 'true'
ACCTEST_REGEX_RUN: ${{ inputs.reduced_tests && '^TestAccMockable' || env.ACCTEST_REGEX_RUN }}
ACCTEST_PACKAGES: |
./internal/service/advancedcluster
./internal/service/advancedclustertpf
run: make testacc

Expand Down Expand Up @@ -436,7 +431,7 @@ jobs:
MONGODB_ATLAS_LAST_1X_VERSION: ${{ inputs.mongodb_atlas_last_1x_version }}
ACCTEST_REGEX_RUN: '^TestMig'
ACCTEST_PACKAGES: |
./internal/service/advancedcluster
./internal/service/advancedclustertpf
Copy link
Member

Choose a reason for hiding this comment

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

do we still need advanced_cluster_tpf_mig_from_sdkv2 tests? Maybe we can remove it, and remove logic associated with env.var MONGODB_ATLAS_TEST_SDKV2_TO_TPF from the code.

Copy link
Collaborator Author

@maastha maastha Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I recommended in the team to keep these for at least a couple next releases until we have a better idea of adoption & may also help us understand if we need to handle any additional scenarios that might come up based on changes in the next releases

run: make testacc

assume_role:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/code-health.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ jobs:
secrets: inherit
uses: ./.github/workflows/acceptance-tests.yml
with:
reduced_tests: false
reduced_tests: true
6 changes: 0 additions & 6 deletions internal/config/preview_provider_v2.go
View file
Open in desktop

This file was deleted.

84 changes: 0 additions & 84 deletions internal/service/advancedcluster/move_state_test.go
View file
Open in desktop

This file was deleted.

138 changes: 0 additions & 138 deletions internal/service/advancedcluster/resource_advanced_cluster_state_upgrader_test.go
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion internal/service/advancedclustertpf/plan_modifier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
"github.com/hashicorp/terraform-plugin-testing/plancheck"
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"

"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/unit"
)

Expand All @@ -14,7 +15,6 @@ var (
repSpec1 = tfjsonpath.New("replication_specs").AtSliceIndex(1)
regionConfig0 = repSpec0.AtMapKey("region_configs").AtSliceIndex(0)
regionConfig1 = repSpec1.AtMapKey("region_configs").AtSliceIndex(0)
mockConfig = unit.MockConfigAdvancedClusterTPF
)

func autoScalingKnownValue(computeEnabled, diskEnabled, scaleDown bool, minInstanceSize, maxInstanceSize string) knownvalue.Check {
Expand Down
2 changes: 1 addition & 1 deletion ...source_advanced_cluster_migration_test.go → ...ncedclustertpf/resource_migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package advancedcluster_test
package advancedclustertpf_test

import (
"os"
Expand Down
4 changes: 3 additions & 1 deletion ...cluster/resource_advanced_cluster_test.go → ...rvice/advancedclustertpf/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package advancedcluster_test
package advancedclustertpf_test

import (
"errors"
"fmt"
"net/http"
"os"
Expand Down Expand Up @@ -62,6 +63,7 @@ var (
configServerManagementModeFixedToDedicated = "FIXED_TO_DEDICATED"
configServerManagementModeAtlasManaged = "ATLAS_MANAGED"
mockConfig = unit.MockConfigAdvancedClusterTPF
errGeneric = errors.New("generic")
)

func TestGetReplicationSpecAttributesFromOldAPI(t *testing.T) {
Expand Down
Loading
Loading