From e336d98ab668625ca370f35444912f2d61e6197c Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 27 Feb 2024 11:50:09 -0500 Subject: [PATCH] Count all errors in the example coverage tracker (#1713) There is an anomaly in the stats in https://github.com/pulumi/pulumi-azuread/pull/484 where YAML has fewer examples counted in the denominator than other languages, this should be impossible. Looks like some error handling assumptions do not apply to PULUMI_CONVERT=1 anymore and was swallowing errors. With this small tweak the errors for the failed examples now show up in the coverage tracker: ``` warning: failed to convert HCL for #/functions/azuread:index/getDomains:getDomains to yaml: :3,11-43: Failed to generate YAML program: Splat; A 'Splat' expression is equivalent in expressive power to a 'for loop', and is not representable in YAML. If the values of the function are known, you can manually unroll the loop, otherwise it is necessary to switch to a more expressive language., and 1 other diagnostic(s) ``` --- pkg/tfgen/docs.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/pkg/tfgen/docs.go b/pkg/tfgen/docs.go index c8c78b560..09e274ff2 100644 --- a/pkg/tfgen/docs.go +++ b/pkg/tfgen/docs.go @@ -1590,12 +1590,36 @@ func (g *Generator) legacyConvert( func (g *Generator) convertHCLToString(e *Example, hclCode, path, languageName string) (string, error) { fileName := fmt.Sprintf("/%s.tf", strings.ReplaceAll(path, "/", "-")) + failure := func(diags hcl.Diagnostics) error { + // Remove the temp filename from the error, since it will be confusing to users of the bridge who do not know + // we write an example to a temp file internally in order to pass to convert.Convert(). + // + // fileName starts with a "/" which is not present in the resulting error, so we need to skip the first rune. + errMsg := strings.ReplaceAll(diags.Error(), fileName[1:], "") + + g.warn("failed to convert HCL for %s to %v: %v", path, languageName, errMsg) + g.coverageTracker.languageConversionFailure(e, languageName, diags) + return fmt.Errorf(errMsg) + } + var convertedHcl string var diags hcl.Diagnostics var err error if cliConverterEnabled() { + // The cliConverter has a slightly different error behavior as it can return both + // err and diags but does not panic. Handle this by re-coding err as a diag and + // proceeding to handle diags normally. convertedHcl, diags, err = g.cliConverter().Convert(hclCode, languageName) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: err.Error(), + }) + } + if diags.HasErrors() { + return "", failure(diags) + } } else { convertedHcl, diags, err = g.legacyConvert(e, hclCode, fileName, languageName) } @@ -1612,15 +1636,7 @@ func (g *Generator) convertHCLToString(e *Example, hclCode, path, languageName s return "", fmt.Errorf("failed to convert HCL for %s to %v: %w", path, languageName, err) } if diags.HasErrors() { - // Remove the temp filename from the error, since it will be confusing to users of the bridge who do not know - // we write an example to a temp file internally in order to pass to convert.Convert(). - // - // fileName starts with a "/" which is not present in the resulting error, so we need to skip the first rune. - errMsg := strings.ReplaceAll(diags.Error(), fileName[1:], "") - - g.warn("failed to convert HCL for %s to %v: %v", path, languageName, errMsg) - g.coverageTracker.languageConversionFailure(e, languageName, diags) - return "", fmt.Errorf(errMsg) + return "", failure(diags) } g.coverageTracker.languageConversionSuccess(e, languageName, convertedHcl)