Swap to go-junit-report for parsing go test output

src/test/BUILD

@@ -15,6 +15,7 @@ go_library(
         "//src/utils",
         "//src/worker",
         "//third_party/go:cover",
+        "//third_party/go:go-junit-report",
         "//third_party/go:logging",
     ],
 )

src/test/go_results.go

@@ -1,143 +1,68 @@
 // Parser for output from Go's testing package.
-//
-// This is a fairly straightforward microformat so pretty easy to parse ourselves.
-// There's at least one package out there to convert it to JUnit XML but not worth
-// the complexity of getting that installed as a standalone tool.
 
 package test
 
 import (
 	"bytes"
 	"fmt"
-	"regexp"
-	"strconv"
 	"strings"
-	"time"
+
+	parser "github.com/jstemmer/go-junit-report/parser"
 
 	"github.com/thought-machine/please/src/core"
 )
 
-// Not sure what the -6 suffixes are about.
-var testStart = regexp.MustCompile(`^=== RUN (.*)(?:-6)?$`)
-var testResult = regexp.MustCompile(`^ *--- (PASS|FAIL|SKIP): (.*)(?:-6)? \(([0-9]+\.[0-9]+)s\)$`)
-
 func parseGoTestResults(data []byte) (core.TestSuite, error) {
+
+	testPackage, err := parser.Parse(bytes.NewReader(data), "")
+
+	if err != nil {
+		return core.TestSuite{}, fmt.Errorf("Failed to parse go test output: %w", err)
+	}
+
+	results := fromGoJunitReport(testPackage)
+
+	return results, nil
+}
+
+// Conversion between a go-junit-report Report into a core.TestSuite.
+// A Package is mapped to TestSuite & Tests mapped onto testCases.
+func fromGoJunitReport(report *parser.Report) core.TestSuite {
 	results := core.TestSuite{}
-	lines := bytes.Split(data, []byte{'\n'})
-	testsStarted := map[string]bool{}
-	var suiteDuration time.Duration
-	testOutput := make([]string, 0)
-	for i, line := range lines {
-		testStartMatches := testStart.FindSubmatch(line)
-		testResultMatches := testResult.FindSubmatch(line)
-		if testStartMatches != nil {
-			testsStarted[strings.TrimSpace(string(testStartMatches[1]))] = true
-		} else if testResultMatches != nil {
-			testName := strings.TrimSpace(string(testResultMatches[2]))
-			if !testsStarted[testName] {
-				continue
-			}
-			f, _ := strconv.ParseFloat(string(testResultMatches[3]), 64)
-			duration := time.Duration(f * float64(time.Second))
-			suiteDuration += duration
-			testCase := core.TestCase{
-				Name: testName,
-			}
-			if bytes.Equal(testResultMatches[1], []byte("PASS")) {
-				testCase.Executions = append(testCase.Executions, core.TestExecution{
-					Duration: &duration,
-					Stderr:   strings.Join(testOutput, ""),
-				})
-			} else if bytes.Equal(testResultMatches[1], []byte("SKIP")) {
-				// The skip message is found at the bottom of the test output segment.
-				// Prior to Go 1.14 the test output segment follows the results line.
-				// In Go 1.14 the test output segment sits between the start line and the results line.
-				outputLines := getTestOutputLines(i, lines)
-				skipMessage := ""
-				if len(outputLines) > 0 {
-					skipMessage = strings.TrimSpace(outputLines[len(outputLines)-1])
-				}
 
-				testCase.Executions = append(testCase.Executions, core.TestExecution{
+	for _, pkg := range report.Packages {
+		for _, test := range pkg.Tests {
+			coreTestCase := core.TestCase{Name: test.Name}
+			testOutput := strings.Join(test.Output, "\n")
+
+			if test.Result == parser.PASS {
+				coreTestCase.Executions = append(coreTestCase.Executions, core.TestExecution{
+					Stderr:   testOutput,
+					Duration: &test.Duration,
+				})
+			} else if test.Result == parser.SKIP {
+				coreTestCase.Executions = append(coreTestCase.Executions, core.TestExecution{
 					Skip: &core.TestResultSkip{
-						Message: skipMessage,
+						// Given the possibility of test setup, teardowns & custom logging, we can't do anything
+						// more targeted than using the whole test output as the skip message.
+						Message: testOutput,
 					},
-					Stderr:   strings.Join(testOutput, ""),
-					Duration: &duration,
+					Stderr:   testOutput,
+					Duration: &test.Duration,
 				})
 			} else {
-
-				outputLines := getTestOutputLines(i, lines)
-
-				output := strings.Join(outputLines, "\n")
-				testCase.Executions = append(testCase.Executions, core.TestExecution{
+				// A "FAIL" result
+				coreTestCase.Executions = append(coreTestCase.Executions, core.TestExecution{
 					Failure: &core.TestResultFailure{
-						Traceback: output,
+						Traceback: testOutput,
 					},
-					Stderr:   strings.Join(testOutput, ""),
-					Duration: &duration,
+					Stderr:   testOutput,
+					Duration: &test.Duration,
 				})
 			}
-			results.TestCases = append(results.TestCases, testCase)
-			testOutput = make([]string, 0)
-		} else if bytes.Equal(line, []byte("PASS")) {
-			// Do nothing, all's well.
-		} else if bytes.Equal(line, []byte("FAIL")) {
-			if results.Failures() == 0 {
-				return results, fmt.Errorf("Test indicated final failure but no failures found yet")
-			}
-		} else {
-			testOutput = append(testOutput, string(line), "\n")
+			results.TestCases = append(results.TestCases, coreTestCase)
 		}
+		results.Duration += pkg.Duration
 	}
-	results.Duration = suiteDuration
-	return results, nil
-}
-
-func getTestOutputLines(currentIndex int, lines [][]byte) []string {
-	if resultLooksPriorGo114(currentIndex, lines) {
-		return getPostResultOutput(currentIndex, lines)
-	}
-	return getPreResultOutput(currentIndex, lines)
-}
-
-// Go test output looks prior to 114 if the previous line matches against a start test block.
-// Only fully applicable for failing and skipped tests as a message may not
-// appear for passed tests.
-func resultLooksPriorGo114(currentIndex int, lines [][]byte) bool {
-	if currentIndex == 0 {
-		return false
-	}
-
-	prevLine := lines[currentIndex-1]
-	prevLineMatchesStart := testStart.FindSubmatch(prevLine)
-
-	return prevLineMatchesStart != nil
-}
-
-// Get the output for Go test output prior to Go 1.14
-func getPostResultOutput(resultsIndex int, lines [][]byte) []string {
-	output := []string{}
-	for j := resultsIndex + 1; j < len(lines) && !lineMatchesRunOrResultsLine(lines[j]); j++ {
-		output = append(output, string(lines[j]))
-	}
-
-	return output
-}
-
-// Get output for Go tests output after Go 1.14
-func getPreResultOutput(resultsIndex int, lines [][]byte) []string {
-	output := []string{}
-	for j := resultsIndex - 1; j > 0 && !lineMatchesRunOrResultsLine(lines[j]); j-- {
-		output = append([]string{string(lines[j])}, output...)
-	}
-	return output
-}
-
-func lineMatchesRunOrResultsLine(line []byte) bool {
-
-	testStartMatches := testStart.FindSubmatch(line)
-	matchesRunLine := (testStartMatches != nil)
-
-	return matchesRunLine || bytes.Equal(line, []byte("PASS")) || bytes.Equal(line, []byte("FAIL"))
+	return results
 }

src/test/results_test.go

@@ -60,7 +60,7 @@ func TestGoSkippedMessage114(t *testing.T) {
 	require.NoError(t, err)
 
 	var skippedTC = getFirstSkippedTestCase(results)
-	assert.Equal(t, "TestSomething: my_test.go:9: This test is skipped", skippedTC.Executions[0].Skip.Message)
+	assert.Equal(t, "    TestSomething: my_test.go:8: This thing is also here\n    TestSomething: my_test.go:9: This test is skipped", skippedTC.Executions[0].Skip.Message)
 }
 
 func getFirstSkippedTestCase(ts core.TestSuite) *core.TestCase {
@@ -78,7 +78,7 @@ func TestGoFailedTraceback(t *testing.T) {
 	require.NoError(t, err)
 
 	var failedTC = getFirstFailedTestCase(results)
-	assert.Equal(t, "\tresults_test.go:11: Unable to parse file: EOF", failedTC.Executions[0].Failure.Traceback)
+	assert.Equal(t, "results_test.go:11: Unable to parse file: EOF", failedTC.Executions[0].Failure.Traceback)
 }
 
 // Go 1.14 changes the ordering of failed messages in Go tests
@@ -169,11 +169,6 @@ func TestGoIgnoreUnknownOutput(t *testing.T) {
 	assert.Equal(t, 0, results.Skips())
 }
 
-func TestGoFailIfUnknownTestPasses(t *testing.T) {
-	_, err := parseTestResultsFile("src/test/test_data/go_test_unknown_test.txt")
-	assert.Error(t, err)
-}
-
 func TestParseGoFileWithNoTests(t *testing.T) {
 	_, err := parseTestResultsFile("src/test/test_data/go_empty_test.txt")
 	assert.NoError(t, err)

third_party/go/BUILD

@@ -637,3 +637,10 @@ go_get(
     revision = "v1.0.0",
     test_only = True,
 )
+
+go_get(
+    name = "go-junit-report",
+    get = "github.com/jstemmer/go-junit-report/...",
+    licences = ["MIT"],
+    revision = "984a47ca6b0a7d704c4b589852051b4d7865aa17",
+)