search

pulumi / pulumi-terraform-bridge

A library allowing providers built with the Terraform Plugin SDK to be bridged into Pulumi.
Apache License 2.0
183 stars 41 forks source link

Unknown tests #2054

Closed VenelinMartinov closed 3 weeks ago

VenelinMartinov commented 3 weeks ago

This adds GRPC around unknowns for both attributes and collections for both PlanResourceChange and non-PlanResourceChange

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.48%. Comparing base (b5efa0d) to head (f2a07a8). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2054 +/- ## ========================================== + Coverage 61.36% 61.48% +0.11% ========================================== Files 334 334 Lines 44951 44964 +13 ========================================== + Hits 27583 27644 +61 + Misses 15841 15796 -45 + Partials 1527 1524 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

VenelinMartinov commented 3 weeks ago

Both of these assert on the current behaviour for both plan resource change and non-plan resource change.

I built it out to compare behaviour in https://github.com/pulumi/pulumi-terraform-bridge/pull/1971 and make sure we aren't regressing.

I'd like to merge as is and leave refactoring for later as https://github.com/pulumi/pulumi-terraform-bridge/pull/1971 depends on this and it has been hanging for quite a while

VenelinMartinov commented 3 weeks ago

The structure of the tests it that for each of the property it injects an unknown at every possible level and asserts on the response.

VenelinMartinov commented 3 weeks ago

Comparing PlanResourceChange and non-PlanResourceChange is not the point here

VenelinMartinov commented 3 weeks ago

GH token rate limits ❤️

t0yv0 commented 3 weeks ago

@iwahbe we need some plumbing work to do to allow cross-tests to express unknowns in the meanwhile we test at this level. I requested a few additional comments/cross-links but it looks good. It was very important to me to have some edge cases covered here with regression tests and some explanation as to why we think it behaves the way it does.

VenelinMartinov commented 3 weeks ago

I've done some further digging around the results here in https://github.com/pulumi/pulumi-terraform-bridge/issues/2032 and https://github.com/pulumi/pulumi-terraform-bridge/pull/2060/files and https://github.com/pulumi/pulumi-terraform-bridge/pull/2060/files

I suspect we can do better with unknowns