Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents #7555

SarahFrench · 2023-03-29T18:53:07Z

This PR:

Adds unit tests for the getProject function that depends on the provider SDK
Adds unit tests for the getProjectFramework function, a version that that depends on the plugin framework instead of the SDK
Refactors existing unit tests for util functions:
- TestGetRegion
- TestGetZone
- Made one pre-existing test use sub-tests
- Moved the location and name of another pre-existing test

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

modular-magician · 2023-03-29T18:53:23Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

SarahFrench · 2023-03-29T19:08:12Z

mmv1/third_party/terraform/utils/provider_test.go.erb

I moved this test to a different file as it's not really a 'provider' test, though that may have made sense at the time this code was written!

It's now called TestGetRegionFromZone

modular-magician · 2023-03-29T19:08:39Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 209 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 4 files changed, 209 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-03-29T19:43:01Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-03-29T20:37:54Z

Tests analytics

Total tests: 2581
Passed tests 2285
Skipped tests: 267
Affected tests: 29

Action taken

Found 29 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeRegionPerInstanceConfig_statefulIps|TestAccComputePerInstanceConfig_statefulIps|TestAccComputeRegionBackendService_withBackendMultiNic|TestAccComputeInstanceTemplate_IPv6|TestAccComputeInstanceTemplate_secondaryAliasIpRange|TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample|TestAccInstanceGroupManager_stateful|TestAccComputeInstanceTemplate_subnet_auto|TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample|TestAccComputeInstanceTemplate_subnet_custom|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample|TestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample|TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample|TestAccComposerEnvironment_withWebServerConfig|TestAccComposerEnvironmentAirflow2_withSoftwareConfig|TestAccComposerEnvironment_withSoftwareConfig|TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample|TestAccComputeForwardingRule_forwardingRuleHttpLbExample|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccCloudRunV2Job_cloudrunv2JobVpcaccessExample|TestAccFrameworkProviderMeta_setModuleName|TestAccComposerEnvironment_UpdateComposerV2WithTriggerer|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDNSKeys_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceGoogleFirebaseAppleAppConfig|TestAccDataSourceDnsRecordSet_basic|TestAccFrameworkProviderBasePath_setBasePath

Get to know how VCR tests work

modular-magician · 2023-03-29T21:26:26Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeRegionPerInstanceConfig_statefulIps[Debug log]
TestAccComputePerInstanceConfig_statefulIps[Debug log]
TestAccComputeRegionBackendService_withBackendMultiNic[Debug log]
TestAccComputeInstanceTemplate_IPv6[Debug log]
TestAccComputeInstanceTemplate_secondaryAliasIpRange[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample[Debug log]
TestAccInstanceGroupManager_stateful[Debug log]
TestAccComputeInstanceTemplate_subnet_auto[Debug log]
TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample[Debug log]
TestAccComputeInstanceTemplate_subnet_custom[Debug log]
TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample[Debug log]
TestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample[Debug log]
TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample[Debug log]
TestAccComposerEnvironment_withWebServerConfig[Debug log]
TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleHttpLbExample[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccCloudRunV2Job_cloudrunv2JobVpcaccessExample[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]

Tests failed during RECORDING mode:
TestAccComposerEnvironmentAirflow2_withSoftwareConfig[Error message] [Debug log]
TestAccComposerEnvironment_withSoftwareConfig[Error message] [Debug log]
TestAccComposerEnvironment_UpdateComposerV2WithTriggerer[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

SarahFrench · 2023-03-30T15:10:19Z

👆 That's the last little tweak from me, I learned a better way to omit a value in a framework test

modular-magician · 2023-03-30T15:26:13Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-03-30T16:16:26Z

Tests analytics

Total tests: 2581
Passed tests 2301
Skipped tests: 267
Affected tests: 13

Action taken

Found 13 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComposerEnvironmentAirflow2_withSoftwareConfig|TestAccComputeForwardingRule_update|TestAccComposerEnvironment_withSoftwareConfig|TestAccComposerEnvironment_UpdateComposerV2WithTriggerer|TestAccFrameworkProviderBasePath_setBasePath|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccDataSourceDNSKeys_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceDnsRecordSet_basic|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceGoogleFirebaseAppleAppConfig

Get to know how VCR tests work

modular-magician · 2023-03-30T17:04:33Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComposerEnvironmentAirflow2_withSoftwareConfig[Debug log]
TestAccComputeForwardingRule_update[Debug log]
TestAccComposerEnvironment_withSoftwareConfig[Debug log]
TestAccFrameworkProviderBasePath_setBasePath[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccDataSourceDNSKeys_basic[Debug log]
TestAccDataSourceDnsManagedZone_basic[Debug log]
TestAccDataSourceDNSKeys_noDnsSec[Debug log]
TestAccDataSourceDnsRecordSet_basic[Debug log]
TestAccFrameworkProviderMeta_setModuleName[Debug log]
TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]

Tests failed during RECORDING mode:
TestAccComposerEnvironment_UpdateComposerV2WithTriggerer[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

zli82016

LGTM. Please rebase the main branch because of the merged PR #7446. Thanks.

modular-magician · 2023-04-04T10:06:18Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-04-04T10:07:40Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

SarahFrench · 2023-04-04T13:34:07Z

I'll look into the failure later today

Edit: it's due to the issues with the compute region instance template data source, see: hashicorp/terraform-provider-google#14183

I'll sit and wait until that's fixed

modular-magician · 2023-04-04T18:11:29Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-04-04T18:12:49Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

`ResourceComputeRegionInstanceTemplate` was removed?

SarahFrench · 2023-04-04T19:00:12Z

Seems like the issue is that my test was using something that has since been removed from TPGB, very confusing

modular-magician · 2023-04-04T19:16:56Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
Terraform Beta: Diff ( 5 files changed, 261 insertions(+), 66 deletions(-))
TF Validator: Diff ( 3 files changed, 6 insertions(+), 3 deletions(-))

modular-magician · 2023-04-04T20:06:53Z

Tests analytics

Total tests: 2548
Passed tests 2269
Skipped tests: 275
Affected tests: 4

Action taken

Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComposerEnvironment_withEncryptionConfigComposer2|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccApigeeEnvKeystoreAliasPkcs12_apigeeEnvKeystoreAliasPkcs12Example

Get to know how VCR tests work

modular-magician · 2023-04-04T20:31:34Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComposerEnvironment_withEncryptionConfigComposer2[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccApigeeEnvKeystoreAliasPkcs12_apigeeEnvKeystoreAliasPkcs12Example[Debug log]

All tests passed
View the build log or the debug log for each test

…in framework equivalents (GoogleCloudPlatform#7555) * Add unit tests for `getProject` util * Refactor existing `TestGetZone` test * Move existing `getRegionFromZone` test alongside other util unit tests * Rename `getRegionFromZone` unit test * Refactor `TestGetRegion` unit test * Simplify acc test setup code * Change unit test to use subtests via `t.Run` * Add unit test for `getProjectFramework` util function * Fix comment * Update test to set null value more explicitly using `types.StringNull()` * Replace example schema for test `ResourceComputeRegionInstanceTemplate` was removed?

SarahFrench commented Mar 29, 2023

View reviewed changes

SarahFrench marked this pull request as ready for review March 29, 2023 19:39

SarahFrench requested a review from a team as a code owner March 29, 2023 19:39

SarahFrench requested review from zli82016 and removed request for a team March 29, 2023 19:39

SarahFrench requested a review from megan07 March 30, 2023 15:04

zli82016 approved these changes Apr 3, 2023

View reviewed changes

SarahFrench force-pushed the utils-test branch from 3a6f37b to e5417e7 Compare April 4, 2023 09:49

SarahFrench added 10 commits April 4, 2023 18:54

Add unit tests for getProject util

466042a

Refactor existing TestGetZone test

5bf561d

Move existing getRegionFromZone test alongside other util unit tests

9880519

Rename getRegionFromZone unit test

225c151

Refactor TestGetRegion unit test

648333f

Simplify acc test setup code

097f2a1

Change unit test to use subtests via t.Run

2abb624

Add unit test for getProjectFramework util function

ca33bba

Fix comment

b27886c

Update test to set null value more explicitly using types.StringNull()

e165f14

SarahFrench force-pushed the utils-test branch from e5417e7 to e165f14 Compare April 4, 2023 17:54

Replace example schema for test

214a80c

`ResourceComputeRegionInstanceTemplate` was removed?

zli82016 approved these changes Apr 4, 2023

View reviewed changes

SarahFrench merged commit e06e095 into GoogleCloudPlatform:main Apr 5, 2023

SarahFrench deleted the utils-test branch March 26, 2024 11:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents #7555

Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents #7555

SarahFrench commented Mar 29, 2023 •

edited

Loading

modular-magician commented Mar 29, 2023

SarahFrench Mar 29, 2023 •

edited

Loading

modular-magician commented Mar 29, 2023

modular-magician commented Mar 29, 2023

modular-magician commented Mar 29, 2023

modular-magician commented Mar 29, 2023

SarahFrench commented Mar 30, 2023

modular-magician commented Mar 30, 2023

modular-magician commented Mar 30, 2023

modular-magician commented Mar 30, 2023

zli82016 left a comment

modular-magician commented Apr 4, 2023

modular-magician commented Apr 4, 2023

SarahFrench commented Apr 4, 2023 •

edited

Loading

modular-magician commented Apr 4, 2023

modular-magician commented Apr 4, 2023

SarahFrench commented Apr 4, 2023

modular-magician commented Apr 4, 2023

modular-magician commented Apr 4, 2023

modular-magician commented Apr 4, 2023

Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents #7555

Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents #7555

Conversation

SarahFrench commented Mar 29, 2023 • edited Loading

modular-magician commented Mar 29, 2023

SarahFrench Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

modular-magician commented Mar 29, 2023

Diff report

modular-magician commented Mar 29, 2023

Diff report

modular-magician commented Mar 29, 2023

Tests analytics

Action taken

modular-magician commented Mar 29, 2023

SarahFrench commented Mar 30, 2023

modular-magician commented Mar 30, 2023

Diff report

modular-magician commented Mar 30, 2023

Tests analytics

Action taken

modular-magician commented Mar 30, 2023

zli82016 left a comment

Choose a reason for hiding this comment

modular-magician commented Apr 4, 2023

Diff report

modular-magician commented Apr 4, 2023

Tests analytics

SarahFrench commented Apr 4, 2023 • edited Loading

modular-magician commented Apr 4, 2023

Diff report

modular-magician commented Apr 4, 2023

Tests analytics

SarahFrench commented Apr 4, 2023

modular-magician commented Apr 4, 2023

Diff report

modular-magician commented Apr 4, 2023

Tests analytics

Action taken

modular-magician commented Apr 4, 2023

SarahFrench commented Mar 29, 2023 •

edited

Loading

SarahFrench Mar 29, 2023 •

edited

Loading

SarahFrench commented Apr 4, 2023 •

edited

Loading