Check example vars

[slevenick]

Fixes: hashicorp/terraform-provider-google#15679 (comment)

Check Vars and TestEnvVars to make sure that all variables used in examples have values specified in YAML configs. This does add significant file IO as we have to read every example file prior to generating tests.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1209
Skipped tests: 104
Affected tests: 3

Click here to see the affected service packages

• bigqueryanalyticshub
• compute
• gkebackup
• iap

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample
• TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample
• TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample [Debug log]
TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample [Debug log]
TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1209
Skipped tests: 104
Affected tests: 3

Click here to see the affected service packages

• compute
• gkebackup
• iap
• bigqueryanalyticshub

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample
• TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample
• TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample [Debug log]
TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample [Debug log]
TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1209
Skipped tests: 104
Affected tests: 3

Click here to see the affected service packages

• bigqueryanalyticshub
• compute
• gkebackup
• iap

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample
• TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample
• TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample [Debug log]
TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample [Debug log]
TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1209
Skipped tests: 104
Affected tests: 3

Click here to see the affected service packages

• gkebackup
• iap
• bigqueryanalyticshub
• compute

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample
• TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample
• TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample [Debug log]
TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample [Debug log]
TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1209
Skipped tests: 104
Affected tests: 3

Click here to see the affected service packages

• gkebackup
• iap
• bigqueryanalyticshub
• compute

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample
• TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample
• TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccBigqueryAnalyticsHubListingSubscription_bigqueryAnalyticshubListingSubscriptionBasicExample [Debug log]
TestAccGKEBackupBackupChannel_gkebackupBackupchannelBasicExample [Debug log]
TestAccGKEBackupRestoreChannel_gkebackupRestorechannelBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[zli82016]

Check Vars and TestEnvVars to make sure that all variables used in examples have values specified in YAML configs. This does add significant file IO as we have to read every example file prior to generating tests.

Does the file IO cause the provider generation time much longer?

[modular-magician]

Tests analytics

Total tests: 1316
Passed tests: 1212
Skipped tests: 104
Affected tests: 0

Click here to see the affected service packages

• compute
• gkebackup
• iap
• bigqueryanalyticshub

🟢 All tests passed!

View the build log

[zli82016]

The examples files are read automatically and then parsed during the template execution with the function ParseFiles in ExecuteTemplate

tmpl, err := template.New(templateFileName).Funcs(google.TemplateFunctions).ParseFiles(templates...)

To avoid reading the files twice, one for parsing, and the other one for validation, we can read the files once.

We can first read the file content manually for each file, and then parse the content using tmpl.Parse().

The content of the example file can be used for variables validation.

Does this option make sense?

[slevenick]

The examples files are read automatically and then parsed during the template execution with the function ParseFiles in ExecuteTemplate

tmpl, err := template.New(templateFileName).Funcs(google.TemplateFunctions).ParseFiles(templates...)

To avoid reading the files twice, one for parsing, and the other one for validation, we can read the files once.

We can first read the file content manually for each file, and then parse the content using tmpl.Parse().

The content of the example file can be used for variables validation.

Does this option make sense?

Yeah, that option makes sense. I'm a little confused at the structure of ExecuteTemplate though.

ExecuteTemplate within examples.go seems to be the central place that Go templates are executed through for the entire provider generation (resources... types... etc go through here). It's not limited to example generation, so it doesn't make sense to modify it for example-specific behavior.

I can split off ExecuteTemplate to be specific to *Examples, but I'm not sure if that's worth it.

[zli82016]

ExecuteTemplate within examples.go seems to be the central place that Go templates are executed through for the entire provider generation (resources... types... etc go through here). It's not limited to example generation, so it doesn't make sense to modify it for example-specific behavior.

I can split off ExecuteTemplate to be specific to *Examples, but I'm not sure if that's worth it.

Sorry for the confusion.
ExecuteTemplate in examples.go is only for *Examples. We can modify it for examples execution.

The other generation (resources, tests and docs) uses the template in
https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/provider/template_data.go

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1321
Passed tests: 1217
Skipped tests: 104
Affected tests: 0

Click here to see the affected service packages

• bigqueryanalyticshub
• compute
• gkebackup
• iap

🟢 All tests passed!

View the build log

[zli82016]

LGTM. Thanks for the fix.

Rebasing the main branch is needed to address the unit test failure.

[modular-magician]

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.

google provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
google-beta provider: Diff ( 25 files changed, 56 insertions(+), 56 deletions(-))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+), 4 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1321
Passed tests: 1217
Skipped tests: 104
Affected tests: 0

Click here to see the affected service packages

• bigqueryanalyticshub
• compute
• gkebackup
• iap

🟢 All tests passed!

View the build log

[ScottSuarez]

Fixes: hashicorp/terraform-provider-google#15679 (comment)

Check Vars and TestEnvVars to make sure that all variables used in examples have values specified in YAML configs. This does add significant file IO as we have to read every example file prior to generating tests.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

Why don't we add this as a unit test instead. Similar to https://github.com/ScottSuarez/magic-modules/blob/c3e7ad903b246f3f92ed5191ee526e485adf3bf4/mmv1/validate_third_party_test.go

[rileykarson]

For both cases could they be returned by the compiler as warnings, and upgraded to errors in CI? Unit tests are unlikely to be run by contributors locally, while errors at runtime can block development. Warnings w/ warn2err in CI would theoretically both notify folks in local runs & enforce compliance.

[ScottSuarez]

For both cases could they be returned by the compiler as warnings, and upgraded to errors in CI? Unit tests are unlikely to be run by contributors locally, while errors at runtime can block development. Warnings w/ warn2err in CI would theoretically both notify folks in local runs & enforce compliance.

My nit is adding compilation runtime for what I would consider unit test behavior. I agree that the user would catch more often before proposing a pr if apart of build, but I am not sure it is worth an impact to overall build time. Pending the actual impact to build time it is likely a non issue. It is just the more of the things like this we do, deaths by a thousand cuts.

[rileykarson]

Fair consideration! I think bazel may allow us to parallelise more (especially if we integrate more deeply over time, i.e. breaking service packages into dedicated build rules) and will help keep latency down. If it doesn't or we do see runtime increases we'd want to restructure, yeah.