Skip to content
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

Add regression test for type check warnings #2389

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

VenelinMartinov
Copy link
Contributor

@VenelinMartinov VenelinMartinov commented Sep 17, 2024

This adds a regression test for the warnings we issue to users when a property has the wrong type. This could result in panics down the line, so we need to catch them early.

Testing this in the bridge is currently quite hard as we can't use YAML for this because it is too strict in type-checking. We only have unit tests which indirectly test this but these break under PRC.

This was introduced in pulumi/pulumi-terraform-bridge#1987
original GCP issue: pulumi/pulumi-terraform-bridge#1979

Bridge issue for integration tests for the feature: pulumi/pulumi-terraform-bridge#2418

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

@guineveresaenger
Copy link
Contributor

Can we add a bit of context what "panics down the line" means? What is the scenario we're trying to catch with this test? We currently catch upstream panics and issue warnings, so I'm curious what other type of panic we'd like to prevent, or maybe I'm just misreading here.

When this test fails, does it point at a type checker bug regression in the bridge?

Test itself looks reasonable.

@VenelinMartinov
Copy link
Contributor Author

Yeah, I meant that the TF provider might panic if passed wrongly typed input, so we catch TF panics if a type mismatch occurs and issue a sensible warning.

When this test fails, that does point to a regression in the bridge. Ideally we'd pull this in the bridge and I've opened pulumi/pulumi-terraform-bridge#2418 for that.

@VenelinMartinov VenelinMartinov merged commit 99484f2 into master Sep 17, 2024
24 checks passed
@VenelinMartinov VenelinMartinov deleted the vvm/type_check_warning_regression_test branch September 17, 2024 16:05
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v8.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants