-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 project
arguments
#11607
Add acceptance tests for how provider handles project
arguments
#11607
Conversation
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: 3883 Click here to see the affected service packages
Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
|
TestAccFwProvider_project did run, but wasn't reported here. Look in gcs bucket for file |
This failure is weird, as I can get the original version of the acctest's checks to pass locally. I assume that there's a difference in the CI environment and my laptop that affects the data source being refreshed or not? I assume the plugin-framework version won't be as flaky as unknown handling is more explicit and less of "lucky timing" where a data source refreshes before you try to use its attributes in dependent resources |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
b86f569
to
83a0020
Compare
This comment has been minimized.
This comment has been minimized.
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.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Tests analyticsTotal tests: 3907 Click here to see the affected service packages
Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
|
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: 3804 Click here to see the affected service packages
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 failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer. |
…ameworkProvider_LoadAndValidateFramework_project`
62ce0dd
to
d33c692
Compare
This comment has been minimized.
This comment has been minimized.
mmv1/third_party/terraform/fwprovider/framework_provider_project_test.go
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -14,168 +14,6 @@ import ( | |||
transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport" | |||
) | |||
|
|||
func TestFrameworkProvider_LoadAndValidateFramework_project(t *testing.T) { |
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.
Do you mind adding this function back? Thanks.
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.
Added in f66e3ad
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.
Ah sorry, this is the go rewrite file - I'll push it back
Tests analyticsTotal tests: 3996 Click here to see the affected service packages
Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
|
…unit-test level. Add comments to group test cases by type.
Tests analyticsTotal tests: 3998 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
|
|
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. The failed test looks like unrelated.
…ersion of framework_config_test.go
I realised that I was confused before - once that latest push completes it's automation I'll merge the PR. Thank you for the review & bearing with me on the weird Unknown behaviour! |
Tests analyticsTotal tests: 4036 Click here to see the affected service packages
Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
Tests analyticsTotal tests: 4043 Click here to see the affected service packages
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
|
That resolves the missing test, merging now! |
This PR adds acc tests to test how the provider handles the project argument, using the new datasources
google_provider_config_plugin_framework
andgoogle_provider_config_sdk
.In the existing unit tests there is a test case in the
TestFrameworkProvider_LoadAndValidateFramework_project
test called"when project is an unknown value, the provider treats it as if it's unset and uses an environment variable instead"
.I added a test case called
"when project is unknown in the config, environment variables are used"
in the new SDK and PF acc tests. However the test case is flakey and has different outcomes on different runs. Because of this I'm going to leave testing unknown handling to unit tests only.Release Note Template for Downstream PRs (will be copied)