gtr,junit: move creation of JUnit testsuites from gtr to junit

Package gtr shouldn't need to know about the existence of different
output formats like junit.

go-junit-report.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/jstemmer/go-junit-report/v2/pkg/gtr"
+	"github.com/jstemmer/go-junit-report/v2/pkg/junit"
 	"github.com/jstemmer/go-junit-report/v2/pkg/parser/gotest"
 )
 
@@ -77,7 +78,7 @@ func main() {
 	report := gtr.FromEvents(events, *packageName)
 
 	hostname, _ := os.Hostname() // ignore error
-	testsuites := gtr.JUnit(report, hostname, time.Now())
+	testsuites := junit.CreateFromReport(report, hostname, time.Now())
 
 	var out io.Writer = os.Stdout
 	if *output != "" {

go-junit-report_test.go

@@ -204,7 +204,8 @@ func testReport(input, reportFile, packageName string, t *testing.T) {
 	}
 
 	testTime := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)
-	actual := gtr.JUnit(gtr.FromEvents(events, packageName), "hostname", testTime)
+	report := gtr.FromEvents(events, packageName)
+	actual := junit.CreateFromReport(report, "hostname", testTime)
 
 	expectedXML, err := loadTestReport(reportFile, "")
 	if err != nil {

pkg/gtr/gtr.go

@@ -6,8 +6,6 @@ import (
 	"fmt"
 	"strings"
 	"time"
-
-	"github.com/jstemmer/go-junit-report/v2/pkg/junit"
 )
 
 type Report struct {
@@ -100,119 +98,13 @@ func FromEvents(events []Event, packageName string) Report {
 	return report.Build()
 }
 
-// JUnit converts the given report to a collection of JUnit Testsuites.
-func JUnit(report Report, hostname string, now time.Time) junit.Testsuites {
-	timestamp := now.Format(time.RFC3339)
-
-	var suites junit.Testsuites
-	for _, pkg := range report.Packages {
-		var duration time.Duration
-		suite := junit.Testsuite{
-			Name:      pkg.Name,
-			Timestamp: timestamp,
-			Hostname:  hostname,
-		}
-
-		if len(pkg.Output) > 0 {
-			suite.SystemOut = &junit.Output{Data: formatOutput(pkg.Output, 0)}
-		}
-
-		if pkg.Coverage > 0 {
-			suite.AddProperty("coverage.statements.pct", fmt.Sprintf("%.2f", pkg.Coverage))
-		}
-
-		for _, test := range pkg.Tests {
-			duration += test.Duration
-
-			tc := junit.Testcase{
-				Classname: pkg.Name,
-				Name:      test.Name,
-				Time:      junit.FormatDuration(test.Duration),
-			}
-
-			if test.Result == Fail {
-				tc.Failure = &junit.Result{
-					Message: "Failed",
-					Data:    formatOutput(test.Output, test.Level),
-				}
-			} else if test.Result == Skip {
-				tc.Skipped = &junit.Result{
-					Message: formatOutput(test.Output, test.Level),
-				}
-			} else if test.Result == Unknown {
-				tc.Error = &junit.Result{
-					Message: "No test result found",
-					Data:    formatOutput(test.Output, test.Level),
-				}
-			}
-
-			suite.AddTestcase(tc)
-		}
-
-		for _, bm := range mergeBenchmarks(pkg.Benchmarks) {
-			tc := junit.Testcase{
-				Classname: pkg.Name,
-				Name:      bm.Name,
-				Time:      junit.FormatBenchmarkTime(time.Duration(bm.NsPerOp)),
-			}
-
-			if bm.Result == Fail {
-				tc.Failure = &junit.Result{
-					Message: "Failed",
-				}
-			}
-
-			suite.AddTestcase(tc)
-		}
-
-		// JUnit doesn't have a good way of dealing with build or runtime
-		// errors that happen before a test has started, so we create a single
-		// failing test that contains the build error details.
-		if pkg.BuildError.Name != "" {
-			tc := junit.Testcase{
-				Classname: pkg.BuildError.Name,
-				Name:      pkg.BuildError.Cause,
-				Time:      junit.FormatDuration(0),
-				Error: &junit.Result{
-					Message: "Build error",
-					Data:    strings.Join(pkg.BuildError.Output, "\n"),
-				},
-			}
-			suite.AddTestcase(tc)
-		}
-
-		if pkg.RunError.Name != "" {
-			tc := junit.Testcase{
-				Classname: pkg.RunError.Name,
-				Name:      "Failure",
-				Time:      junit.FormatDuration(0),
-				Error: &junit.Result{
-					Message: "Run error",
-					Data:    strings.Join(pkg.RunError.Output, "\n"),
-				},
-			}
-			suite.AddTestcase(tc)
-		}
-
-		if (pkg.Duration) == 0 {
-			suite.Time = junit.FormatDuration(duration)
-		} else {
-			suite.Time = junit.FormatDuration(pkg.Duration)
-		}
-		suites.AddSuite(suite)
-	}
-	return suites
-}
-
-func formatOutput(output []string, level int) string {
-	var lines []string
-	for _, line := range output {
-		lines = append(lines, trimOutputPrefix(line, level))
-	}
-	return strings.Join(lines, "\n")
-}
-
-func trimOutputPrefix(line string, level int) string {
+// TrimPrefixSpaces trims the leading whitespace of the given line using the
+// indentation level of the test. Printing logs in a Go test is typically
+// prepended by blocks of 4 spaces to align it with the rest of the test
+// output. TrimPrefixSpaces intends to only trim the whitespace added by the Go
+// test command, without inadvertently trimming whitespace added by the test
+// author.
+func TrimPrefixSpaces(line string, indent int) string {
 	// We only want to trim the whitespace prefix if it was part of the test
 	// output. Test output is usually prefixed by a series of 4-space indents,
 	// so we'll check for that to decide whether this output was likely to be
@@ -221,37 +113,9 @@ func trimOutputPrefix(line string, level int) string {
 	if prefixLen%4 == 0 {
 		// Use the subtest level to trim a consistenly sized prefix from the
 		// output lines.
-		for i := 0; i <= level; i++ {
+		for i := 0; i <= indent; i++ {
 			line = strings.TrimPrefix(line, "    ")
 		}
 	}
 	return strings.TrimPrefix(line, "\t")
 }
-
-func mergeBenchmarks(benchmarks []Benchmark) []Benchmark {
-	var merged []Benchmark
-
-	benchmap := make(map[string][]Benchmark)
-	for _, bm := range benchmarks {
-		if _, ok := benchmap[bm.Name]; !ok {
-			merged = append(merged, Benchmark{Name: bm.Name})
-		}
-		benchmap[bm.Name] = append(benchmap[bm.Name], bm)
-	}
-
-	for i, bm := range merged {
-		for _, b := range benchmap[bm.Name] {
-			bm.NsPerOp += b.NsPerOp
-			bm.MBPerSec += b.MBPerSec
-			bm.BytesPerOp += b.BytesPerOp
-			bm.AllocsPerOp += b.AllocsPerOp
-		}
-		n := len(benchmap[bm.Name])
-		merged[i].NsPerOp = bm.NsPerOp / float64(n)
-		merged[i].MBPerSec = bm.MBPerSec / float64(n)
-		merged[i].BytesPerOp = bm.BytesPerOp / int64(n)
-		merged[i].AllocsPerOp = bm.AllocsPerOp / int64(n)
-	}
-
-	return merged
-}

pkg/junit/junit.go

@@ -5,7 +5,10 @@ package junit
 import (
 	"encoding/xml"
 	"fmt"
+	"strings"
 	"time"
+
+	"github.com/jstemmer/go-junit-report/v2/pkg/gtr"
 )
 
 // Testsuites is a collection of JUnit testsuites.
@@ -123,14 +126,156 @@ type Output struct {
 	Data string `xml:",cdata"`
 }
 
-// FormatDuration returns the JUnit string representation of the given
+// CreateFromReport creates a JUnit representation of the given gtr.Report.
+func CreateFromReport(report gtr.Report, hostname string, timestamp time.Time) Testsuites {
+	ts := timestamp.Format(time.RFC3339)
+
+	var suites Testsuites
+	for _, pkg := range report.Packages {
+		var duration time.Duration
+		suite := Testsuite{
+			Name:      pkg.Name,
+			Timestamp: ts,
+			Hostname:  hostname,
+		}
+
+		if len(pkg.Output) > 0 {
+			suite.SystemOut = &Output{Data: formatOutput(pkg.Output, 0)}
+		}
+
+		if pkg.Coverage > 0 {
+			suite.AddProperty("coverage.statements.pct", fmt.Sprintf("%.2f", pkg.Coverage))
+		}
+
+		for _, test := range pkg.Tests {
+			duration += test.Duration
+
+			tc := Testcase{
+				Classname: pkg.Name,
+				Name:      test.Name,
+				Time:      formatDuration(test.Duration),
+			}
+
+			if test.Result == gtr.Fail {
+				tc.Failure = &Result{
+					Message: "Failed",
+					Data:    formatOutput(test.Output, test.Level),
+				}
+			} else if test.Result == gtr.Skip {
+				tc.Skipped = &Result{
+					Message: formatOutput(test.Output, test.Level),
+				}
+			} else if test.Result == gtr.Unknown {
+				tc.Error = &Result{
+					Message: "No test result found",
+					Data:    formatOutput(test.Output, test.Level),
+				}
+			}
+
+			suite.AddTestcase(tc)
+		}
+
+		for _, bm := range groupBenchmarksByName(pkg.Benchmarks) {
+			tc := Testcase{
+				Classname: pkg.Name,
+				Name:      bm.Name,
+				Time:      formatBenchmarkTime(time.Duration(bm.NsPerOp)),
+			}
+
+			if bm.Result == gtr.Fail {
+				tc.Failure = &Result{
+					Message: "Failed",
+				}
+			}
+
+			suite.AddTestcase(tc)
+		}
+
+		// JUnit doesn't have a good way of dealing with build or runtime
+		// errors that happen before a test has started, so we create a single
+		// failing test that contains the build error details.
+		if pkg.BuildError.Name != "" {
+			tc := Testcase{
+				Classname: pkg.BuildError.Name,
+				Name:      pkg.BuildError.Cause,
+				Time:      formatDuration(0),
+				Error: &Result{
+					Message: "Build error",
+					Data:    strings.Join(pkg.BuildError.Output, "\n"),
+				},
+			}
+			suite.AddTestcase(tc)
+		}
+
+		if pkg.RunError.Name != "" {
+			tc := Testcase{
+				Classname: pkg.RunError.Name,
+				Name:      "Failure",
+				Time:      formatDuration(0),
+				Error: &Result{
+					Message: "Run error",
+					Data:    strings.Join(pkg.RunError.Output, "\n"),
+				},
+			}
+			suite.AddTestcase(tc)
+		}
+
+		if (pkg.Duration) == 0 {
+			suite.Time = formatDuration(duration)
+		} else {
+			suite.Time = formatDuration(pkg.Duration)
+		}
+		suites.AddSuite(suite)
+	}
+	return suites
+}
+
+// formatDuration returns the JUnit string representation of the given
 // duration.
-func FormatDuration(d time.Duration) string {
+func formatDuration(d time.Duration) string {
 	return fmt.Sprintf("%.3f", d.Seconds())
 }
 
-// FormatBenchmarkTime returns the JUnit string representation of the given
+// formatBenchmarkTime returns the JUnit string representation of the given
 // benchmark time.
-func FormatBenchmarkTime(d time.Duration) string {
+func formatBenchmarkTime(d time.Duration) string {
 	return fmt.Sprintf("%.9f", d.Seconds())
 }
+
+// formatOutput trims the test whitespace prefix from each line and joins all
+// the lines.
+func formatOutput(output []string, indent int) string {
+	var lines []string
+	for _, line := range output {
+		lines = append(lines, gtr.TrimPrefixSpaces(line, indent))
+	}
+	return strings.Join(lines, "\n")
+}
+
+func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark {
+	var grouped []gtr.Benchmark
+
+	benchmap := make(map[string][]gtr.Benchmark)
+	for _, bm := range benchmarks {
+		if _, ok := benchmap[bm.Name]; !ok {
+			grouped = append(grouped, gtr.Benchmark{Name: bm.Name})
+		}
+		benchmap[bm.Name] = append(benchmap[bm.Name], bm)
+	}
+
+	for i, bm := range grouped {
+		for _, b := range benchmap[bm.Name] {
+			bm.NsPerOp += b.NsPerOp
+			bm.MBPerSec += b.MBPerSec
+			bm.BytesPerOp += b.BytesPerOp
+			bm.AllocsPerOp += b.AllocsPerOp
+		}
+		n := len(benchmap[bm.Name])
+		grouped[i].NsPerOp = bm.NsPerOp / float64(n)
+		grouped[i].MBPerSec = bm.MBPerSec / float64(n)
+		grouped[i].BytesPerOp = bm.BytesPerOp / int64(n)
+		grouped[i].AllocsPerOp = bm.AllocsPerOp / int64(n)
+	}
+
+	return grouped
+}