Repository: stretchr/testify
Pull request #1288 (2022-10-26 08:56:22)

Fix `CallerInfo()` source file paths
## Summary
`Error Trace` produce strange output with invalid file paths: it join current execution directory and base source filename, but source file can be located in another directory.

It is especially strange that these path conversions are not needed, since the desired path initially got from stacktrace.

## Related issues
Fix https://github.com/stretchr/testify/issues/1230
Fix https://github.com/stretchr/testify/issues/1268


twelvelabs: :+1: thank you so much for this - it's been driving me nuts.
bozaro: I compare test output before and after this PR (source code: https://github.com/bozaro/testify-example).
## go test ./example
### Before
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	/home/bozaro/git.bozaro.ru/testify-example/1.8.1/example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
### After
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	/home/bozaro/git.bozaro.ru/testify-example/1.8.1+1288/example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
## go test -trimpath ./example
### Before
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	/home/bozaro/git.bozaro.ru/testify-example/1.8.1/example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
### After
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	github.com/bozaro/testify-example/example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
## bazel test --test_output=errors //...
### Before
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	/home/bozaro/.cache/bazel/_bazel_bozaro/b8360faabc5337403861613e54adf2d0/sandbox/linux-sandbox/122/execroot/__main__/bazel-out/k8-fastbuild/bin/example/example_test_/example_test.runfiles/__main__/example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
### After
```
--- FAIL: TestSimple (0.00s)
    example_test.go:9: 
        	Error Trace:	example/example_test.go:9
        	Error:      	Hello, world!
        	Test:       	TestSimple
FAIL
```
bozaro: A create tree with two files:
- `example/helper/helper.go`
- `example/example_test.go`

And run `go test ./example`

### Before
```
--- FAIL: TestHelper (0.00s)
    helper.go:9: 
        	Error Trace:	/home/bozaro/github/testify-example/1.8.1/example/helper.go:9
        	            				/home/bozaro/github/testify-example/1.8.1/example/example_test.go:14
        	Error:      	Hello from helper!
        	Test:       	TestHelper
FAIL
```
File `/home/bozaro/github/testify-example/1.8.1/example/helper.go` is not exists.

### After
```
--- FAIL: TestHelper (0.00s)
    helper.go:9: 
        	Error Trace:	/home/bozaro/github/testify-example/1.8.1+1288/example/helper/helper.go:9
        	            				/home/bozaro/github/testify-example/1.8.1+1288/example/example_test.go:14
        	Error:      	Hello from helper!
        	Test:       	TestHelper
```

Patch:
diff --git a/assert/assertions.go b/assert/assertions.go
index fa1245b..c540bb0 100644
--- a/assert/assertions.go
+++ b/assert/assertions.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"math"
 	"os"
-	"path/filepath"
 	"reflect"
 	"regexp"
 	"runtime"
@@ -141,12 +140,11 @@ func CallerInfo() []string {
 		}
 
 		parts := strings.Split(file, "/")
-		file = parts[len(parts)-1]
 		if len(parts) > 1 {
+			filename := parts[len(parts)-1]
 			dir := parts[len(parts)-2]
-			if (dir != "assert" && dir != "mock" && dir != "require") || file == "mock_test.go" {
-				path, _ := filepath.Abs(file)
-				callers = append(callers, fmt.Sprintf("%s:%d", path, line))
+			if (dir != "assert" && dir != "mock" && dir != "require") || filename == "mock_test.go" {
+				callers = append(callers, fmt.Sprintf("%s:%d", file, line))
 			}
 		}
 
@@ -530,7 +528,7 @@ func isNil(object interface{}) bool {
 		[]reflect.Kind{
 			reflect.Chan, reflect.Func,
 			reflect.Interface, reflect.Map,
-			reflect.Ptr, reflect.Slice},
+			reflect.Ptr, reflect.Slice, reflect.UnsafePointer},
 		kind)
 
 	if isNilableKind && value.IsNil() {
diff --git a/mock/mock.go b/mock/mock.go
index f0af824..e6ff8df 100644
--- a/mock/mock.go
+++ b/mock/mock.go
@@ -218,16 +218,22 @@ func (c *Call) Unset() *Call {
 
 	foundMatchingCall := false
 
-	for i, call := range c.Parent.ExpectedCalls {
+	// in-place filter slice for calls to be removed - iterate from 0'th to last skipping unnecessary ones
+	var index int // write index
+	for _, call := range c.Parent.ExpectedCalls {
 		if call.Method == c.Method {
 			_, diffCount := call.Arguments.Diff(c.Arguments)
 			if diffCount == 0 {
 				foundMatchingCall = true
-				// Remove from ExpectedCalls
-				c.Parent.ExpectedCalls = append(c.Parent.ExpectedCalls[:i], c.Parent.ExpectedCalls[i+1:]...)
+				// Remove from ExpectedCalls - just skip it
+				continue
 			}
 		}
+		c.Parent.ExpectedCalls[index] = call
+		index++
 	}
+	// trim slice up to last copied index
+	c.Parent.ExpectedCalls = c.Parent.ExpectedCalls[:index]
 
 	if !foundMatchingCall {
 		unlockOnce.Do(c.unlock)
diff --git a/suite/interfaces.go b/suite/interfaces.go
index 8b98a8a..fed037d 100644
--- a/suite/interfaces.go
+++ b/suite/interfaces.go
@@ -7,6 +7,7 @@ import "testing"
 type TestingSuite interface {
 	T() *testing.T
 	SetT(*testing.T)
+	SetS(suite TestingSuite)
 }
 
 // SetupAllSuite has a SetupSuite method, which will run before the
@@ -51,3 +52,15 @@ type AfterTest interface {
 type WithStats interface {
 	HandleStats(suiteName string, stats *SuiteInformation)
 }
+
+// SetupSubTest has a SetupSubTest method, which will run before each
+// subtest in the suite.
+type SetupSubTest interface {
+	SetupSubTest()
+}
+
+// TearDownSubTest has a TearDownSubTest method, which will run after
+// each subtest in the suite have been run.
+type TearDownSubTest interface {
+	TearDownSubTest()
+}
diff --git a/suite/suite.go b/suite/suite.go
index 8955918..8b4202d 100644
--- a/suite/suite.go
+++ b/suite/suite.go
@@ -22,9 +22,13 @@ var matchMethod = flag.String("testify.m", "", "regular expression to select tes
 // retrieving the current *testing.T context.
 type Suite struct {
 	*assert.Assertions
+
 	mu      sync.RWMutex
 	require *require.Assertions
 	t       *testing.T
+
+	// Parent suite to have access to the implemented methods of parent struct
+	s TestingSuite
 }
 
 // T retrieves the current *testing.T context.
@@ -43,6 +47,12 @@ func (suite *Suite) SetT(t *testing.T) {
 	suite.require = require.New(t)
 }
 
+// SetS needs to set the current test suite as parent
+// to get access to the parent methods
+func (suite *Suite) SetS(s TestingSuite) {
+	suite.s = s
+}
+
 // Require returns a require context for suite.
 func (suite *Suite) Require() *require.Assertions {
 	suite.mu.Lock()
@@ -85,7 +95,18 @@ func failOnPanic(t *testing.T, r interface{}) {
 // Provides compatibility with go test pkg -run TestSuite/TestName/SubTestName.
 func (suite *Suite) Run(name string, subtest func()) bool {
 	oldT := suite.T()
-	defer suite.SetT(oldT)
+
+	if setupSubTest, ok := suite.s.(SetupSubTest); ok {
+		setupSubTest.SetupSubTest()
+	}
+
+	defer func() {
+		suite.SetT(oldT)
+		if tearDownSubTest, ok := suite.s.(TearDownSubTest); ok {
+			tearDownSubTest.TearDownSubTest()
+		}
+	}()
+
 	return oldT.Run(name, func(t *testing.T) {
 		suite.SetT(t)
 		subtest()
@@ -98,6 +119,7 @@ func Run(t *testing.T, suite TestingSuite) {
 	defer recoverAndFailOnPanic(t)
 
 	suite.SetT(t)
+	suite.SetS(suite)
 
 	var suiteSetupDone bool
 


Test patch:
diff --git a/assert/assertions_test.go b/assert/assertions_test.go
index bdab184..b5193a5 100644
--- a/assert/assertions_test.go
+++ b/assert/assertions_test.go
@@ -15,6 +15,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"unsafe"
 )
 
 var (
@@ -2558,3 +2559,10 @@ func TestErrorAs(t *testing.T) {
 		})
 	}
 }
+
+func TestIsNil(t *testing.T) {
+	var n unsafe.Pointer = nil
+	if !isNil(n) {
+		t.Fatal("fail")
+	}
+}
diff --git a/mock/mock_test.go b/mock/mock_test.go
index f77dfe4..260bb9c 100644
--- a/mock/mock_test.go
+++ b/mock/mock_test.go
@@ -569,6 +569,80 @@ func Test_Mock_UnsetIfAlreadyUnsetFails(t *testing.T) {
 	assert.Equal(t, 0, len(mockedService.ExpectedCalls))
 }
 
+func Test_Mock_UnsetByOnMethodSpec(t *testing.T) {
+	// make a test impl object
+	var mockedService = new(TestExampleImplementation)
+
+	mock1 := mockedService.
+		On("TheExampleMethod", 1, 2, 3).
+		Return(0, nil)
+
+	assert.Equal(t, 1, len(mockedService.ExpectedCalls))
+	mock1.On("TheExampleMethod", 1, 2, 3).
+		Return(0, nil).Unset()
+
+	assert.Equal(t, 0, len(mockedService.ExpectedCalls))
+
+	assert.Panics(t, func() {
+		mock1.Unset()
+	})
+
+	assert.Equal(t, 0, len(mockedService.ExpectedCalls))
+}
+
+func Test_Mock_UnsetByOnMethodSpecAmongOthers(t *testing.T) {
+	// make a test impl object
+	var mockedService = new(TestExampleImplementation)
+
+	_, filename, line, _ := runtime.Caller(0)
+	mock1 := mockedService.
+		On("TheExampleMethod", 1, 2, 3).
+		Return(0, nil).
+		On("TheExampleMethodVariadic", 1, 2, 3, 4, 5).Once().
+		Return(nil)
+	mock1.
+		On("TheExampleMethodFuncType", Anything).
+		Return(nil)
+
+	assert.Equal(t, 3, len(mockedService.ExpectedCalls))
+	mock1.On("TheExampleMethod", 1, 2, 3).
+		Return(0, nil).Unset()
+
+	assert.Equal(t, 2, len(mockedService.ExpectedCalls))
+
+	expectedCalls := []*Call{
+		{
+			Parent:          &mockedService.Mock,
+			Method:          "TheExampleMethodVariadic",
+			Repeatability:   1,
+			Arguments:       []interface{}{1, 2, 3, 4, 5},
+			ReturnArguments: []interface{}{nil},
+			callerInfo:      []string{fmt.Sprintf("%s:%d", filename, line+4)},
+		},
+		{
+			Parent:          &mockedService.Mock,
+			Method:          "TheExampleMethodFuncType",
+			Arguments:       []interface{}{Anything},
+			ReturnArguments: []interface{}{nil},
+			callerInfo:      []string{fmt.Sprintf("%s:%d", filename, line+7)},
+		},
+	}
+
+	assert.Equal(t, 2, len(mockedService.ExpectedCalls))
+	assert.Equal(t, expectedCalls, mockedService.ExpectedCalls)
+}
+
+func Test_Mock_Unset_WithFuncPanics(t *testing.T) {
+	// make a test impl object
+	var mockedService = new(TestExampleImplementation)
+	mock1 := mockedService.On("TheExampleMethod", 1)
+	mock1.Arguments = append(mock1.Arguments, func(string) error { return nil })
+
+	assert.Panics(t, func() {
+		mock1.Unset()
+	})
+}
+
 func Test_Mock_Return(t *testing.T) {
 
 	// make a test impl object
diff --git a/suite/suite_test.go b/suite/suite_test.go
index 446029a..d684f52 100644
--- a/suite/suite_test.go
+++ b/suite/suite_test.go
@@ -151,14 +151,16 @@ type SuiteTester struct {
 	Suite
 
 	// Keep counts of how many times each method is run.
-	SetupSuiteRunCount    int
-	TearDownSuiteRunCount int
-	SetupTestRunCount     int
-	TearDownTestRunCount  int
-	TestOneRunCount       int
-	TestTwoRunCount       int
-	TestSubtestRunCount   int
-	NonTestMethodRunCount int
+	SetupSuiteRunCount      int
+	TearDownSuiteRunCount   int
+	SetupTestRunCount       int
+	TearDownTestRunCount    int
+	TestOneRunCount         int
+	TestTwoRunCount         int
+	TestSubtestRunCount     int
+	NonTestMethodRunCount   int
+	SetupSubTestRunCount    int
+	TearDownSubTestRunCount int
 
 	SuiteNameBefore []string
 	TestNameBefore  []string
@@ -255,6 +257,14 @@ func (suite *SuiteTester) TestSubtest() {
 	}
 }
 
+func (suite *SuiteTester) TearDownSubTest() {
+	suite.TearDownSubTestRunCount++
+}
+
+func (suite *SuiteTester) SetupSubTest() {
+	suite.SetupSubTestRunCount++
+}
+
 type SuiteSkipTester struct {
 	// Include our basic suite logic.
 	Suite
@@ -336,6 +346,9 @@ func TestRunSuite(t *testing.T) {
 	assert.Equal(t, suiteTester.TestTwoRunCount, 1)
 	assert.Equal(t, suiteTester.TestSubtestRunCount, 1)
 
+	assert.Equal(t, suiteTester.TearDownSubTestRunCount, 2)
+	assert.Equal(t, suiteTester.SetupSubTestRunCount, 2)
+
 	// Methods that don't match the test method identifier shouldn't
 	// have been run at all.
 	assert.Equal(t, suiteTester.NonTestMethodRunCount, 0)