From 2c71f26a5490b05d473973011170bff35229fef3 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 10 Jan 2025 12:20:08 +0000 Subject: [PATCH] Provide sources to junitXMLTestReport, allowing complete error messages in the XML We need to ensure that artifact.NewTestJUnitXMLFile is called once the config Loader is available as a non-nil pointer --- internal/command/artifact/junit.go | 25 +++++++++++------- internal/command/test.go | 42 ++++++++++++++++-------------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/internal/command/artifact/junit.go b/internal/command/artifact/junit.go index 65c925c8fabb..99e381711e7c 100644 --- a/internal/command/artifact/junit.go +++ b/internal/command/artifact/junit.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/command/format" + "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/moduletest" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -33,6 +34,9 @@ import ( type TestJUnitXMLFile struct { filename string + + // A config loader is required to access sources, which are used with diagnostics to create XML content + configLoader *configload.Loader } type Artifact interface { @@ -48,16 +52,19 @@ var _ Artifact = (*TestJUnitXMLFile)(nil) // point of being asked to write a conclusion. Otherwise it will create the // file at that time. If creating or overwriting the file fails, a subsequent // call to method Err will return information about the problem. -func NewTestJUnitXMLFile(filename string) *TestJUnitXMLFile { +func NewTestJUnitXMLFile(filename string, configLoader *configload.Loader) *TestJUnitXMLFile { return &TestJUnitXMLFile{ - filename: filename, + filename: filename, + configLoader: configLoader, } } func (v *TestJUnitXMLFile) Save(suite *moduletest.Suite) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - xmlSrc, err := junitXMLTestReport(suite) + // Prepare XML content + sources := v.configLoader.Parser().Sources() + xmlSrc, err := junitXMLTestReport(suite, sources) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -66,6 +73,8 @@ func (v *TestJUnitXMLFile) Save(suite *moduletest.Suite) tfdiags.Diagnostics { }) return diags } + + // Save XML to the specified path err = os.WriteFile(v.filename, xmlSrc, 0660) if err != nil { diags = diags.Append(&hcl.Diagnostic{ @@ -79,7 +88,7 @@ func (v *TestJUnitXMLFile) Save(suite *moduletest.Suite) tfdiags.Diagnostics { return diags } -func junitXMLTestReport(suite *moduletest.Suite) ([]byte, error) { +func junitXMLTestReport(suite *moduletest.Suite, sources map[string][]byte) ([]byte, error) { var buf bytes.Buffer enc := xml.NewEncoder(&buf) enc.EncodeToken(xml.ProcInst{ @@ -191,9 +200,7 @@ func junitXMLTestReport(suite *moduletest.Suite) ([]byte, error) { case moduletest.Error: var diagsStr strings.Builder for _, diag := range run.Diagnostics { - // FIXME: Pass in the sources so that these diagnostics - // can include source snippets when appropriate. - diagsStr.WriteString(format.DiagnosticPlain(diag, nil, 80)) + diagsStr.WriteString(format.DiagnosticPlain(diag, sources, 80)) } testCase.Error = &WithMessage{ Message: "Encountered an error", @@ -208,9 +215,7 @@ func junitXMLTestReport(suite *moduletest.Suite) ([]byte, error) { // they'll be reported _somewhere_ at least. var diagsStr strings.Builder for _, diag := range run.Diagnostics { - // FIXME: Pass in the sources so that these diagnostics - // can include source snippets when appropriate. - diagsStr.WriteString(format.DiagnosticPlain(diag, nil, 80)) + diagsStr.WriteString(format.DiagnosticPlain(diag, sources, 80)) } testCase.Stderr = &WithMessage{ Body: diagsStr.String(), diff --git a/internal/command/test.go b/internal/command/test.go index 09692664caff..e3afe58be014 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -101,6 +101,26 @@ func (c *TestCommand) Run(rawArgs []string) int { } view := views.NewTest(args.ViewType, c.View) + + // The specified testing directory must be a relative path, and it must + // point to a directory that is a descendant of the configuration directory. + if !filepath.IsLocal(args.TestDirectory) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid testing directory", + "The testing directory must be a relative path pointing to a directory local to the configuration directory.")) + + view.Diagnostics(nil, nil, diags) + return 1 + } + + config, configDiags := c.loadConfigWithTests(".", args.TestDirectory) + diags = diags.Append(configDiags) + if configDiags.HasErrors() { + view.Diagnostics(nil, nil, diags) + return 1 + } + var junit *artifact.TestJUnitXMLFile if args.JUnitXMLFile != "" { // JUnit XML output is currently experimental, so that we can gather @@ -115,31 +135,15 @@ func (c *TestCommand) Run(rawArgs []string) int { view.Diagnostics(nil, nil, diags) return 1 } + diags = diags.Append(tfdiags.Sourceless( tfdiags.Warning, "JUnit XML output is experimental", "The -junit-xml option is currently experimental and therefore subject to breaking changes or removal, even in patch releases.", )) - junit = artifact.NewTestJUnitXMLFile(args.JUnitXMLFile) - } - - // The specified testing directory must be a relative path, and it must - // point to a directory that is a descendant of the configuration directory. - if !filepath.IsLocal(args.TestDirectory) { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid testing directory", - "The testing directory must be a relative path pointing to a directory local to the configuration directory.")) - view.Diagnostics(nil, nil, diags) - return 1 - } - - config, configDiags := c.loadConfigWithTests(".", args.TestDirectory) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - view.Diagnostics(nil, nil, diags) - return 1 + // This line must happen after the TestCommand's calls loadConfigWithTests and has the configLoader field set + junit = artifact.NewTestJUnitXMLFile(args.JUnitXMLFile, c.configLoader) } // Users can also specify variables via the command line, so we'll parse