Skip to content

tfv: Fix GetOk field name in hand written iam files#6745

Merged
melinath merged 2 commits into
GoogleCloudPlatform:mainfrom
iyabchen:fix-bracket-iam
Nov 11, 2022
Merged

tfv: Fix GetOk field name in hand written iam files#6745
melinath merged 2 commits into
GoogleCloudPlatform:mainfrom
iyabchen:fix-bracket-iam

Conversation

@iyabchen

@iyabchen iyabchen commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

b/255831148
terraform validator: GetOk() function should not have {{}} within the parameter.

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

@iyabchen iyabchen requested a review from melinath October 26, 2022 22:13
@modular-magician

modular-magician commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

TF Validator: Diff ( 7 files changed, 8 insertions(+), 8 deletions(-))

@modular-magician

modular-magician commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2210
Passed tests 1971
Skipped tests: 239
Failed tests: 0

All tests passed in REPLAYING mode
View the build log

melinath
melinath requested changes Oct 27, 2022
View reviewed changes

@melinath melinath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes seem reasonable - it would be great to add tests for these resources to make sure they're actually working otherwise.

@modular-magician

modular-magician commented Nov 1, 2022

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 1 file changed, 111 insertions(+))
Terraform Beta: Diff ( 1 file changed, 122 insertions(+))
TF Validator: Diff ( 20 files changed, 586 insertions(+), 494 deletions(-))

@modular-magician

modular-magician commented Nov 1, 2022

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2210
Passed tests 1971
Skipped tests: 239
Failed tests: 0

All tests passed in REPLAYING mode
View the build log

@modular-magician

modular-magician commented Nov 9, 2022

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 1 file changed, 112 insertions(+))
Terraform Beta: Diff ( 1 file changed, 124 insertions(+))
TF Validator: Diff ( 10 files changed, 248 insertions(+), 10 deletions(-))

@modular-magician

modular-magician commented Nov 9, 2022

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2235
Passed tests 1992
Skipped tests: 241
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests
TestAccFirebaserulesRelease_BasicRelease|TestAccComputeForwardingRule_update

@modular-magician

modular-magician commented Nov 9, 2022

Copy link
Copy Markdown
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeForwardingRule_update[Debug log]

All tests passed
View the build log or the debug log for each test

@iyabchen

iyabchen commented Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

these changes seem reasonable - it would be great to add tests for these resources to make sure they're actually working otherwise.

Test added, generating a new file to test the FullFetchResource. Created mmv1/third_party/terraform/utils/config_test_utils.go.erb to accept the test server as parameter, so it can mock the iam api fetch.

melinath
melinath approved these changes Nov 11, 2022
View reviewed changes

@melinath melinath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 this is really cool! Love that we have a pattern to directly unit test the fetch behavior - and that it will automatically cover all IAM resources.

@melinath melinath merged commit 31a061c into GoogleCloudPlatform:main Nov 11, 2022
This was referenced Nov 11, 2022
Merged
Merged
Merged
@iyabchen iyabchen deleted the fix-bracket-iam branch November 16, 2022 18:10
googlerjk pushed a commit to googlerjk/magic-modules that referenced this pull request Nov 22, 2022
@iyabchen @googlerjk
tfv: Fix GetOk field name in hand written iam files (GoogleCloudPlatf…
d553d0c 
…orm#6745)

* tfv: Fix GetOk field name in hand written iam files

* tfv: Add unit test for IAM FetchFullResource
googlerjk pushed a commit to googlerjk/magic-modules that referenced this pull request Nov 25, 2022
@iyabchen @googlerjk
tfv: Fix GetOk field name in hand written iam files (GoogleCloudPlatf…
a5b526a 
…orm#6745)

* tfv: Fix GetOk field name in hand written iam files

* tfv: Add unit test for IAM FetchFullResource
hao-nan-li pushed a commit to hao-nan-li/magic-modules that referenced this pull request Dec 6, 2022
@iyabchen @hao-nan-li
tfv: Fix GetOk field name in hand written iam files (GoogleCloudPlatf…
abbd3a8 
…orm#6745)

* tfv: Fix GetOk field name in hand written iam files

* tfv: Add unit test for IAM FetchFullResource
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@melinath melinath melinath approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iyabchen @modular-magician @melinath