Merge pull request #133 from maxatome/trace++

fix(trace): ignore internal testing/go-testdeep calls in error traces

Author: maxatome

internal/trace/stack.go

@@ -0,0 +1,57 @@
+// Copyright (c) 2021, Maxime Soulé
+// All rights reserved.
+//
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree.
+
+package trace
+
+import (
+	"strings"
+)
+
+// Level is a level when retrieving a stack trace.
+type Level struct {
+	Package  string
+	Func     string
+	FileLine string
+}
+
+// Stack is a simple stack trace.
+type Stack []Level
+
+// Match returns true if the ith level of s matches pkg (if not empty)
+// and any function in anyFunc.
+//
+// If anyFunc is empty, only the package is tested.
+//
+// If a function in anyFunc ends with "*", only the prefix is checked.
+func (s Stack) Match(i int, pkg string, anyFunc ...string) bool {
+	if i < 0 {
+		i = len(s) + i
+	}
+	if i < 0 || i >= len(s) {
+		return false
+	}
+
+	level := s[i]
+
+	if pkg != "" && level.Package != pkg {
+		return false
+	}
+
+	if len(anyFunc) == 0 {
+		return true
+	}
+
+	for _, fn := range anyFunc {
+		if strings.HasSuffix(fn, "*") {
+			if strings.HasPrefix(level.Func, fn[:len(fn)-1]) {
+				return true
+			}
+		} else if level.Func == fn {
+			return true
+		}
+	}
+	return false
+}

internal/trace/stack_test.go

@@ -0,0 +1,36 @@
+// Copyright (c) 2021, Maxime Soulé
+// All rights reserved.
+//
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree.
+
+package trace_test
+
+import (
+	"testing"
+
+	"github.com/maxatome/go-testdeep/internal/test"
+	"github.com/maxatome/go-testdeep/internal/trace"
+)
+
+func TestStack(t *testing.T) {
+	s := trace.Stack{
+		{Package: "A", Func: "Aaa.func1"},
+		{Package: "A", Func: "Aaa.func2"},
+		{Package: "B", Func: "Bbb"},
+		{Package: "C", Func: "Ccc"},
+	}
+
+	test.IsFalse(t, s.Match(100, "A"))
+	test.IsFalse(t, s.Match(-100, "A"))
+
+	test.IsFalse(t, s.Match(3, "B"))
+	test.IsFalse(t, s.Match(-1, "B"))
+
+	test.IsTrue(t, s.Match(3, "C"))
+	test.IsTrue(t, s.Match(-1, "C"))
+
+	test.IsFalse(t, s.Match(1, "A", "Aaa.func3", "Aaa.func1"))
+	test.IsTrue(t, s.Match(1, "A", "Aaa.func3", "Aaa.func2"))
+	test.IsTrue(t, s.Match(1, "A", "Aaa.func3", "Aaa.func*"))
+}

internal/trace/trace.go

@@ -21,13 +21,6 @@ var (
 	goModDir  string
 )
 
-// Level represents a level when retrieving a trace.
-type Level struct {
-	Package  string
-	Func     string
-	FileLine string
-}
-
 func getPackage(skip ...int) string {
 	sk := 2
 	if len(skip) > 0 {
@@ -149,8 +142,8 @@ var CallersFrames = func(callers []uintptr) Frames {
 }
 
 // Retrieve retrieves a trace and returns it.
-func Retrieve(skip int, endFunction string) []Level {
-	var trace []Level
+func Retrieve(skip int, endFunction string) Stack {
+	var trace Stack
 	var pc [40]uintptr
 	if num := runtime.Callers(skip+2, pc[:]); num > 0 {
 		checkIgnore := true

internal/trace/trace_test.go

@@ -14,9 +14,8 @@ import (
 	"runtime"
 	"testing"
 
-	"github.com/maxatome/go-testdeep/internal/trace"
-
 	"github.com/maxatome/go-testdeep/internal/test"
+	"github.com/maxatome/go-testdeep/internal/trace"
 )
 
 func TestIgnorePackage(t *testing.T) {

td/cmp_deeply.go

@@ -25,64 +25,92 @@ func init() {
 }
 
 // stripTrace removes go-testdeep useless calls in a trace returned by
-// trace.Retrieve().
-//
-// Remove (*T).Run() call:
-//    A()                   zz.go:20
-//    TestSubtestTd.func1() zz_test.go:24
-//   →(*T).Run.func1()      github.com/maxatome/go-testdeep@xxx/td/t_struct.go:554
-//
-// Remove (*T).RunAssertRequire() call:
-//    A()                           zz.go:20
-//    TestSubtestTd.func1()         zz_test.go:24
-//   →(*T).RunAssertRequire.func1() github.com/maxatome/go-testdeep@xxx/td/t_struct.go:554
-//
-// Remove (*TestAPI).Run() call:
-//    Subtest.func1() zz.go:20
-//   →(*TestAPI).Run.func1() github.com/maxatome/go-testdeep@xxx/helpers/tdhttp/test_api.go:119
-//   ⇒(*T).Run.func1()       github.com/maxatome/go-testdeep@xxx/td/t_struct.go:554
-//
-// Remove github.com/maxatome/go-testdeep/helpers/tdsuite calls:
-//    A()               zz.go:20
-//    Suite.TestSuite() zz_test.go:35
-//   →Value.call()      $GOROOT/src/reflect/value.go:476
-//   →Value.Call()      $GOROOT/src/reflect/value.go:337
-//   →run.func2()       github.com/maxatome/go-testdeep@xxx/helpers/tdsuite/suite.go:301
-//   ⇒(*T).Run.func1()  github.com/maxatome/go-testdeep@xxx/td/t_struct.go:554
-func stripTrace(tce []trace.Level) []trace.Level {
-	if len(tce) <= 1 {
-		return tce
+// trace.Retrieve() to make it clearer for the reader.
+func stripTrace(s trace.Stack) trace.Stack {
+	if len(s) <= 1 {
+		return s
 	}
 
+	const (
+		tdPkg      = "github.com/maxatome/go-testdeep/td"
+		tdhttpPkg  = "github.com/maxatome/go-testdeep/helpers/tdhttp"
+		tdsuitePkg = "github.com/maxatome/go-testdeep/helpers/tdsuite"
+	)
+
 	// Remove useless possible (*T).Run() or (*T).RunAssertRequire() first call
-	first := tce[len(tce)-1]
-	if first.Package != "github.com/maxatome/go-testdeep/td" ||
-		(first.Func != "(*T).Run.func1" &&
-			first.Func != "(*T).RunAssertRequire.func1") {
-		return tce
-	}
+	if s.Match(-1, tdPkg, "(*T).Run.func1", "(*T).RunAssertRequire.func1") {
+		// Remove useless tdhttp (*TestAPI).Run() call
+		//
+		// ✓ xxx      Subtest.func1()
+		// ✗ …/tdhttp (*TestAPI).Run.func1
+		// ✗ …/td     (*T).Run.func1()
+		if s.Match(-2, tdhttpPkg, "(*TestAPI).Run.func1") {
+			return s[:len(s)-2]
+		}
 
-	tce = tce[:len(tce)-1]
-	first = tce[len(tce)-1]
+		// Remove useless tdsuite calls
+		//
+		// ✓ xxx       Suite.TestSuite
+		// ✗ reflect   Value.call
+		// ✗ reflect   Value.Call
+		// ✗ …/tdsuite run.func2
+		// ✗ …/td      (*T).Run.func1() or (*T).RunAssertRequire.func1()
+		//
+		// or for PostTest
+		// ✓ xxx       Suite.PostTest
+		// ✗ …/tdsuite run.func2.1
+		// ✗ …/tdsuite run.func2
+		// ✗ …/td      (*T).Run.func1() or (*T).RunAssertRequire.func1()
+		if s.Match(-2, tdsuitePkg, "run.func*") {
+			// PostTest
+			if s.Match(-3, tdsuitePkg, "run.func*") &&
+				len(s) > 4 &&
+				strings.HasSuffix(s[len(s)-4].Func, ".PostTest") {
+				return s[:len(s)-3]
+			}
+
+			for i := len(s) - 3; i >= 1; i-- {
+				if !s.Match(i, "reflect") {
+					return s[:i+1]
+				}
+			}
+			return nil
+		}
 
-	// Remove useless tdhttp (*TestAPI).Run() call
-	if first.Package == "github.com/maxatome/go-testdeep/helpers/tdhttp" &&
-		first.Func == "(*TestAPI).Run.func1" {
-		return tce[:len(tce)-1]
+		return s[:len(s)-1]
 	}
 
-	// Remove useless tdsuite calls
-	if first.Package != "github.com/maxatome/go-testdeep/helpers/tdsuite" ||
-		!strings.HasPrefix(first.Func, "run.func") {
-		return tce
+	// Remove testing.Cleanup() stack
+	//
+	// ✓ xxx     TestCleanup.func2
+	// ✗ testing (*common).Cleanup.func1
+	// ✗ testing (*common).runCleanup
+	// ✗ testing tRunner.func2
+	if s.Match(-1, "testing", "tRunner.func*") &&
+		s.Match(-2, "testing", "(*common).runCleanup") &&
+		s.Match(-3, "testing", "(*common).Cleanup.func1") {
+		return s[:len(s)-3]
 	}
 
-	for i := len(tce) - 2; i >= 1; i-- {
-		if tce[i].Package != "reflect" {
-			return tce[:i+1]
+	// Remove tdsuite pre-Setup/BetweenTests/Destroy stack
+	//
+	// ✓ xxx       Suite.Destroy
+	// ✗ …/tdsuite run.func1
+	// ✗ …/tdsuite run
+	// ✗ …/tdsuite Run
+	// ✓ xxx       TestSuiteDestroy
+	if !s.Match(-1, tdsuitePkg) &&
+		s.Match(-2, tdsuitePkg, "Run") {
+		for i := len(s) - 3; i >= 0; i-- {
+			if !s.Match(i, tdsuitePkg) {
+				s[i+1] = s[len(s)-1]
+				return s[:i+2]
+			}
 		}
+		return s[:1]
 	}
-	return nil
+
+	return s
 }
 
 func formatError(t TestingT, isFatal bool, err *ctxerr.Error, args ...interface{}) {
@@ -106,19 +134,19 @@ func formatError(t TestingT, isFatal bool, err *ctxerr.Error, args ...interface{
 	err.Append(&buf, "")
 
 	// Stask trace
-	if tce := stripTrace(trace.Retrieve(0, "testing.tRunner")); len(tce) > 1 {
+	if s := stripTrace(trace.Retrieve(0, "testing.tRunner")); len(s) > 1 {
 		buf.WriteString("\nThis is how we got here:\n")
 
 		fnMaxLen := 0
-		for _, level := range tce {
+		for _, level := range s {
 			if len(level.Func) > fnMaxLen {
 				fnMaxLen = len(level.Func)
 			}
 		}
 		fnMaxLen += 2
 
 		nl := ""
-		for _, level := range tce {
+		for _, level := range s {
 			fmt.Fprintf(&buf, "%s\t%-*s %s", nl, fnMaxLen, level.Func+"()", level.FileLine)
 			nl = "\n"
 		}