Repository: gotestyourself/gotestsum
Pull request #161 (2020-11-08 22:45:24)

Add TestCase.Time
Also extract some functions from `cmd/tool/slowest` so they can be used in the future for test metrics.


Patch:
diff --git a/cmd/tool/slowest/slowest.go b/cmd/tool/slowest/slowest.go
index 8a0fa88..7455e7b 100644
--- a/cmd/tool/slowest/slowest.go
+++ b/cmd/tool/slowest/slowest.go
@@ -5,10 +5,10 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"sort"
 	"time"
 
 	"github.com/spf13/pflag"
+	"gotest.tools/gotestsum/internal/aggregate"
 	"gotest.tools/gotestsum/log"
 	"gotest.tools/gotestsum/testjson"
 )
@@ -73,7 +73,7 @@ predefined statement, --skip-stmt=testing.Short, which uses this Go statement:
 Alternatively, a custom --skip-stmt may be provided as a string:
 
     skip_stmt='
-        if os.Getenv("TEST_FAST") {
+        if os.GetEnv("TEST_FAST") != "" {
             t.Skip("too slow for TEST_FAST")
         }
     '
@@ -118,7 +118,7 @@ func run(opts *options) error {
 		return fmt.Errorf("failed to scan testjson: %v", err)
 	}
 
-	tcs := slowTestCases(exec, opts.threshold)
+	tcs := aggregate.Slowest(exec, opts.threshold)
 	if opts.skipStatement != "" {
 		skipStmt, err := parseSkipStatement(opts.skipStatement)
 		if err != nil {
@@ -133,69 +133,6 @@ func run(opts *options) error {
 	return nil
 }
 
-// slowTestCases returns a slice of all tests with an elapsed time greater than
-// threshold. The slice is sorted by Elapsed time in descending order (slowest
-// test first).
-//
-// If there are multiple runs of a TestCase, all of them will be represented
-// by a single TestCase with the median elapsed time in the returned slice.
-func slowTestCases(exec *testjson.Execution, threshold time.Duration) []testjson.TestCase {
-	if threshold == 0 {
-		return nil
-	}
-	pkgs := exec.Packages()
-	tests := make([]testjson.TestCase, 0, len(pkgs))
-	for _, pkg := range pkgs {
-		pkgTests := aggregateTestCases(exec.Package(pkg).TestCases())
-		tests = append(tests, pkgTests...)
-	}
-	sort.Slice(tests, func(i, j int) bool {
-		return tests[i].Elapsed > tests[j].Elapsed
-	})
-	end := sort.Search(len(tests), func(i int) bool {
-		return tests[i].Elapsed < threshold
-	})
-	return tests[:end]
-}
-
-// collectTestCases maps all test cases by name, and if there is more than one
-// instance of a TestCase, finds the median elapsed time for all the runs.
-//
-// All cases are assumed to be part of the same package.
-func aggregateTestCases(cases []testjson.TestCase) []testjson.TestCase {
-	if len(cases) < 2 {
-		return cases
-	}
-	pkg := cases[0].Package
-	// nolint: prealloc // size is not predictable
-	m := make(map[testjson.TestName][]time.Duration)
-	for _, tc := range cases {
-		m[tc.Test] = append(m[tc.Test], tc.Elapsed)
-	}
-	result := make([]testjson.TestCase, 0, len(m))
-	for name, timing := range m {
-		result = append(result, testjson.TestCase{
-			Package: pkg,
-			Test:    name,
-			Elapsed: median(timing),
-		})
-	}
-	return result
-}
-
-func median(times []time.Duration) time.Duration {
-	switch len(times) {
-	case 0:
-		return 0
-	case 1:
-		return times[0]
-	}
-	sort.Slice(times, func(i, j int) bool {
-		return times[i] < times[j]
-	})
-	return times[len(times)/2]
-}
-
 func jsonfileReader(v string) (io.ReadCloser, error) {
 	switch v {
 	case "", "-":
diff --git a/internal/aggregate/slowest.go b/internal/aggregate/slowest.go
new file mode 100644
index 0000000..43165d0
--- /dev/null
+++ b/internal/aggregate/slowest.go
@@ -0,0 +1,71 @@
+package aggregate
+
+import (
+	"sort"
+	"time"
+
+	"gotest.tools/gotestsum/testjson"
+)
+
+// Slowest returns a slice of all tests with an elapsed time greater than
+// threshold. The slice is sorted by Elapsed time in descending order (slowest
+// test first).
+//
+// If there are multiple runs of a TestCase, all of them will be represented
+// by a single TestCase with the median elapsed time in the returned slice.
+func Slowest(exec *testjson.Execution, threshold time.Duration) []testjson.TestCase {
+	if threshold == 0 {
+		return nil
+	}
+	pkgs := exec.Packages()
+	tests := make([]testjson.TestCase, 0, len(pkgs))
+	for _, pkg := range pkgs {
+		pkgTests := ByElapsed(exec.Package(pkg).TestCases(), median)
+		tests = append(tests, pkgTests...)
+	}
+	sort.Slice(tests, func(i, j int) bool {
+		return tests[i].Elapsed > tests[j].Elapsed
+	})
+	end := sort.Search(len(tests), func(i int) bool {
+		return tests[i].Elapsed < threshold
+	})
+	return tests[:end]
+}
+
+// ByElapsed maps all test cases by name, and if there is more than one
+// instance of a TestCase, uses fn to select the elapsed time for the group.
+//
+// All cases are assumed to be part of the same package.
+func ByElapsed(cases []testjson.TestCase, fn func(times []time.Duration) time.Duration) []testjson.TestCase {
+	if len(cases) <= 1 {
+		return cases
+	}
+	pkg := cases[0].Package
+	// nolint: prealloc // size is not predictable
+	m := make(map[testjson.TestName][]time.Duration)
+	for _, tc := range cases {
+		m[tc.Test] = append(m[tc.Test], tc.Elapsed)
+	}
+	result := make([]testjson.TestCase, 0, len(m))
+	for name, timing := range m {
+		result = append(result, testjson.TestCase{
+			Package: pkg,
+			Test:    name,
+			Elapsed: fn(timing),
+		})
+	}
+	return result
+}
+
+func median(times []time.Duration) time.Duration {
+	switch len(times) {
+	case 0:
+		return 0
+	case 1:
+		return times[0]
+	}
+	sort.Slice(times, func(i, j int) bool {
+		return times[i] < times[j]
+	})
+	return times[len(times)/2]
+}


Test patch:
diff --git a/cmd/tool/slowest/slowest_test.go b/cmd/tool/slowest/slowest_test.go
index a412a9a..a539b37 100644
--- a/cmd/tool/slowest/slowest_test.go
+++ b/cmd/tool/slowest/slowest_test.go
@@ -2,13 +2,8 @@ package slowest
 
 import (
 	"bytes"
-	"strings"
 	"testing"
-	"time"
 
-	"github.com/google/go-cmp/cmp/cmpopts"
-	"gotest.tools/gotestsum/testjson"
-	"gotest.tools/v3/assert"
 	"gotest.tools/v3/env"
 	"gotest.tools/v3/golden"
 )
@@ -23,55 +18,3 @@ func TestUsage_WithFlagsFromSetupFlags(t *testing.T) {
 
 	golden.Assert(t, buf.String(), "cmd-flags-help-text")
 }
-
-func TestAggregateTestCases(t *testing.T) {
-	cases := []testjson.TestCase{
-		{Test: "TestOne", Package: "pkg", Elapsed: time.Second},
-		{Test: "TestTwo", Package: "pkg", Elapsed: 2 * time.Second},
-		{Test: "TestOne", Package: "pkg", Elapsed: 3 * time.Second},
-		{Test: "TestTwo", Package: "pkg", Elapsed: 4 * time.Second},
-		{Test: "TestOne", Package: "pkg", Elapsed: 5 * time.Second},
-		{Test: "TestTwo", Package: "pkg", Elapsed: 6 * time.Second},
-	}
-	actual := aggregateTestCases(cases)
-	expected := []testjson.TestCase{
-		{Test: "TestOne", Package: "pkg", Elapsed: 3 * time.Second},
-		{Test: "TestTwo", Package: "pkg", Elapsed: 4 * time.Second},
-	}
-	assert.DeepEqual(t, actual, expected,
-		cmpopts.SortSlices(func(x, y testjson.TestCase) bool {
-			return strings.Compare(x.Test.Name(), y.Test.Name()) == -1
-		}),
-		cmpopts.IgnoreUnexported(testjson.TestCase{}))
-}
-
-func TestMedian(t *testing.T) {
-	var testcases = []struct {
-		name     string
-		times    []time.Duration
-		expected time.Duration
-	}{
-		{
-			name:     "one item slice",
-			times:    []time.Duration{time.Minute},
-			expected: time.Minute,
-		},
-		{
-			name:     "odd number of items",
-			times:    []time.Duration{time.Millisecond, time.Hour, time.Second},
-			expected: time.Second,
-		},
-		{
-			name:     "even number of items",
-			times:    []time.Duration{time.Second, time.Millisecond, time.Microsecond, time.Hour},
-			expected: time.Second,
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			actual := median(tc.times)
-			assert.Equal(t, actual, tc.expected)
-		})
-	}
-}
diff --git a/cmd/tool/slowest/testdata/cmd-flags-help-text b/cmd/tool/slowest/testdata/cmd-flags-help-text
index 8dcbff5..38bf4cd 100644
--- a/cmd/tool/slowest/testdata/cmd-flags-help-text
+++ b/cmd/tool/slowest/testdata/cmd-flags-help-text
@@ -26,7 +26,7 @@ predefined statement, --skip-stmt=testing.Short, which uses this Go statement:
 Alternatively, a custom --skip-stmt may be provided as a string:
 
     skip_stmt='
-        if os.Getenv("TEST_FAST") {
+        if os.GetEnv("TEST_FAST") != "" {
             t.Skip("too slow for TEST_FAST")
         }
     '
diff --git a/internal/aggregate/slowest_test.go b/internal/aggregate/slowest_test.go
new file mode 100644
index 0000000..010007b
--- /dev/null
+++ b/internal/aggregate/slowest_test.go
@@ -0,0 +1,63 @@
+package aggregate
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"gotest.tools/gotestsum/testjson"
+	"gotest.tools/v3/assert"
+)
+
+func TestByElapsed_WithMedian(t *testing.T) {
+	cases := []testjson.TestCase{
+		{Test: "TestOne", Package: "pkg", Elapsed: time.Second},
+		{Test: "TestTwo", Package: "pkg", Elapsed: 2 * time.Second},
+		{Test: "TestOne", Package: "pkg", Elapsed: 3 * time.Second},
+		{Test: "TestTwo", Package: "pkg", Elapsed: 4 * time.Second},
+		{Test: "TestOne", Package: "pkg", Elapsed: 5 * time.Second},
+		{Test: "TestTwo", Package: "pkg", Elapsed: 6 * time.Second},
+	}
+	actual := ByElapsed(cases, median)
+	expected := []testjson.TestCase{
+		{Test: "TestOne", Package: "pkg", Elapsed: 3 * time.Second},
+		{Test: "TestTwo", Package: "pkg", Elapsed: 4 * time.Second},
+	}
+	assert.DeepEqual(t, actual, expected,
+		cmpopts.SortSlices(func(x, y testjson.TestCase) bool {
+			return strings.Compare(x.Test.Name(), y.Test.Name()) == -1
+		}),
+		cmpopts.IgnoreUnexported(testjson.TestCase{}))
+}
+
+func TestMedian(t *testing.T) {
+	var testcases = []struct {
+		name     string
+		times    []time.Duration
+		expected time.Duration
+	}{
+		{
+			name:     "one item slice",
+			times:    []time.Duration{time.Minute},
+			expected: time.Minute,
+		},
+		{
+			name:     "odd number of items",
+			times:    []time.Duration{time.Millisecond, time.Hour, time.Second},
+			expected: time.Second,
+		},
+		{
+			name:     "even number of items",
+			times:    []time.Duration{time.Second, time.Millisecond, time.Microsecond, time.Hour},
+			expected: time.Second,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := median(tc.times)
+			assert.Equal(t, actual, tc.expected)
+		})
+	}
+}
diff --git a/testjson/execution.go b/testjson/execution.go
index 5830e2e..577c664 100644
--- a/testjson/execution.go
+++ b/testjson/execution.go
@@ -250,6 +250,8 @@ type TestCase struct {
 	// hasSubTestFailed is true when a subtest of this TestCase has failed. It is
 	// used to find root TestCases which have no failing subtests.
 	hasSubTestFailed bool
+	// Time when the test was run.
+	Time time.Time
 }
 
 func newPackage() *Package {
@@ -307,6 +309,7 @@ func (p *Package) newTestCaseFromEvent(event TestEvent) TestCase {
 		Test:    TestName(event.Test),
 		ID:      p.Total,
 		RunID:   event.RunID,
+		Time:    event.Time,
 	}
 }
 
diff --git a/testjson/execution_test.go b/testjson/execution_test.go
index 0d5bf38..6bdd603 100644
--- a/testjson/execution_test.go
+++ b/testjson/execution_test.go
@@ -90,3 +90,20 @@ func (s *handlerFails) Event(_ TestEvent, _ *Execution) error {
 func (s *handlerFails) Err(_ string) error {
 	return nil
 }
+
+func TestParseEvent(t *testing.T) {
+	// nolint: lll
+	raw := `{"Time":"2018-03-22T22:33:35.168308334Z","Action":"output","Package":"example.com/good","Test": "TestOk","Output":"PASS\n"}`
+	event, err := parseEvent([]byte(raw))
+	assert.NilError(t, err)
+	expected := TestEvent{
+		Time:    time.Date(2018, 3, 22, 22, 33, 35, 168308334, time.UTC),
+		Action:  "output",
+		Package: "example.com/good",
+		Test:    "TestOk",
+		Output:  "PASS\n",
+		raw:     []byte(raw),
+	}
+	cmpTestEvent := cmp.AllowUnexported(TestEvent{})
+	assert.DeepEqual(t, event, expected, cmpTestEvent)
+}