Addressing code review comments.

Mainly:
* Moving the averaging/merging of benchmarks from the parser to the formatter package
* Tightening up the regex so it only captures the numeric values (no more of trimming spaces and the ns/op)
* Deleting the writing up in xml file the benchmark memory sections of B/op and Allocs/op

Also added a test case for parseNanoseconds().
This commit is contained in:
Brittany Walentin 2018-05-25 10:59:00 -07:00
parent b2f467b67c
commit 260b47cabe
9 changed files with 185 additions and 128 deletions

View File

@ -37,9 +37,13 @@ go test -v 2>&1 | go-junit-report > report.xml
Note that it also can parse benchmark output with `-bench` flag:
```bash
go test -bench . -benchmem -count 100
go test -v -bench . 2>&1 | ./go-junit-report > report.xml
```
will return the average mean benchmark time as the test case time.
or using the optional -count parameter:
```bash
go test -v -bench . -count 5 2>&1 | ./go-junit-report > report.xml
```
[travis-badge]: https://travis-ci.org/jstemmer/go-junit-report.svg
[travis-link]: https://travis-ci.org/jstemmer/go-junit-report

View File

@ -6,7 +6,6 @@ import (
"fmt"
"io"
"runtime"
"strconv"
"strings"
"time"
@ -39,9 +38,6 @@ type JUnitTestCase struct {
Time string `xml:"time,attr"`
SkipMessage *JUnitSkipMessage `xml:"skipped,omitempty"`
Failure *JUnitFailure `xml:"failure,omitempty"`
// for benchmarks
Bytes string `xml:"bytes,attr,omitempty"`
Allocs string `xml:"allocs,attr,omitempty"`
}
// JUnitSkipMessage contains the reason why a testcase was skipped.
@ -69,17 +65,8 @@ func JUnitReportXML(report *parser.Report, noXMLHeader bool, goVersion string, w
// convert Report to JUnit test suites
for _, pkg := range report.Packages {
var tests int
if len(pkg.Tests) >= 1 && len(pkg.Benchmarks) >= 1 {
tests = len(pkg.Tests) + len(pkg.Benchmarks)
} else if len(pkg.Benchmarks) >= 1 {
tests = len(pkg.Benchmarks)
} else {
tests = len(pkg.Tests)
}
ts := JUnitTestSuite{
Tests: tests,
Tests: len(pkg.Tests) + len(pkg.Benchmarks),
Failures: 0,
Time: formatTime(pkg.Duration),
Name: pkg.Name,
@ -128,7 +115,7 @@ func JUnitReportXML(report *parser.Report, noXMLHeader bool, goVersion string, w
}
// individual benchmarks
for _, benchmark := range pkg.Benchmarks {
for _, benchmark := range mergeBenchmarks(pkg.Benchmarks) {
benchmarkCase := JUnitTestCase{
Classname: classname,
Name: benchmark.Name,
@ -136,13 +123,6 @@ func JUnitReportXML(report *parser.Report, noXMLHeader bool, goVersion string, w
Failure: nil,
}
if benchmark.Bytes != 0 {
benchmarkCase.Bytes = strconv.Itoa(benchmark.Bytes)
}
if benchmark.Allocs != 0 {
benchmarkCase.Allocs = strconv.Itoa(benchmark.Allocs)
}
ts.TestCases = append(ts.TestCases, benchmarkCase)
}
@ -168,6 +148,44 @@ func JUnitReportXML(report *parser.Report, noXMLHeader bool, goVersion string, w
return nil
}
func mergeBenchmarks(benchmarks []*parser.Benchmark) []*parser.Benchmark {
// calculate the cumulative moving average CMA for each benchmark.
// CMA(n + 1) = val(n+1) + n*CMA/(n+1)
alloc := "Allocs"
bytes := "Bytes"
dur := "Duration"
n := 1
var merged []*parser.Benchmark
averages := make(map[string] /*bench name*/ map[string] /* type */ int)
for _, b := range benchmarks {
if avg, found := averages[b.Name]; found {
// calculate CMAs
averages[b.Name][alloc] = (b.Allocs + n*averages[b.Name][alloc]) / (n + 1)
averages[b.Name][bytes] = (b.Bytes + n*avg[bytes]) / (n + 1)
averages[b.Name][dur] = (int(b.Duration.Nanoseconds()) + n*avg[dur]) / (n + 1)
n++
continue
}
n = 1 // reset duplicate counter
merged = append(merged, &parser.Benchmark{Name: b.Name})
averages[b.Name] = make(map[string]int)
averages[b.Name][alloc] = b.Allocs
averages[b.Name][bytes] = b.Bytes
averages[b.Name][dur] = int(b.Duration.Nanoseconds())
}
// fill out benchmarks
for i := range merged {
avgVals := averages[merged[i].Name]
merged[i].Allocs = avgVals[alloc]
merged[i].Bytes = avgVals[bytes]
merged[i].Duration = time.Duration(avgVals[dur])
}
return merged
}
func formatTime(d time.Duration) string {
return fmt.Sprintf("%.3f", d.Seconds())
}

View File

@ -957,14 +957,12 @@ var testCases = []TestCase{
Duration: 52568 * time.Nanosecond,
Bytes: 24879,
Allocs: 494,
Output: []string{},
},
{
Name: "BenchmarkIpsHistoryLookup",
Duration: 15208 * time.Nanosecond,
Bytes: 7369,
Allocs: 143,
Output: []string{},
},
},
},
@ -1016,14 +1014,12 @@ var testCases = []TestCase{
Duration: 2611 * time.Nanosecond,
Bytes: 1110,
Allocs: 16,
Output: []string{},
},
{
Name: "BenchmarkNext",
Duration: 100 * time.Nanosecond,
Bytes: 100,
Allocs: 1,
Output: []string{},
},
},
},
@ -1042,19 +1038,63 @@ var testCases = []TestCase{
Benchmarks: []*parser.Benchmark{
{
Name: "BenchmarkNew",
Duration: 352 * time.Nanosecond,
Duration: 350 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
Count: 5,
Output: []string{},
},
{
Name: "BenchmarkNew",
Duration: 357 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 354 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 358 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 345 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkFew",
Duration: 100 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 105 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
Count: 5,
Output: []string{},
},
},
},
@ -1076,53 +1116,92 @@ var testCases = []TestCase{
Duration: 0,
Time: 0,
Result: parser.PASS,
Output: []string{},
},
{
Name: "TestRepeat",
Duration: 0,
Time: 0,
Result: parser.PASS,
Output: []string{},
},
{
Name: "TestRepeat",
Duration: 0,
Time: 0,
Result: parser.PASS,
Output: []string{},
},
{
Name: "TestRepeat",
Duration: 0,
Time: 0,
Result: parser.PASS,
Output: []string{},
},
{
Name: "TestRepeat",
Duration: 0,
Time: 0,
Result: parser.PASS,
Output: []string{},
},
},
Benchmarks: []*parser.Benchmark{
{
Name: "BenchmarkNew",
Duration: 352 * time.Nanosecond,
Duration: 350 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
Count: 5,
Output: []string{},
},
{
Name: "BenchmarkNew",
Duration: 357 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 354 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 358 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkNew",
Duration: 345 * time.Nanosecond,
Bytes: 80,
Allocs: 3,
},
{
Name: "BenchmarkFew",
Duration: 100 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 105 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
},
{
Name: "BenchmarkFew",
Duration: 102 * time.Nanosecond,
Bytes: 20,
Allocs: 1,
Count: 5,
Output: []string{},
},
},
},
@ -1142,17 +1221,14 @@ var testCases = []TestCase{
{
Name: "BenchmarkItsy",
Duration: 45 * time.Nanosecond,
Output: []string{},
},
{
Name: "BenchmarkTeeny",
Duration: 2 * time.Nanosecond,
Output: []string{},
},
{
Name: "BenchmarkWeeny",
Duration: 0 * time.Second,
Output: []string{},
},
},
},
@ -1257,12 +1333,6 @@ func TestParser(t *testing.T) {
if benchmark.Allocs != expBenchmark.Allocs {
t.Errorf("benchmark.Allocs == %d, want %d", benchmark.Allocs, expBenchmark.Allocs)
}
benchmarkOutput := strings.Join(benchmark.Output, "\n")
expBenchmarkOutput := strings.Join(expBenchmark.Output, "\n")
if benchmarkOutput != expBenchmarkOutput {
t.Errorf("Benchmark.Output (%s) ==\n%s\n, want\n%s", benchmark.Name, benchmarkOutput, expBenchmarkOutput)
}
}
if pkg.CoveragePct != expPkg.CoveragePct {

View File

@ -55,9 +55,6 @@ type Benchmark struct {
Bytes int
// number of allocs/op
Allocs int
// number of times this benchmark has been seen (for averaging).
Count int
Output []string
}
var (
@ -65,7 +62,7 @@ var (
regexCoverage = regexp.MustCompile(`^coverage:\s+(\d+\.\d+)%\s+of\s+statements(?:\sin\s.+)?$`)
regexResult = regexp.MustCompile(`^(ok|FAIL)\s+([^ ]+)\s+(?:(\d+\.\d+)s|\(cached\)|(\[\w+ failed]))(?:\s+coverage:\s+(\d+\.\d+)%\sof\sstatements(?:\sin\s.+)?)?$`)
// regexBenchmark captures 3-5 groups: benchmark name, number of times ran, ns/op (with or without decimal), B/op (optional), and allocs/op (optional).
regexBenchmark = regexp.MustCompile(`^(Benchmark\w+)-\d\s+(\d+)\s+((\d+|\d+\.\d+)\sns/op)(\s+\d+\sB/op)?(\s+\d+\sallocs/op)?`)
regexBenchmark = regexp.MustCompile(`^(Benchmark[^ ]+)-\d\s+(\d+)\s+(\d+|\d+\.\d+)\sns/op(?:\s+(\d+)\sB/op)?(?:\s+(\d+)\sallocs/op)?`)
regexOutput = regexp.MustCompile(`( )*\t(.*)`)
regexSummary = regexp.MustCompile(`^(PASS|FAIL|SKIP)$`)
)
@ -128,60 +125,16 @@ func Parse(r io.Reader, pkgName string) (*Report, error) {
// clear the current build package, so output lines won't be added to that build
capturedPackage = ""
seenSummary = false
} else if strings.HasPrefix(line, "Benchmark") {
// parse benchmarking info
matches := regexBenchmark.FindStringSubmatch(line)
if len(matches) < 1 {
continue
}
var name string
var duration time.Duration
var bytes int
var allocs int
for _, field := range matches[1:] {
field = strings.TrimSpace(field)
if strings.HasPrefix(field, "Benchmark") {
name = field
}
if strings.HasSuffix(field, " ns/op") {
durString := strings.TrimSuffix(field, " ns/op")
duration = parseNanoseconds(durString)
}
if strings.HasSuffix(field, " B/op") {
b, _ := strconv.Atoi(strings.TrimSuffix(field, " B/op"))
bytes = b
}
if strings.HasSuffix(field, " allocs/op") {
a, _ := strconv.Atoi(strings.TrimSuffix(field, " allocs/op"))
allocs = a
}
}
var duplicate bool
// check if duplicate benchmark
for _, bench := range benchmarks {
if bench.Name == name {
duplicate = true
bench.Count++
bench.Duration += duration
bench.Bytes += bytes
bench.Allocs += allocs
}
}
if len(benchmarks) < 1 || duplicate == false {
// the first time this benchmark has been seen
benchmarks = append(benchmarks, &Benchmark{
Name: name,
Duration: duration,
Bytes: bytes,
Allocs: allocs,
Count: 1,
},
)
}
} else if matches := regexBenchmark.FindStringSubmatch(line); len(matches) == 6 {
bytes, _ := strconv.Atoi(matches[4])
allocs, _ := strconv.Atoi(matches[5])
benchmarks = append(benchmarks, &Benchmark{
Name: matches[1],
Duration: parseNanoseconds(matches[3]),
Bytes: bytes,
Allocs: allocs,
})
} else if strings.HasPrefix(line, "=== PAUSE ") {
continue
} else if strings.HasPrefix(line, "=== CONT ") {
@ -212,15 +165,6 @@ func Parse(r io.Reader, pkgName string) (*Report, error) {
// all tests in this package are finished
for _, bench := range benchmarks {
if bench.Count > 1 {
bench.Allocs = bench.Allocs / bench.Count
bench.Bytes = bench.Bytes / bench.Count
newDuration := bench.Duration / time.Duration(bench.Count)
bench.Duration = newDuration
}
}
report.Packages = append(report.Packages, Package{
Name: matches[2],
Duration: parseSeconds(matches[3]),

View File

@ -24,3 +24,24 @@ func TestParseSeconds(t *testing.T) {
}
}
}
func TestParseNanoseconds(t *testing.T) {
tests := []struct {
in string
d time.Duration
}{
{"", 0},
{"0.1", 0 * time.Nanosecond},
{"0.9", 0 * time.Nanosecond},
{"4", 4 * time.Nanosecond},
{"5000", 5 * time.Microsecond},
{"2000003", 2*time.Millisecond + 3*time.Nanosecond},
}
for _, test := range tests {
d := parseNanoseconds(test.in)
if d != test.d {
t.Errorf("parseSeconds(%q) == %v, want %v\n", test.in, d, test.d)
}
}
}

View File

@ -4,7 +4,7 @@
<properties>
<property name="go.version" value="1.0"></property>
</properties>
<testcase classname="one" name="BenchmarkIpsHistoryInsert" time="0.000052568" bytes="24879" allocs="494"></testcase>
<testcase classname="one" name="BenchmarkIpsHistoryLookup" time="0.000015208" bytes="7369" allocs="143"></testcase>
<testcase classname="one" name="BenchmarkIpsHistoryInsert" time="0.000052568"></testcase>
<testcase classname="one" name="BenchmarkIpsHistoryLookup" time="0.000015208"></testcase>
</testsuite>
</testsuites>

View File

@ -8,7 +8,7 @@
<testcase classname="baz" name="TestNew/no" time="0.000"></testcase>
<testcase classname="baz" name="TestNew/normal" time="0.000"></testcase>
<testcase classname="baz" name="TestWriteThis" time="0.000"></testcase>
<testcase classname="baz" name="BenchmarkDeepMerge" time="0.000002611" bytes="1110" allocs="16"></testcase>
<testcase classname="baz" name="BenchmarkNext" time="0.000000100" bytes="100" allocs="1"></testcase>
<testcase classname="baz" name="BenchmarkDeepMerge" time="0.000002611"></testcase>
<testcase classname="baz" name="BenchmarkNext" time="0.000000100"></testcase>
</testsuite>
</testsuites>

View File

@ -1,10 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
<testsuite tests="2" failures="0" time="14.211" name="pkg/count">
<testsuite tests="10" failures="0" time="14.211" name="pkg/count">
<properties>
<property name="go.version" value="1.0"></property>
</properties>
<testcase classname="count" name="BenchmarkNew" time="0.000000352" bytes="80" allocs="3"></testcase>
<testcase classname="count" name="BenchmarkFew" time="0.000000102" bytes="20" allocs="1"></testcase>
<testcase classname="count" name="BenchmarkNew" time="0.000000352"></testcase>
<testcase classname="count" name="BenchmarkFew" time="0.000000102"></testcase>
</testsuite>
</testsuites>

View File

@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
<testsuite tests="7" failures="0" time="14.211" name="multiple/repeating">
<testsuite tests="15" failures="0" time="14.211" name="multiple/repeating">
<properties>
<property name="go.version" value="1.0"></property>
</properties>
@ -9,7 +9,7 @@
<testcase classname="repeating" name="TestRepeat" time="0.000"></testcase>
<testcase classname="repeating" name="TestRepeat" time="0.000"></testcase>
<testcase classname="repeating" name="TestRepeat" time="0.000"></testcase>
<testcase classname="repeating" name="BenchmarkNew" time="0.000000352" bytes="80" allocs="3"></testcase>
<testcase classname="repeating" name="BenchmarkFew" time="0.000000102" bytes="20" allocs="1"></testcase>
<testcase classname="repeating" name="BenchmarkNew" time="0.000000352"></testcase>
<testcase classname="repeating" name="BenchmarkFew" time="0.000000102"></testcase>
</testsuite>
</testsuites>