Backport of Add more details to JUnit terraform test output to describe why a test was skipped into v1.11 (#36347)

* Add ability for TestJUnitXMLFile to access data about whether the test runner was Stopped

* Add details to XML describing why a Run was skipped

* Fix wording

* Code consistency changes

* Move all JUnit-related code down to where it's used

Away from the Views section of the code where it was relevant before

* Move JUnit-related error and warning diags to above where cancellable contexts are created

* Fix wording of user feedback

* Fix test to match updated skipped message text

* Fix test

* Add missing changes from a1716b8

---------

Co-authored-by: Sarah French <[email protected]>

Author: github-actions[bot]

internal/backend/local/test.go

@@ -86,6 +86,10 @@ func (runner *TestSuiteRunner) Stop() {
 	runner.Stopped = true
 }
 
+func (runner *TestSuiteRunner) IsStopped() bool {
+	return runner.Stopped
+}
+
 func (runner *TestSuiteRunner) Cancel() {
 	runner.Cancelled = true
 }

internal/cloud/test.go

@@ -109,6 +109,10 @@ func (runner *TestSuiteRunner) Stop() {
 	runner.Stopped = true
 }
 
+func (runner *TestSuiteRunner) IsStopped() bool {
+	return runner.Stopped
+}
+
 func (runner *TestSuiteRunner) Cancel() {
 	runner.Cancelled = true
 }

internal/command/junit/junit.go

@@ -40,6 +40,9 @@ type TestJUnitXMLFile struct {
 
 	// A config loader is required to access sources, which are used with diagnostics to create XML content
 	configLoader *configload.Loader
+
+	// A pointer to the containing test suite runner is needed to monitor details like the command being stopped
+	testSuiteRunner moduletest.TestSuiteRunner
 }
 
 type JUnit interface {
@@ -55,10 +58,11 @@ var _ JUnit = (*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, configLoader *configload.Loader) *TestJUnitXMLFile {
+func NewTestJUnitXMLFile(filename string, configLoader *configload.Loader, testSuiteRunner moduletest.TestSuiteRunner) *TestJUnitXMLFile {
 	return &TestJUnitXMLFile{
-		filename:     filename,
-		configLoader: configLoader,
+		filename:        filename,
+		configLoader:    configLoader,
+		testSuiteRunner: testSuiteRunner,
 	}
 }
 
@@ -69,7 +73,7 @@ func (v *TestJUnitXMLFile) Save(suite *moduletest.Suite) tfdiags.Diagnostics {
 
 	// Prepare XML content
 	sources := v.configLoader.Parser().Sources()
-	xmlSrc, err := junitXMLTestReport(suite, sources)
+	xmlSrc, err := junitXMLTestReport(suite, v.testSuiteRunner.IsStopped(), sources)
 	if err != nil {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
@@ -130,7 +134,7 @@ type testCase struct {
 	Timestamp string  `xml:"timestamp,attr,omitempty"`
 }
 
-func junitXMLTestReport(suite *moduletest.Suite, sources map[string][]byte) ([]byte, error) {
+func junitXMLTestReport(suite *moduletest.Suite, suiteRunnerStopped bool, sources map[string][]byte) ([]byte, error) {
 	var buf bytes.Buffer
 	enc := xml.NewEncoder(&buf)
 	enc.EncodeToken(xml.ProcInst{
@@ -182,7 +186,7 @@ func junitXMLTestReport(suite *moduletest.Suite, sources map[string][]byte) ([]b
 			},
 		})
 
-		for _, run := range file.Runs {
+		for i, run := range file.Runs {
 			// Each run is a "test case".
 
 			testCase := testCase{
@@ -201,9 +205,10 @@ func junitXMLTestReport(suite *moduletest.Suite, sources map[string][]byte) ([]b
 			}
 			switch run.Status {
 			case moduletest.Skip:
+				message, body := getSkipDetails(i, file, suiteRunnerStopped)
 				testCase.Skipped = &withMessage{
-					// FIXME: Is there something useful we could say here about
-					// why the test was skipped?
+					Message: message,
+					Body:    body,
 				}
 			case moduletest.Fail:
 				testCase.Failure = &withMessage{
@@ -248,6 +253,37 @@ func junitXMLTestReport(suite *moduletest.Suite, sources map[string][]byte) ([]b
 	return buf.Bytes(), nil
 }
 
+// getSkipDetails checks data about the test suite, file and runs to determine why a given run was skipped
+// Test can be skipped due to:
+// 1. terraform test recieving an interrupt from users; all unstarted tests will be skipped
+// 2. A previous run in a file has failed, causing subsequent run blocks to be skipped
+func getSkipDetails(runIndex int, file *moduletest.File, suiteStopped bool) (string, string) {
+	if suiteStopped {
+		// Test suite experienced an interrupt
+		// This block only handles graceful Stop interrupts, as Cancel interrupts will prevent a JUnit file being produced at all
+		message := "Testcase skipped due to an interrupt"
+		body := "Terraform received an interrupt and stopped gracefully. This caused all remaining testcases to be skipped"
+
+		return message, body
+	}
+
+	if file.Status == moduletest.Error {
+		// Overall test file marked as errored in the context of a skipped test means tests have been skipped after
+		// a previous test/run blocks has errored out
+		for i := runIndex; i >= 0; i-- {
+			if file.Runs[i].Status == moduletest.Error {
+				// Skipped due to error in previous run within the file
+				message := "Testcase skipped due to a previous testcase error"
+				body := fmt.Sprintf("Previous testcase %q ended in error, which caused the remaining tests in the file to be skipped", file.Runs[i].Name)
+				return message, body
+			}
+		}
+	}
+
+	// Unhandled case: This results in <skipped></skipped> with no attributes or body
+	return "", ""
+}
+
 func suiteFilesAsSortedList(files map[string]*moduletest.File) []*moduletest.File {
 	fileNames := make([]string, len(files))
 	i := 0

internal/command/junit/junit_internal_test.go

@@ -0,0 +1,153 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+package junit
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/hashicorp/terraform/internal/moduletest"
+)
+
+func Test_TestJUnitXMLFile_save(t *testing.T) {
+
+	cases := map[string]struct {
+		filename    string
+		expectError bool
+	}{
+		"can save output to the specified filename": {
+			filename: func() string {
+				td := t.TempDir()
+				return fmt.Sprintf("%s/output.xml", td)
+			}(),
+		},
+		"returns an error when given a filename that isn't absolute or relative": {
+			filename:    "~/output.xml",
+			expectError: true,
+		},
+	}
+
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			j := TestJUnitXMLFile{
+				filename: tc.filename,
+			}
+
+			xml := []byte(`<?xml version="1.0" encoding="UTF-8"?><testsuites>
+  <testsuite name="example_1.tftest.hcl" tests="1" skipped="0" failures="0" errors="0">
+    <testcase name="true_is_true" classname="example_1.tftest.hcl" time="0.005381209"></testcase>
+  </testsuite>
+</testsuites>`)
+
+			diags := j.save(xml)
+
+			if diags.HasErrors() {
+				if !tc.expectError {
+					t.Fatalf("got unexpected error: %s", diags.Err())
+				}
+				// return early if testing error case
+				return
+			}
+
+			if !diags.HasErrors() && tc.expectError {
+				t.Fatalf("expected an error but got none")
+			}
+
+			fileContent, err := os.ReadFile(tc.filename)
+			if err != nil {
+				t.Fatalf("unexpected error opening file")
+			}
+
+			if !bytes.Equal(fileContent, xml) {
+				t.Fatalf("wanted XML:\n%s\n got XML:\n%s\n", string(xml), string(fileContent))
+			}
+		})
+	}
+}
+
+func Test_suiteFilesAsSortedList(t *testing.T) {
+	cases := map[string]struct {
+		Suite         *moduletest.Suite
+		ExpectedNames map[int]string
+	}{
+		"no test files": {
+			Suite: &moduletest.Suite{},
+		},
+		"3 test files ordered in map": {
+			Suite: &moduletest.Suite{
+				Status: moduletest.Skip,
+				Files: map[string]*moduletest.File{
+					"test_file_1.tftest.hcl": {
+						Name:   "test_file_1.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+					"test_file_2.tftest.hcl": {
+						Name:   "test_file_2.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+					"test_file_3.tftest.hcl": {
+						Name:   "test_file_3.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+				},
+			},
+			ExpectedNames: map[int]string{
+				0: "test_file_1.tftest.hcl",
+				1: "test_file_2.tftest.hcl",
+				2: "test_file_3.tftest.hcl",
+			},
+		},
+		"3 test files unordered in map": {
+			Suite: &moduletest.Suite{
+				Status: moduletest.Skip,
+				Files: map[string]*moduletest.File{
+					"test_file_3.tftest.hcl": {
+						Name:   "test_file_3.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+					"test_file_1.tftest.hcl": {
+						Name:   "test_file_1.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+					"test_file_2.tftest.hcl": {
+						Name:   "test_file_2.tftest.hcl",
+						Status: moduletest.Skip,
+						Runs:   []*moduletest.Run{},
+					},
+				},
+			},
+			ExpectedNames: map[int]string{
+				0: "test_file_1.tftest.hcl",
+				1: "test_file_2.tftest.hcl",
+				2: "test_file_3.tftest.hcl",
+			},
+		},
+	}
+
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			list := suiteFilesAsSortedList(tc.Suite.Files)
+
+			if len(tc.ExpectedNames) != len(tc.Suite.Files) {
+				t.Fatalf("expected there to be %d items, got %d", len(tc.ExpectedNames), len(tc.Suite.Files))
+			}
+
+			if len(tc.ExpectedNames) == 0 {
+				return
+			}
+
+			for k, v := range tc.ExpectedNames {
+				if list[k].Name != v {
+					t.Fatalf("expected element %d in sorted list to be named %s, got %s", k, v, list[k].Name)
+				}
+			}
+		})
+	}
+}