feat: Updates migration tests to run separately and use last released version of provider for plan checks #1565

maastha · 2023-10-25T07:54:52Z

Description

This PR adds a new Github action workflow 'Migration Tests' and separates out existing migration acceptance tests into this workflow only i.e. migration tests will no longer run as part of existing Acceptance tests workflows in PRs.

What are 'Migration tests'?
Migration tests are similar to Terraform acceptance tests but focus on ensuring that empty plans are generated when upgrading from a previous released version to the current version of the Terraform provider. See documentation.

Releases: This workflow will run during release and use the last released version of our provider to run migration tests to ensure no unexpected plans are generated when our users upgrade.

Manual: This workflow can be manually run if required, using labels or via UI.

We can eventually disable these tests to run for every PR as migration tests usually cover scenarios already covered by acceptance tests, and require to only run these during releases or manually.

NOTE: This workflow was not tested for a run during release.
NOTE: This PR also introduces convention to use prefix TestAccMigration* for all migration tests.

Link to any related issue(s):

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contribution guidelines
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals, I defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.

Further comments

maastha · 2023-10-25T09:18:50Z

unit-test step failing due to issue with Adds coverage report to PRs step

lantoli · 2023-10-25T09:28:16Z

.github/workflows/code-health.yml

@@ -88,7 +88,7 @@ jobs:
      - uses: actions/checkout@v4
      - name: Run ShellCheck
        uses: bewuethr/shellcheck-action@v2
-  call-accettance-tests-workflow:
+  call-acceptance-tests-workflow:


finally, thanks! :-)

lantoli · 2023-10-25T09:31:29Z

.github/workflows/migration-tests.yml

+
+  project: 
+    needs: [ change-detection ]
+    if: ${{ needs.change-detection.outputs.project == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event.label.name == 'run-testacc-migration' || github.event.label.name == 'run-testacc-project-migration' || inputs.parent-event-name == 'release' }}


i know we have this in other gh actions but I wonder if we could make this easier or have some functions or something

perhaps we could start fixing this file and then a follow-up to change the others?

@lantoli I couldn't find a way to make this reusable or use a function, is there any example or reference you could share?

I'm not sure about the best way, but for instance what about creating a variable in the workflow and use the variable? something like:

on: ...

env:
hasToTrigger: ${{ needs.change-detection.outputs.config == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event.label.name == 'run-testacc-migration' || github.event.label.name == 'run-testacc-config-migration' || inputs.parent-event-name == 'release' }}

job...

and use:

if: ${{ env.hasToTrigger }}

or use it for the common part and then:
if: ${{ env.hasToTrigger }} || specific conditions here...

since this doesn't hold a static value, rather holds a condition, i don't believe this is possible to do via a variable as well

i suppose it's possible while you use variables that are know before starting execution

Addressed, see comment #1565 (comment)

lantoli · 2023-10-25T09:31:55Z

.github/workflows/migration-tests.yml

+
+  config:
+    needs: [ change-detection ]
+    if: ${{ needs.change-detection.outputs.config == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event.label.name == 'run-testacc-migration' || github.event.label.name == 'run-testacc-config-migration' || inputs.parent-event-name == 'release' }}


lantoli · 2023-10-25T09:32:59Z

mongodbatlas/fw_resource_mongodbatlas_database_user_migration_test.go

+		orgID                 = os.Getenv("MONGODB_ATLAS_ORG_ID")
+		projectName           = acctest.RandomWithPrefix("test-acc")
+		clusterName           = acctest.RandomWithPrefix("test-acc-cluster")
+		lastVersionConstraint = os.Getenv("MONGODB_ATLAS_LAST_VERSION")


do we need to check that this env var exists?

nice catch, will update

marcosuma

minor comments, but LGTM!

.github/workflows/migration-tests.yml

marcosuma · 2023-10-25T10:10:44Z

.github/workflows/migration-tests.yml

+
+  project: 
+    needs: [ change-detection ]
+    if: ${{ needs.change-detection.outputs.project == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event.label.name == 'run-testacc-migration' || github.event.label.name == 'run-testacc-project-migration' || inputs.parent-event-name == 'release' }}


perhaps we could start fixing this file and then a follow-up to change the others?

AgustinBettati

great idea having the version update automatically, gives proper value to the migration tests

maastha · 2023-10-25T16:16:03Z

.github/workflows/migration-tests.yml

+    types: [ labeled ]
+
+jobs: 
+  set-output:


@lantoli tried with another step: https://github.com/mongodb/terraform-provider-mongodbatlas/pull/1565/files#diff-9e63d3ed50a83cef3c77bb7447f2858374bc51e6dd94ac7f65b3420205c70ea6
This is an example PR running this workflow: maastha#1

Let me know if this approach makes sense, not sure if this is possible with a variable

that's great, thanks!

marcosuma · 2023-10-30T15:52:20Z

I just realised something now: why do we call them migration? shouldn't it be backwardCompatibiltyTest?

lantoli · 2023-10-30T18:18:53Z

I just realised something now: why do we call them migration? shouldn't it be backwardCompatibiltyTest?

@marcosuma I think it's Hashicorp naming

maastha added 9 commits October 25, 2023 05:40

initial

30dbcd7

update git get release

5747e2e

update tests

4dd4e1a

update action

316afdf

update migration tests

b8a83f9

update project tests

89333ab

update project tests

08c1f51

update events

857de3e

update events

11cf6fe

maastha changed the title ~~feat: migration tests~~ feat: Update migration tests to run separately and use last released version of provider for plan checks Oct 25, 2023

maastha marked this pull request as ready for review October 25, 2023 08:50

maastha requested a review from a team as a code owner October 25, 2023 08:50

maastha changed the title ~~feat: Update migration tests to run separately and use last released version of provider for plan checks~~ feat: Updates migration tests to run separately and use last released version of provider for plan checks Oct 25, 2023

lantoli reviewed Oct 25, 2023

View reviewed changes

lantoli approved these changes Oct 25, 2023

View reviewed changes

marcosuma approved these changes Oct 25, 2023

View reviewed changes

maastha added 4 commits October 25, 2023 12:35

update pre checks

599b74b

remove comments

b3a07f7

fix online archive

0ea6a6a

fix online archive

4b1e27a

AgustinBettati approved these changes Oct 25, 2023

View reviewed changes

fix git event condition

a5fb8fe

maastha commented Oct 25, 2023

View reviewed changes

maastha merged commit c3ff8a0 into mongodb:master Oct 31, 2023
7 of 8 checks passed

AgustinBettati mentioned this pull request Oct 31, 2023

fix: overrides to attribute behavior for resource elems. #1572

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Updates migration tests to run separately and use last released version of provider for plan checks #1565

feat: Updates migration tests to run separately and use last released version of provider for plan checks #1565

maastha commented Oct 25, 2023 •

edited

Loading

maastha commented Oct 25, 2023

lantoli Oct 25, 2023

lantoli Oct 25, 2023

marcosuma Oct 25, 2023

maastha Oct 25, 2023

lantoli Oct 25, 2023

lantoli Oct 25, 2023

maastha Oct 25, 2023

lantoli Oct 25, 2023

maastha Oct 25, 2023

lantoli Oct 25, 2023

lantoli Oct 25, 2023

maastha Oct 25, 2023

maastha Oct 25, 2023

marcosuma left a comment

marcosuma Oct 25, 2023

AgustinBettati left a comment

maastha Oct 25, 2023

lantoli Oct 26, 2023

marcosuma commented Oct 30, 2023

lantoli commented Oct 30, 2023

feat: Updates migration tests to run separately and use last released version of provider for plan checks #1565

feat: Updates migration tests to run separately and use last released version of provider for plan checks #1565

Conversation

maastha commented Oct 25, 2023 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

maastha commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosuma commented Oct 30, 2023

lantoli commented Oct 30, 2023

maastha commented Oct 25, 2023 •

edited

Loading