-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Composer #12463
Composer #12463
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @slevenick, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Tests analyticsTotal tests: 50 Click here to see the affected service packages
🟢 All tests passed! 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.
We can also remove corresponding configurations: testAccComposerEnvironment_update (on line 2588) and testAccComposerEnvironment_webServerCfg (on line 1943), they are not used in any other tests.
Done |
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: 0 Click here to see the affected service packages
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR. View the build log |
Tests analyticsTotal tests: 50 Click here to see the affected service packages
🟢 All tests passed! 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.
These tests are valuable and should not be removed without a good reason.
If the specific label is causing the issue, please rework the test to work as intended but to not set this label or add logic to the resource to prevent sending these labels as described in hashicorp/terraform-provider-google#20249 (comment)
This label is automatically added "Version 6.0.0 introduced the goog-terraform-provisioned attribution label, which is added automatically to all Terraform-provisioned resources. This is causing problems with the google_composer_environment resource.". The tests cannot be reworked, because these labels are passed automatically and are not accepted by the API. We are not planning to change API and I got a green light to delete these tests, because 1) Composer 1 creation is not supported anymore 2) Composer 1 update operations are partially covered with other tests. |
Co-authored-by: rlapin-pl <[email protected]>
Remove 2 Composer 1 tests that are failing due to labels with google- prefix.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.