resourceIdentity: testing generation template
#14993
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
modular-magician
added
the
awaiting-approval
modular-magician
added
service/terraform
and removed
awaiting-approval
Collaborator
|
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.
|
Collaborator
|
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.
|
Collaborator
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 3561 Click here to see the affected service packages
Action takenFound 751 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Collaborator
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 3558 Click here to see the affected service packages
Action takenFound 750 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Collaborator
Author
|
/gcbrun |
modular-magician
added
awaiting-approval
Collaborator
|
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.
|
c2thorn
reviewed
Aug 27, 2025
Member
c2thorn
left a comment
c2thorn
left a comment
There was a problem hiding this comment.
This is a ton of tests that we are now duplicating the first example of. I have some concerns like:
- What is the overall test time added to our test suite with 500+ new tests?
- Should we be introducing resource identity + tests to a subset first?
- Can we just append the resource identity step as an extra test step to the first generated test? Instead of a completely new test that spins up new real resources?
Collaborator
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 3560 Click here to see the affected service packages
Action takenFound 757 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Collaborator
Author
are you talking about only having certain resources containing resource identity to start out? I'd wonder how'd we determine which ones to move forward with.
We can do that, something like adding a third step followiing the regular import can work func TestAccPubsubTopic_pubsubTopicBasicExample(t *testing.T) {
t.Parallel()
context := map[string]interface{}{
"random_suffix": acctest.RandString(t, 10),
}
acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckPubsubTopicDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccPubsubTopic_pubsubTopicBasicExample(context),
},
{
ResourceName: "google_pubsub_topic.example",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
},
{
ResourceName: "google_sql_user.user1",
ImportState: true,
ImportStateKind: resource.ImportBlockWithResourceIdentity, <--- this third step is what we can add
},
},
})
} |
Member
In generation we could filter out products alphabetically. Mostly a suggestion just to avoid the sheer amount of tests needed to be added/run per PR.
great, that seems like a much better way forward and should be a much smaller time addition |
modular-magician
added
awaiting-approval
Collaborator
|
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.
|
c2thorn
reviewed
Sep 3, 2025
Member
c2thorn
left a comment
c2thorn
left a comment
There was a problem hiding this comment.
yeah looks like VCR timed out, we'll need to batch the tests by product letter
|
@BBBmau, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
BBBmau
force-pushed
the
mmv1/test-generation
branch
from
0b9d0b9 to
297ef12
Compare
BBBmau
changed the title
resourceIdentity: testing generationresourceIdentity: testing generation template
Collaborator
|
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.
|
Collaborator
|
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.
|
Collaborator
Tests analyticsTotal tests: 119 Click here to see the affected service packages
Action takenFound 9 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Collaborator
|
🟢 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. |
Collaborator
Author
|
/gcbrun |
Collaborator
|
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.
|
Collaborator
Tests analyticsTotal tests: 188 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
|
Collaborator
|
🟢 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. |
Collaborator
Tests analyticsTotal tests: 3103 Click here to see the affected service packages
Action takenFound 372 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
c2thorn
approved these changes
Sep 16, 2025
BBBmau
merged commit 4ef6dac
into
GoogleCloudPlatform:FEATURE-BRANCH-resource-identity-generation
21 of 23 checks passed
This was referenced Sep 16, 2025
This was referenced Oct 3, 2025
Merged
Merged
BBBmau
added a commit
to BBBmau/magic-modules
that referenced
this pull request
Oct 31, 2025
This was referenced Nov 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.