Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock can be deadlocked by a panic #1157

Merged

Conversation

brackendawson
Copy link
Collaborator

Summary

If an argumentMatcher function panics and AssertExpectations is deferred then the test would deadlock.

Changes

  • Capture panics from argumentMatcher functions and show them to the user as a difference

Motivation

It was reported that when AssertExpectations() is deferred and an argumentMatcher function panics, that the test will deadlock.

Example from linked issue:

import (
	"testing"

	"github.com/stretchr/testify/mock"
)

type Mock struct {
	mock.Mock
}

func (_m *Mock) F() {
	_m.Called()
}

func TestMatcherPanic(t *testing.T) {
  m := &Mock{}
  defer m.AssertExpectations(t)

  matcher := mock.MatchedBy(func(_ interface{}) bool {
    panic("my hovercraft is full of eels")
  })
  m.On("F", matcher)

  m.F()
}

After this change the output will now look like:

--- FAIL: TestMatcherPanic (0.00s)
    /Users/alandaws/src/github.com/brackendawson/kata/mock.go:264: 
        
        mock: Unexpected Method Call
        -----------------------------
        
        F()
        		
        
        The closest call I have is: 
        
        F(mock.argumentMatcher)
        		0: mock.argumentMatcher{fn:reflect.Value{typ:(*reflect.rtype)(0x126a520), ptr:(unsafe.Pointer)(0x12b5f88), flag:0x13}}
        
        Provided 1 arguments, mocked for 0 arguments
        Diff: 0: FAIL:  panic in argument matcher: my hovercraft is full of eels not matched by func(interface {}) bool
    /Users/alandaws/src/github.com/brackendawson/kata/panic.go:661: FAIL:	F(mock.argumentMatcher)
        		at: [kata_test.go:25]
    /Users/alandaws/src/github.com/brackendawson/kata/panic.go:661: FAIL: 0 out of 1 expectation(s) were met.
        	The code you are testing needs to make 1 more call(s).
        	at: [panic.go:661 testing.go:756 kata_test.go:14 kata_test.go:27]

I wonder if that is obvious enough?

Related issues

fixes #1155

If an argumentMatcher function panics and AssertExpectations is deferred then the test would deadlock.
@lzambarda
Copy link

It would be really good to have this merged as it's really hard to write tests when you need to guess why it is hanging!

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@boyan-soubachov boyan-soubachov merged commit c206b2e into stretchr:master Jun 23, 2022
@dolmen dolmen added pkg-mock Any issues related to Mock bug mock.ArgumentMatcher About matching arguments in mock labels Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mock.ArgumentMatcher About matching arguments in mock pkg-mock Any issues related to Mock
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If a matcher panics and AssertExpectations has been deferred, a hang occurs
4 participants