-
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
refactor: update resource manager tag references #12132
refactor: update resource manager tag references #12132
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. |
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: 1198 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 during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
I’ll double check the changes, and try to run some of the failed ones locally - do the errors make it clear if the failures are due to this change? |
At least the BQ one seems to be This one seems like a bad config, missing a quote or something: TestAccComputeRegionNetworkFirewallPolicyWithRules_computeRegionNetworkFirewallPolicyWithRulesFullExample The others may be unrelated |
@slevenick I have a fix for the BQ one that I'm pushing up (works locally). I couldn't figure out how to run the generated test locally, but I found the errant quote, so updating that as well. |
ccab35e
to
e94928c
Compare
e94928c
to
d1810c1
Compare
hashicorp/terraform-provider-google#19885 << I think this is still not fixed and that may be the issue with |
I don't know if it's the right fix, but opened #12149 on the off chance that it is. |
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: 1198 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 passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
@slevenick can you try re-running those tests now that the test fix is merged? |
Update resource manager tag references in terraform code templates and some minor formatting changes - Update the code to use foo.id vs "tagKeys/${foo.name}" or "tagValues/${foo.name}", and foo.namespaced_name vs. "${data.google_project.project.project_id}/${foo.short_name}" - Wrap map keys in parens where needed. Followup to GoogleCloudPlatform#12118
d0276a6
to
d3bcaf9
Compare
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
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: 1209 Click here to see the affected service packages
🟢 All tests passed! View the build log |
Part 2:
Update resource manager tag references in terraform code templates for generated tests (also some minor formatting changes)
foo.id
vs"tagKeys/${foo.name}"
or"tagValues/${foo.name}"
project.id
vsprojects/.....number
Followup to #12118
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.