-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add acceptance tests for how provider handles request_reason
argument
#11835
Add acceptance tests for how provider handles request_reason
argument
#11835
Conversation
… `google_provider_config_plugin_framework` data source
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.
|
Tests analyticsTotal tests: 4105 Click here to see the affected service packages
Tests were added that are skipped in VCR:
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
Tests running in TeamCity: |
Approved the PR for testing branch settings earlier. Sorry for the noise
mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go
Outdated
Show resolved
Hide resolved
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.
|
Tests analyticsTotal tests: 4103 Click here to see the affected service packages
View the build log |
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.
took a moment to follow how the tests were operating, but LGTM!
Thanks for the review! Yeah, I probably should have left a note about the 2 data sources used in these acceptance tests. You've read through it now but they basically show what values the SDK and PF sides of the provider have been configured with. Basically non-user facing versions of google_client_config that expose all values impacted by user-supplied arguments. It makes tests a lot easier versus trying to test things indirectly, though indirect testing is still needed for the 'usage' test cases in particular. |
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.
|
Tests analyticsTotal tests: 4116 Click here to see the affected service packages
Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
Going ahead with merge as the VCR test failure above, after I addressed merge conflicts, isn't related to this PR |
This PR:
X-Goog-Request-Reason
header is for auditing purposes in GCPWriting these tests made me realise that empty string validation doesn't cover request_reason, so I opened: hashicorp/terraform-provider-google#19643 . The acceptance tests are written to define the current behaviour and my expectation is that the PF schema will be update in future to add validation, make those tests fail, and the tests will be updated.
Release Note Template for Downstream PRs (will be copied)