Merge pull request #1665 from vyas-git/master

suite: validate method signatures and continue execution for valid tests

.github/workflows/main.yml

@@ -32,6 +32,7 @@ jobs:
           - "1.20"
           - "1.21"
           - "1.22"
+          - "1.23"
     steps:
       - uses: actions/checkout@v5
       - name: Setup Go

mock/mock.go

@@ -833,6 +833,10 @@ type IsTypeArgument struct {
 // For example:
 //
 //	args.Assert(t, IsType(""), IsType(0))
+//
+// Mock cannot match interface types because the contained type will be  passed
+// to both IsType and Mock.Called, for the zero value of all interfaces this
+// will be <nil> type.
 func IsType(t interface{}) *IsTypeArgument {
 	return &IsTypeArgument{t: reflect.TypeOf(t)}
 }
@@ -948,8 +952,6 @@ func (args Arguments) Is(objects ...interface{}) bool {
 	return true
 }
 
-type outputRenderer func() string
-
 // Diff gets a string describing the differences between the arguments
 // and the specified objects.
 //
@@ -957,7 +959,7 @@ type outputRenderer func() string
 func (args Arguments) Diff(objects []interface{}) (string, int) {
 	// TODO: could return string as error and nil for No difference
 
-	var outputBuilder strings.Builder
+	output := "\n"
 	var differences int
 
 	maxArgCount := len(args)
@@ -965,35 +967,24 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 		maxArgCount = len(objects)
 	}
 
-	outputRenderers := []outputRenderer{}
-
 	for i := 0; i < maxArgCount; i++ {
-		i := i
 		var actual, expected interface{}
-		var actualFmt, expectedFmt func() string
+		var actualFmt, expectedFmt string
 
 		if len(objects) <= i {
 			actual = "(Missing)"
-			actualFmt = func() string {
+			actualFmt = "(Missing)"
-				return "(Missing)"
-			}
 		} else {
 			actual = objects[i]
-			actualFmt = func() string {
+			actualFmt = fmt.Sprintf("(%[1]T=%[1]v)", actual)
-				return fmt.Sprintf("(%[1]T=%[1]v)", actual)
-			}
 		}
 
 		if len(args) <= i {
 			expected = "(Missing)"
-			expectedFmt = func() string {
+			expectedFmt = "(Missing)"
-				return "(Missing)"
-			}
 		} else {
 			expected = args[i]
-			expectedFmt = func() string {
+			expectedFmt = fmt.Sprintf("(%[1]T=%[1]v)", expected)
-				return fmt.Sprintf("(%[1]T=%[1]v)", expected)
-			}
 		}
 
 		if matcher, ok := expected.(argumentMatcher); ok {
@@ -1001,22 +992,16 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 			func() {
 				defer func() {
 					if r := recover(); r != nil {
-						actualFmt = func() string {
+						actualFmt = fmt.Sprintf("panic in argument matcher: %v", r)
-							return fmt.Sprintf("panic in argument matcher: %v", r)
-						}
 					}
 				}()
 				matches = matcher.Matches(actual)
 			}()
 			if matches {
-				outputRenderers = append(outputRenderers, func() string {
+				output = fmt.Sprintf("%s\t%d: PASS:  %s matched by %s\n", output, i, actualFmt, matcher)
-					return fmt.Sprintf("\t%d: PASS:  %s matched by %s\n", i, actualFmt(), matcher)
-				})
 			} else {
 				differences++
-				outputRenderers = append(outputRenderers, func() string {
+				output = fmt.Sprintf("%s\t%d: FAIL:  %s not matched by %s\n", output, i, actualFmt, matcher)
-					return fmt.Sprintf("\t%d: FAIL:  %s not matched by %s\n", i, actualFmt(), matcher)
-				})
 			}
 		} else {
 			switch expected := expected.(type) {
@@ -1025,17 +1010,13 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 				if reflect.TypeOf(actual).Name() != string(expected) && reflect.TypeOf(actual).String() != string(expected) {
 					// not match
 					differences++
-					outputRenderers = append(outputRenderers, func() string {
+					output = fmt.Sprintf("%s\t%d: FAIL:  type %s != type %s - %s\n", output, i, expected, reflect.TypeOf(actual).Name(), actualFmt)
-						return fmt.Sprintf("\t%d: FAIL:  type %s != type %s - %s\n", i, expected, reflect.TypeOf(actual).Name(), actualFmt())
-					})
 				}
 			case *IsTypeArgument:
 				actualT := reflect.TypeOf(actual)
 				if actualT != expected.t {
 					differences++
-					outputRenderers = append(outputRenderers, func() string {
+					output = fmt.Sprintf("%s\t%d: FAIL:  type %s != type %s - %s\n", output, i, safeTypeName(expected.t), safeTypeName(actualT), actualFmt)
-						return fmt.Sprintf("\t%d: FAIL:  type %s != type %s - %s\n", i, expected.t.Name(), actualT.Name(), actualFmt())
-					})
 				}
 			case *FunctionalOptionsArgument:
 				var name string
@@ -1046,36 +1027,26 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 				const tName = "[]interface{}"
 				if name != reflect.TypeOf(actual).String() && len(expected.values) != 0 {
 					differences++
-					outputRenderers = append(outputRenderers, func() string {
+					output = fmt.Sprintf("%s\t%d: FAIL:  type %s != type %s - %s\n", output, i, tName, reflect.TypeOf(actual).Name(), actualFmt)
-						return fmt.Sprintf("\t%d: FAIL:  type %s != type %s - %s\n", i, tName, reflect.TypeOf(actual).Name(), actualFmt())
-					})
 				} else {
 					if ef, af := assertOpts(expected.values, actual); ef == "" && af == "" {
 						// match
-						outputRenderers = append(outputRenderers, func() string {
+						output = fmt.Sprintf("%s\t%d: PASS:  %s == %s\n", output, i, tName, tName)
-							return fmt.Sprintf("\t%d: PASS:  %s == %s\n", i, tName, tName)
-						})
 					} else {
 						// not match
 						differences++
-						outputRenderers = append(outputRenderers, func() string {
+						output = fmt.Sprintf("%s\t%d: FAIL:  %s != %s\n", output, i, af, ef)
-							return fmt.Sprintf("\t%d: FAIL:  %s != %s\n", i, af, ef)
-						})
 					}
 				}
 
 			default:
 				if assert.ObjectsAreEqual(expected, Anything) || assert.ObjectsAreEqual(actual, Anything) || assert.ObjectsAreEqual(actual, expected) {
 					// match
-					outputRenderers = append(outputRenderers, func() string {
+					output = fmt.Sprintf("%s\t%d: PASS:  %s == %s\n", output, i, actualFmt, expectedFmt)
-						return fmt.Sprintf("\t%d: PASS:  %s == %s\n", i, actualFmt(), expectedFmt())
-					})
 				} else {
 					// not match
 					differences++
-					outputRenderers = append(outputRenderers, func() string {
+					output = fmt.Sprintf("%s\t%d: FAIL:  %s != %s\n", output, i, actualFmt, expectedFmt)
-						return fmt.Sprintf("\t%d: FAIL:  %s != %s\n", i, actualFmt(), expectedFmt())
-					})
 				}
 			}
 		}
@@ -1086,12 +1057,7 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 		return "No differences.", differences
 	}
 
-	outputBuilder.WriteString("\n")
+	return output, differences
-	for _, r := range outputRenderers {
-		outputBuilder.WriteString(r())
-	}
-
-	return outputBuilder.String(), differences
 }
 
 // Assert compares the arguments with the specified objects and fails if
@@ -1179,6 +1145,15 @@ func (args Arguments) Bool(index int) bool {
 	return s
 }
 
+// safeTypeName returns the reflect.Type's name without causing a panic.
+// If the provided reflect.Type is nil, it returns the placeholder string "<nil>"
+func safeTypeName(t reflect.Type) string {
+	if t == nil {
+		return "<nil>"
+	}
+	return t.Name()
+}
+
 func typeAndKind(v interface{}) (reflect.Type, reflect.Kind) {
 	t := reflect.TypeOf(v)
 	k := t.Kind()

mock/mock_test.go

@@ -1,10 +1,12 @@
 package mock
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"regexp"
 	"runtime"
+	"strconv"
 	"sync"
 	"testing"
 	"time"
@@ -1998,6 +2000,25 @@ func Test_Arguments_Diff_WithIsTypeArgument_Failing(t *testing.T) {
 	assert.Contains(t, diff, `string != type int - (int=123)`)
 }
 
+func Test_Arguments_Diff_WithIsTypeArgument_InterfaceType(t *testing.T) {
+	t.Parallel()
+	var ctx = context.Background()
+	args := Arguments([]interface{}{IsType(ctx)})
+	_, count := args.Diff([]interface{}{context.Background()})
+	assert.Equal(t, 0, count)
+}
+
+func Test_Arguments_Diff_WithIsTypeArgument_InterfaceType_Failing(t *testing.T) {
+	t.Parallel()
+
+	var ctx context.Context
+	var args = Arguments([]interface{}{IsType(ctx)})
+	diff, count := args.Diff([]interface{}{context.Background()})
+	assert.Equal(t, 1, count)
+	assert.Contains(t, diff, `type <nil> != type `)
+
+}
+
 func Test_Arguments_Diff_WithArgMatcher(t *testing.T) {
 	t.Parallel()
 
@@ -2421,3 +2442,22 @@ type user interface {
 type mockUser struct{ Mock }
 
 func (m *mockUser) Use(c caller) { m.Called(c) }
+
+type mutatingStringer struct {
+	N int
+	s string
+}
+
+func (m *mutatingStringer) String() string {
+	m.s = strconv.Itoa(m.N)
+	return m.s
+}
+
+func TestIssue1785ArgumentWithMutatingStringer(t *testing.T) {
+	m := &Mock{}
+	m.On("Method", &mutatingStringer{N: 2})
+	m.On("Method", &mutatingStringer{N: 1})
+	m.MethodCalled("Method", &mutatingStringer{N: 1})
+	m.MethodCalled("Method", &mutatingStringer{N: 2})
+	m.AssertExpectations(t)
+}

suite/suite.go

@@ -158,7 +158,19 @@ func Run(t *testing.T, suite TestingSuite) {
 		if matchMethodRE != nil && !matchMethodRE.MatchString(method.Name) {
 			continue
 		}
-
+		// Check method signature
+		if method.Type.NumIn() > 1 || method.Type.NumOut() > 0 {
+			tests = append(tests, test{
+				name: method.Name,
+				run: func(t *testing.T) {
+					t.Errorf(
+						"testify: suite method %q has invalid signature: expected no input or output parameters, method has %d input parameters and %d output parameters",
+						method.Name, method.Type.NumIn()-1, method.Type.NumOut(),
+					)
+				},
+			})
+			continue
+		}
 		test := test{
 			name: method.Name,
 			run: func(t *testing.T) {

suite/suite_test.go

@@ -751,3 +751,65 @@ func TestUnInitializedSuites(t *testing.T) {
 		})
 	})
 }
+
+// SuiteSignatureValidationTester tests valid and invalid method signatures.
+type SuiteSignatureValidationTester struct {
+	Suite
+
+	executedTestCount int
+	setUp             bool
+	toreDown          bool
+}
+
+// SetupSuite runs once before any tests.
+func (s *SuiteSignatureValidationTester) SetupSuite() {
+	s.setUp = true
+}
+
+// TearDownSuite runs once after all tests.
+func (s *SuiteSignatureValidationTester) TearDownSuite() {
+	s.toreDown = true
+}
+
+// Valid test method — should run.
+func (s *SuiteSignatureValidationTester) TestValidSignature() {
+	s.executedTestCount++
+}
+
+// Invalid: has return value.
+func (s *SuiteSignatureValidationTester) TestInvalidSignatureReturnValue() interface{} {
+	s.executedTestCount++
+	return nil
+}
+
+// Invalid: has input arg.
+func (s *SuiteSignatureValidationTester) TestInvalidSignatureArg(somearg string) {
+	s.executedTestCount++
+}
+
+// Invalid: both input arg and return value.
+func (s *SuiteSignatureValidationTester) TestInvalidSignatureBoth(somearg string) interface{} {
+	s.executedTestCount++
+	return nil
+}
+
+// TestSuiteSignatureValidation ensures that invalid signature methods fail and valid method runs.
+func TestSuiteSignatureValidation(t *testing.T) {
+	suiteTester := new(SuiteSignatureValidationTester)
+
+	ok := testing.RunTests(allTestsFilter, []testing.InternalTest{
+		{
+			Name: "signature validation",
+			F: func(t *testing.T) {
+				Run(t, suiteTester)
+			},
+		},
+	})
+
+	require.False(t, ok, "Suite should fail due to invalid method signatures")
+
+	assert.Equal(t, 1, suiteTester.executedTestCount, "Only the valid test method should have been executed")
+
+	assert.True(t, suiteTester.setUp, "SetupSuite should have been executed")
+	assert.True(t, suiteTester.toreDown, "TearDownSuite should have been executed")
+}