-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Allow import steps in beta IAM resource tests #7185
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.
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
Allow import steps in beta IAM resource tests #7185
Conversation
|
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 5 files changed, 16 insertions(+), 16 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccDataCatalogPolicyTagIamPolicyGenerated|TestAccDataCatalogPolicyTagIamBindingGenerated|TestAccDataCatalogPolicyTagIamMemberGenerated|TestAccComputeMachineImageIamPolicyGenerated_withCondition|TestAccComputeMachineImageIamMemberGenerated_withCondition|TestAccComputeMachineImageIamBindingGenerated_withCondition|TestAccComputeMachineImageIamPolicyGenerated|TestAccComputeMachineImageIamMemberGenerated|TestAccComputeMachineImageIamBindingGenerated|TestAccBigqueryDatapolicyDataPolicyIamMemberGenerated|TestAccBigqueryDatapolicyDataPolicyIamPolicyGenerated|TestAccApiGatewayGatewayIamPolicyGenerated|TestAccApiGatewayGatewayIamMemberGenerated|TestAccApiGatewayGatewayIamBindingGenerated|TestAccApiGatewayApiIamPolicyGenerated|TestAccApiGatewayApiIamMemberGenerated|TestAccApiGatewayApiIamBindingGenerated|TestAccApiGatewayApiConfigIamPolicyGenerated|TestAccApiGatewayApiConfigIamMemberGenerated|TestAccApiGatewayApiConfigIamBindingGenerated|TestAccFirebaserulesRelease_BasicRelease|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBasicExample|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBigqueryExample|TestAccComputeMachineImage_machineImageBasicExample|TestAccComputeMachineImage_computeMachineImageKmsExample|TestAccContainerCluster_failedCreation|TestAccApiGatewayApiConfig_apigatewayApiConfigGrpcFullExample|TestAccApiGatewayApiConfig_apigatewayApiConfigGrpcExample|TestAccApiGatewayGateway_apigatewayGatewayBasicExampleUpdated|TestAccApiGatewayGateway_apigatewayGatewayBasicExample|TestAccApiGatewayApi_apigatewayApiBasicExampleUpdated|TestAccApiGatewayGateway_apigatewayGatewayFullExample|TestAccApiGatewayApi_apigatewayApiFullExample|TestAccApiGatewayApiConfig_apigatewayApiConfigFullExample|TestAccApiGatewayApi_apigatewayApiBasicExample|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExampleUpdated|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExample|TestAccServiceDirectoryServiceIamPolicyGenerated|TestAccServiceDirectoryNamespaceIamMemberGenerated|TestAccServiceDirectoryNamespaceIamBindingGenerated|TestAccServiceDirectoryServiceIamMemberGenerated|TestAccRuntimeConfigConfigIamPolicyGenerated|TestAccServiceDirectoryServiceIamBindingGenerated|TestAccServiceDirectoryNamespaceIamPolicyGenerated|TestAccDataprocMetastoreFederationIamPolicyGenerated|TestAccDataprocMetastoreFederationIamMemberGenerated|TestAccDataprocMetastoreFederationIamBindingGenerated|TestAccDataCatalogTaxonomyIamBindingGenerated|TestAccDataCatalogTaxonomyIamMemberGenerated|TestAccDataCatalogTaxonomyIamPolicyGenerated |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
|
In general I like the idea of making the changes to iam_test_file.go.erb in this PR but I'm concerned that it'll make these tests start failing on all PRs until they're fixed. What would you think about either backing out the template changes or else finding a way to temporarily skip specifically those tests? |
|
After staring at this some more today, I'm leaning towards leaving the templates in this PR & skipping import tests for these IAM resources and fixing them in separate (bug) tickets / PRs. |
melinath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above - the problem being that we don't have a way to skip import tests for iam resources yet - only for resource tests (via skip_import_test). what would you think about adding skip_import_test for iam resources to skip these for now? (I'm not sure that is the best name & might end up needing to be set in a place that doesn't parallel the skip_import_test placement...)
Sorry, I've been juggling a few things and I'll need to re-pick up this PR(s) next week. I could have a go implementing skip import test for IAM resources, definitely. Another option for the acceptance test is to use this trick I learned recently: test that pulls value out of test, function that does the pulling. However as this would also not fit nicely into generated tests I think the first option is best |
|
As an update re: doing the above skip test generation for IAM resources I'm hoping to complete it next week before I take a week of PTO from Thursday 9th Feb onwards |
|
Ok, I'm finding the split of PRs confusing so I'm going to make this PR a PR to fix the template and add the ability to skip import steps. After, I'll open a new PR to fix the fixable tests I'll close the original PR once that's all done |
df5ec24 to
782de5a
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform Beta: Diff ( 4 files changed, 124 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccDataprocMetastoreFederation_dataprocMetastoreFederationBasicExample|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBigqueryExample|TestAccRegionInstanceGroupManager_stateful|TestAccComputeMachineImage_machineImageBasicExample|TestAccComputeMachineImage_computeMachineImageKmsExample|TestAccApiGatewayApi_apigatewayApiFullExample|TestAccApiGatewayGateway_apigatewayGatewayBasicExampleUpdated|TestAccApiGatewayApi_apigatewayApiBasicExample|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExampleUpdated|TestAccApiGatewayApiConfig_apigatewayApiConfigGrpcFullExample|TestAccApiGatewayGateway_apigatewayGatewayFullExample|TestAccApiGatewayApiConfig_apigatewayApiConfigGrpcExample|TestAccApiGatewayApi_apigatewayApiBasicExampleUpdated|TestAccApiGatewayApiConfig_apigatewayApiConfigBasicExample|TestAccApiGatewayApiConfig_apigatewayApiConfigFullExample|TestAccApiGatewayGateway_apigatewayGatewayBasicExample|TestAccDataprocMetastoreFederationIamPolicyGenerated|TestAccDataprocMetastoreFederationIamMemberGenerated|TestAccDataprocMetastoreFederationIamBindingGenerated|TestAccComputeMachineImageIamPolicyGenerated_withCondition|TestAccComputeMachineImageIamMemberGenerated_withCondition|TestAccComputeMachineImageIamBindingGenerated_withCondition|TestAccComputeMachineImageIamPolicyGenerated|TestAccComputeMachineImageIamMemberGenerated|TestAccComputeMachineImageIamBindingGenerated|TestAccCloudbuildv2ConnectionIamPolicyGenerated|TestAccCloudbuildv2ConnectionIamBindingGenerated|TestAccCloudbuildv2ConnectionIamMemberGenerated|TestAccDataSourceDnsRecordSet_basic|TestAccFrameworkProviderMeta_setModuleName|TestAccApiGatewayGatewayIamMemberGenerated|TestAccApiGatewayApiIamBindingGenerated|TestAccApiGatewayApiIamPolicyGenerated|TestAccApiGatewayGatewayIamPolicyGenerated|TestAccApiGatewayApiIamMemberGenerated|TestAccApiGatewayApiConfigIamMemberGenerated|TestAccApiGatewayGatewayIamBindingGenerated|TestAccApiGatewayApiConfigIamPolicyGenerated|TestAccApiGatewayApiConfigIamBindingGenerated |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
|
I'll add an ignore for Cloud Build IAM - added recently in #6778 (Required a rebase) |
782de5a to
323ad10
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform Beta: Diff ( 2 files changed, 62 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccDataprocMetastoreFederationIamMemberGenerated|TestAccDataprocMetastoreFederationIamBindingGenerated|TestAccDataprocMetastoreFederationIamPolicyGenerated|TestAccFirebaserulesRelease_BasicRelease|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBigqueryExample|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBasicExample|TestAccRegionInstanceGroupManager_stateful|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDnsManagedZone_basic|TestAccDataSourceDnsRecordSet_basic |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
|
☝️ Failing |
|
rebasing to allow checks to run |
323ad10 to
1d04094
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform Beta: Diff ( 2 files changed, 62 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBigqueryExample|TestAccDataprocMetastoreFederation_dataprocMetastoreFederationBasicExample|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccDataprocMetastoreFederationIamPolicyGenerated|TestAccDataprocMetastoreFederationIamMemberGenerated|TestAccDataprocMetastoreFederationIamBindingGenerated|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDnsRecordSet_basic|TestAccDataSourceDnsManagedZone_basic |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
|
As before, failing TestAccDataprocMetastoreFederationIam* tests are due to the API's errors, not due to import steps |
melinath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
/gcbrun |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccApigeeAddonsConfig_apigeeAddonsTestExample |
|
All tests are passing now that the dataprocmetastorefederation tests are skipped, so I think this is good to merge. |
|
Just got back from PTO - thanks for the review and approval! I'll merge Edit: rake test failures are related to the Ruby version changes not this PR |
* Remove ga-only logic from IAM test generation * Add ability to skip import steps in IAM resource tests * Add `skip_import_test` to beta tests that have broken import steps * Add `skip_import_test` to Dataproc Metastore IAM tests * Add `skip_import_test` to Cloud Build v2 IAM tests
* Remove ga-only logic from IAM test generation * Add ability to skip import steps in IAM resource tests * Add `skip_import_test` to beta tests that have broken import steps * Add `skip_import_test` to Dataproc Metastore IAM tests * Add `skip_import_test` to Cloud Build v2 IAM tests

Closes hashicorp/terraform-provider-google#12610
This PR contains:
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintto ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)