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

Reduce tfgen failing example warning verbosity #1843

Merged
merged 7 commits into from
Apr 4, 2024
Merged

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Apr 4, 2024

Before this change we emitted a lot of duplicated warnings for examples that failed to convert. This change removes the
duplication and aggregates similar errors so that when every language conversion fails with the same error, which is
common when the failure pertains to the HCL->PCL pass of the conversion, we get one warning only not many.

* <nil>: unexpected HCL snippet in Convert "\nThis is some intentionally broken HCL that should not convert.\n{}";
* <nil>: unexpected HCL snippet in Convert "\nThis is some intentionally broken HCL that should not convert.\n{}";
* <nil>: unexpected HCL snippet in Convert "\nThis is some intentionally broken HCL that should not convert.\n{}";
autogold.Expect(`warning: unable to convert HCL example for Pulumi entity '#/resources/azure:webpubsub/customCertificate:CustomCertificate': 1 error occurred:
Copy link
Member Author

Choose a reason for hiding this comment

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

This change demonstrates how stderr is affected.

Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

I had a couple questions, and I would like to see a few comments explaining our approach to consolidating the errors but this looks good to me.

pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
@t0yv0 t0yv0 enabled auto-merge (squash) April 4, 2024 19:55
Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Attention: Patch coverage is 78.37838% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 60.51%. Comparing base (88fafd0) to head (a636452).

Files Patch % Lines
pkg/tfgen/docs.go 78.37% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1843      +/-   ##
==========================================
- Coverage   60.91%   60.51%   -0.40%     
==========================================
  Files         303      310       +7     
  Lines       42431    42822     +391     
==========================================
+ Hits        25847    25914      +67     
- Misses      15121    15439     +318     
- Partials     1463     1469       +6     

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

@t0yv0 t0yv0 disabled auto-merge April 4, 2024 21:00
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 4, 2024

Missed handling the partial failure case that's still super duplicative. Adding a few more commits here shortly.

@t0yv0
Copy link
Member Author

t0yv0 commented Apr 4, 2024

Done now

Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

LGTM

@t0yv0 t0yv0 merged commit 78394c5 into master Apr 4, 2024
10 checks passed
@t0yv0 t0yv0 deleted the t0yv0/dedup-warnings branch April 4, 2024 22:50
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.

None yet

2 participants