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

Detailed diff tests for block types #2398

Merged
merged 8 commits into from
Sep 16, 2024

Conversation

VenelinMartinov
Copy link
Contributor

@VenelinMartinov VenelinMartinov commented Sep 6, 2024

This adds tests for the detailed diff output of

  • List blocks
  • Set blocks
  • MaxItemsOne list blocks

for adding, removing, changing the property and adding, removing, changing nested properties in the block.

A few issues here:

@VenelinMartinov VenelinMartinov changed the title Detailed diff block tests Detailed diff tests for block types Sep 6, 2024
Copy link

codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.66%. Comparing base (f0fee4f) to head (89e4b45).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2398   +/-   ##
=======================================
  Coverage   57.66%   57.66%           
=======================================
  Files         369      369           
  Lines       50148    50148           
=======================================
+ Hits        28917    28918    +1     
  Misses      19652    19652           
+ Partials     1579     1578    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

`),
false,
},
// TODO: where is the nested prop diff coming from
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: replace with links to the relevant issues.

Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

It looks like tests are still failing.

Other then that and the impending TODOs, this looks good to me.

@t0yv0
Copy link
Member

t0yv0 commented Sep 9, 2024

Add these issues to the epic please.

map[string]interface{}{"setProps": []interface{}{"val2", "val3"}},
map[string]interface{}{"setProps": []interface{}{"val1", "val2", "val3"}},
autogold.Expect(`Previewing update (test):
pulumi:pulumi:Stack: (same)
Copy link
Member

Choose a reason for hiding this comment

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

Nice, autogodl expect actual Pulumi output is highly trustworthy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that the contract for the detailed diff response between the engine and the provider is not very trustworthy, this seems like the best thing we can do.

@t0yv0
Copy link
Member

t0yv0 commented Sep 9, 2024

Feel free to check in when CI allows. Skip tests that do not pass yet.

@t0yv0 t0yv0 self-requested a review September 9, 2024 14:53
@VenelinMartinov VenelinMartinov enabled auto-merge (squash) September 16, 2024 20:09
@VenelinMartinov VenelinMartinov enabled auto-merge (squash) September 16, 2024 20:12
@VenelinMartinov VenelinMartinov merged commit 16dec78 into master Sep 16, 2024
11 checks passed
@VenelinMartinov VenelinMartinov deleted the vvm/detailed_diff_block_tests branch September 16, 2024 20:49
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v3.91.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.

4 participants