Revert "Swap to go-junit-report for parsing go test output (thought-m…

…achine#982)"

This reverts commit 13adc19.

src/test/BUILD

@@ -15,7 +15,6 @@ 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,68 +1,143 @@
 // 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"
-
-	parser "github.com/jstemmer/go-junit-report/parser"
+	"time"
 
 	"github.com/thought-machine/please/src/core"
 )
 
-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)
+// 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\)$`)
 
-	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 {
+func parseGoTestResults(data []byte) (core.TestSuite, error) {
 	results := core.TestSuite{}
-
-	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,
+	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 test.Result == parser.SKIP {
-				coreTestCase.Executions = append(coreTestCase.Executions, core.TestExecution{
+			} 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{
 					Skip: &core.TestResultSkip{
-						// 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,
+						Message: skipMessage,
 					},
-					Stderr:   testOutput,
-					Duration: &test.Duration,
+					Stderr:   strings.Join(testOutput, ""),
+					Duration: &duration,
 				})
 			} else {
-				// A "FAIL" result
-				coreTestCase.Executions = append(coreTestCase.Executions, core.TestExecution{
+
+				outputLines := getTestOutputLines(i, lines)
+
+				output := strings.Join(outputLines, "\n")
+				testCase.Executions = append(testCase.Executions, core.TestExecution{
 					Failure: &core.TestResultFailure{
-						Traceback: testOutput,
+						Traceback: output,
 					},
-					Stderr:   testOutput,
-					Duration: &test.Duration,
+					Stderr:   strings.Join(testOutput, ""),
+					Duration: &duration,
 				})
 			}
-			results.TestCases = append(results.TestCases, coreTestCase)
+			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.Duration += pkg.Duration
 	}
-	return results
+	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"))
 }

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:8: This thing is also here\n    TestSomething: my_test.go:9: This test is skipped", skippedTC.Executions[0].Skip.Message)
+	assert.Equal(t, "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, "results_test.go:11: Unable to parse file: EOF", failedTC.Executions[0].Failure.Traceback)
+	assert.Equal(t, "\tresults_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,6 +169,11 @@ 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)

src/test/test_data/go_test_unknown_test.txt

@@ -0,0 +1,6 @@
+=== RUN   TestA
+--- PASS: TestB (0.00s)
+=== RUN   TestB
+--- PASS: TestA (0.00s)
+FAIL
+coverage: 22.9% of statements

third_party/go/BUILD

@@ -637,10 +637,3 @@ 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",
-)