From 2b76a9702ee5548248cded996e8614e3469e1cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Jime=CC=81nez?= Date: Fri, 26 May 2017 07:46:02 +0100 Subject: [PATCH 1/3] Revert "Added goconvey style assertions to the mock package" This reverts commit faf0710ff28e0389cd8f4761de3fbf4b7cd1057d. --- README.md | 31 --------- mock/mock.go | 165 +++------------------------------------------- mock/mock_test.go | 49 -------------- 3 files changed, 10 insertions(+), 235 deletions(-) diff --git a/README.md b/README.md index 5100e50..e57b181 100644 --- a/README.md +++ b/README.md @@ -180,37 +180,6 @@ For more information on how to write mock code, check out the [API documentation You can use the [mockery tool](http://github.com/vektra/mockery) to autogenerate the mock code against an interface as well, making using mocks much quicker. -### Plays well with [`goconvey`](https://github.com/smartystreets/goconvey) -`goconvey` is a BDD style testing framework for gophers. The `mock` package defines several assertion functions intended to -work with that framework - -Here is an example test function testing a piece of code that operates on a mock called `Example`. -The test function can setup expectations (testify) for `Example` and assert that they indeed happened: - -```go -package convey - -import ( - "testing" - . "github.com/smartystreets/goconvey/convey" - "github.com/vektra/mockery/mocks" - m "github.com/stretchr/testify/mock" -) - -func TestExampleMock(t *testing.T) { - Convey( "Given the example mock" , t, func() { - mock := new(mocks.Example) - mock.On("A").Return(nil) - Convey("When A is called", func() { - mock.A() - Convey("Assert A is called", func() { - So(mock, m.MethodWasCalled, "A") - }) - }) - }) -} -``` - [`suite`](http://godoc.org/github.com/stretchr/testify/suite "API documentation") package ----------------------------------------------------------------------------------------- diff --git a/mock/mock.go b/mock/mock.go index 82e5ec8..defb681 100644 --- a/mock/mock.go +++ b/mock/mock.go @@ -13,11 +13,6 @@ import ( "github.com/pmezard/go-difflib/difflib" "github.com/stretchr/objx" "github.com/stretchr/testify/assert" - "bytes" -) - -const ( - convey_test_passed_message = "" ) // TestingT is an interface wrapper around *testing.T @@ -376,25 +371,6 @@ func AssertExpectationsForObjects(t TestingT, testObjects ...interface{}) bool { // AssertExpectations asserts that everything specified with On and Return was // in fact called as expected. Calls may have occurred in any order. func (m *Mock) AssertExpectations(t TestingT) bool { - errorMsg := ExpectationsWereMet(m) - if errorMsg != convey_test_passed_message { - t.Errorf(errorMsg) - return false - } else { - return true - } -} - -// ExpectationsWereMet is a goconvey style assertion see: https://github.com/smartystreets/goconvey/wiki/Assertions -// It asserts that everything specified with On and Return was -// in fact called as expected. Calls may have occurred in any order -func ExpectationsWereMet(iFace interface{}, args ...interface{}) string { - m, err := getMock(iFace) - if err != nil { - return err.Error() - } - - b := new(bytes.Buffer) var somethingMissing bool var failedExpectations int @@ -404,176 +380,55 @@ func ExpectationsWereMet(iFace interface{}, args ...interface{}) string { if !m.methodWasCalled(expectedCall.Method, expectedCall.Arguments) && expectedCall.totalCalls == 0 { somethingMissing = true failedExpectations++ - fmt.Fprintf(b, "\u274C\t%s(%s)\n", expectedCall.Method, expectedCall.Arguments.String()) + t.Logf("\u274C\t%s(%s)", expectedCall.Method, expectedCall.Arguments.String()) } else { m.mutex.Lock() if expectedCall.Repeatability > 0 { somethingMissing = true failedExpectations++ } else { - fmt.Fprintf(b, "\u2705\t%s(%s)\n", expectedCall.Method, expectedCall.Arguments.String()) + t.Logf("\u2705\t%s(%s)", expectedCall.Method, expectedCall.Arguments.String()) } m.mutex.Unlock() } } if somethingMissing { - fmt.Fprintf(b, "FAIL: %d out of %d expectation(s) were met.\n\tThe code you are testing needs to make %d more call(s).\n\tat: %s", len(expectedCalls)-failedExpectations, len(expectedCalls), failedExpectations, assert.CallerInfo()) - return b.String() - } else { - return convey_test_passed_message + t.Errorf("FAIL: %d out of %d expectation(s) were met.\n\tThe code you are testing needs to make %d more call(s).\n\tat: %s", len(expectedCalls)-failedExpectations, len(expectedCalls), failedExpectations, assert.CallerInfo()) } + + return !somethingMissing } // AssertNumberOfCalls asserts that the method was called expectedCalls times. func (m *Mock) AssertNumberOfCalls(t TestingT, methodName string, expectedCalls int) bool { - msg := NumberOfCalls(m, methodName, expectedCalls) - passedTest := msg == convey_test_passed_message - if !passedTest { - assert.Fail(t, msg) - } - - return passedTest -} - -func getMock(iFace interface{})(Mock, error) { - var mock Mock - if m, ok := iFace.(Mock); ok { - mock = m - } else { - iVal := reflect.ValueOf(iFace) - if iVal.Type().Kind() == reflect.Ptr { - return getMock(iVal.Elem().Interface()) - } else if iVal.Type().Kind() == reflect.Struct && iVal.NumField() >= 1 { - v := iVal.Field(0) - if m, ok := v.Interface().(Mock); ok { - mock = m - } else { - return Mock{}, fmt.Errorf("Could not get mock from interface %v. Had type %v", iFace, reflect.TypeOf(iFace)) - } - } else { - return Mock{}, fmt.Errorf("Could not get mock from interface %v. Had type %v", iFace, reflect.TypeOf(iFace)) - } - } - - return mock, nil -} - -// NumberOfCalls is a goconvey style assertion see: https://github.com/smartystreets/goconvey/wiki/Assertions -// It asserts that the method was called expectedCalls times. -func NumberOfCalls(iFace interface{}, args ...interface{}) string { - m, err := getMock(iFace) - if err != nil { - return err.Error() - } - - var methodName string - if stringVal, ok := args[0].(string); !ok { - typ := reflect.TypeOf(iFace) - return fmt.Sprintf("Argument 0 (methodName) should be a string. (was %s)", typ.Name()) - } else { - methodName = stringVal - } - - var expectedCalls int - if intVal, ok := args[1].(int); !ok { - typ := reflect.TypeOf(iFace) - return fmt.Sprintf("Argument 1 (expectedCalls) should be a int. (was %s)", typ.Name()) - } else { - expectedCalls = intVal - } - var actualCalls int for _, call := range m.calls() { if call.Method == methodName { actualCalls++ } } - - if actualCalls != expectedCalls { - return fmt.Sprintf("Expected number of calls (%d) does not match the actual number of calls (%d).", expectedCalls, actualCalls) - } else { - return convey_test_passed_message - } + return assert.Equal(t, expectedCalls, actualCalls, fmt.Sprintf("Expected number of calls (%d) does not match the actual number of calls (%d).", expectedCalls, actualCalls)) } // AssertCalled asserts that the method was called. // It can produce a false result when an argument is a pointer type and the underlying value changed after calling the mocked method. func (m *Mock) AssertCalled(t TestingT, methodName string, arguments ...interface{}) bool { - args := make([]interface{}, len(arguments) + 1) - args[0] = methodName - copy(args[1:], arguments) - message := MethodWasCalled(m, args...) - if message == convey_test_passed_message { - return true - } else { - assert.Fail(t, message) + if !assert.True(t, m.methodWasCalled(methodName, arguments), fmt.Sprintf("The \"%s\" method should have been called with %d argument(s), but was not.", methodName, len(arguments))) { t.Logf("%v", m.expectedCalls()) return false } -} - -// MethodWasCalled is a goconvey style assertion see: https://github.com/smartystreets/goconvey/wiki/Assertions -// It asserts that the method was called. -// It can produce a false result when an argument is a pointer type and the underlying value changed after calling the mocked method. -func MethodWasCalled(iFace interface{}, args ...interface{}) string { - m, err := getMock(iFace) - if err != nil { - return err.Error() - } - var methodName string - if stringVal, ok := args[0].(string); !ok { - typ := reflect.TypeOf(iFace) - return fmt.Sprintf("Argument 0 (methodName) should be a string. (was %s)", typ.Name()) - } else { - methodName = stringVal - } - - if !m.methodWasCalled(methodName, args[1:]) { - return fmt.Sprintf("The \"%s\" method should have been called with %d argument(s), but was not.", methodName, len(args[1:])) - } - - return convey_test_passed_message + return true } // AssertNotCalled asserts that the method was not called. // It can produce a false result when an argument is a pointer type and the underlying value changed after calling the mocked method. func (m *Mock) AssertNotCalled(t TestingT, methodName string, arguments ...interface{}) bool { - args := make([]interface{}, len(arguments) + 1) - args[0] = methodName - copy(args[1:], arguments) - - message := MethodWasNotCalled(m, args...) - if message == convey_test_passed_message { - return true - } else { - assert.Fail(t, message) + if !assert.False(t, m.methodWasCalled(methodName, arguments), fmt.Sprintf("The \"%s\" method was called with %d argument(s), but should NOT have been.", methodName, len(arguments))) { t.Logf("%v", m.expectedCalls()) return false } -} - -// MethodWasNotCalled is a goconvey style assertion see: https://github.com/smartystreets/goconvey/wiki/Assertions -// It asserts that the method was not called. -// It can produce a false result when an argument is a pointer type and the underlying value changed after calling the mocked method. -func MethodWasNotCalled(iFace interface{}, args ...interface{}) string { - m, err := getMock(iFace) - if err != nil { - return err.Error() - } - var methodName string - if stringVal, ok := args[0].(string); !ok { - typ := reflect.TypeOf(iFace) - return fmt.Sprintf("Argument 0 (methodName) should be a string. (was %s)", typ.Name()) - } else { - methodName = stringVal - } - - if m.methodWasCalled(methodName, args[1:]) { - return fmt.Sprintf("The \"%s\" method was called with %d argument(s), but should NOT have been.", methodName, len(args[1:])) - } - - return convey_test_passed_message + return true } func (m *Mock) methodWasCalled(methodName string, expected []interface{}) bool { diff --git a/mock/mock_test.go b/mock/mock_test.go index b71fc3e..208dab7 100644 --- a/mock/mock_test.go +++ b/mock/mock_test.go @@ -796,20 +796,6 @@ func Test_Mock_AssertExpectations(t *testing.T) { } -func Test_Mock_Convey_Expectations(t *testing.T) { - - var mockedService = new(TestExampleImplementation) - - mockedService.On("Test_Mock_Convey_Expectations", 1, 2, 3).Return(5, 6, 7) - - // make the call now - mockedService.Called(1, 2, 3) - - // now assert expectations - assert.Empty(t, ExpectationsWereMet(mockedService)) - -} - func Test_Mock_AssertExpectations_Placeholder_NoArgs(t *testing.T) { var mockedService = new(TestExampleImplementation) @@ -943,19 +929,6 @@ func Test_Mock_AssertNumberOfCalls(t *testing.T) { } -func Test_Mock_Convey_NumberOfCalls(t *testing.T) { - - var mockedService = new(TestExampleImplementation) - - mockedService.On("Test_Mock_Convey_NumberOfCalls", 1, 2, 3).Return(5, 6, 7) - - mockedService.Called(1, 2, 3) - assert.Empty(t, NumberOfCalls(mockedService, "Test_Mock_Convey_NumberOfCalls", 1)) - - mockedService.Called(1, 2, 3) - assert.Empty(t, NumberOfCalls(mockedService, "Test_Mock_Convey_NumberOfCalls", 2)) -} - func Test_Mock_AssertCalled(t *testing.T) { var mockedService = new(TestExampleImplementation) @@ -968,17 +941,6 @@ func Test_Mock_AssertCalled(t *testing.T) { } -func Test_Mock_Convey_Called(t *testing.T) { - - var mockedService = new(TestExampleImplementation) - - mockedService.On("Test_Mock_Convey_Called", 1, 2, 3).Return(5, 6, 7) - - mockedService.Called(1, 2, 3) - - assert.Empty(t, MethodWasCalled(mockedService, "Test_Mock_Convey_Called", 1, 2, 3)) -} - func Test_Mock_AssertCalled_WithAnythingOfTypeArgument(t *testing.T) { var mockedService = new(TestExampleImplementation) @@ -1036,17 +998,6 @@ func Test_Mock_AssertNotCalled(t *testing.T) { } -func Test_Mock_Convey_NotCalled(t *testing.T) { - - var mockedService = new(TestExampleImplementation) - - mockedService.On("Test_Mock_Convey_NotCalled", 1, 2, 3).Return(5, 6, 7) - - mockedService.Called(1, 2, 3) - - assert.Empty(t, MethodWasNotCalled(mockedService, "Test_Mock_NotCalled")) -} - /* Arguments helper methods */ From 34687ebd2898f9bd29d30f66689658f82fae0d13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Jime=CC=81nez?= Date: Fri, 26 May 2017 07:46:34 +0100 Subject: [PATCH 2/3] Revert "diffArguments: remove unnecessary range-for (#417)" This reverts commit 5c861cc4a41d0c534deebc34de0fa1ffd15429bf. Bug was already fixed in another merge. --- mock/mock.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/mock/mock.go b/mock/mock.go index defb681..88bec64 100644 --- a/mock/mock.go +++ b/mock/mock.go @@ -718,11 +718,7 @@ func typeAndKind(v interface{}) (reflect.Type, reflect.Kind) { } func diffArguments(expected Arguments, actual Arguments) string { - if diffString := diff(expected, actual); diffString != "" { - return fmt.Sprintf("Difference found in arguments:\n\n%s", diffString) - } - - if len(expected) != len(actual) { + if len(expected) != len(actual) { return fmt.Sprintf("Provided %v arguments, mocked for %v arguments", len(expected), len(actual)) } From f712be92669879285a5ea00f921d0a55587b6172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Jime=CC=81nez?= Date: Fri, 26 May 2017 07:47:23 +0100 Subject: [PATCH 3/3] Revert "add mock.MethodCalled(methodName, args...)" This reverts commit 17a0bd50e9ddb9f2dbff569e5c01a27e9171be96. Should avoid simply exporting an internal method. --- mock/mock.go | 7 ++----- mock/mock_test.go | 23 +++++------------------ 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/mock/mock.go b/mock/mock.go index 88bec64..f04914c 100644 --- a/mock/mock.go +++ b/mock/mock.go @@ -213,7 +213,7 @@ func (m *Mock) On(methodName string, arguments ...interface{}) *Call { // Recording and responding to activity // */ -func (m *Mock) FindExpectedCall(method string, arguments ...interface{}) (int, *Call) { +func (m *Mock) findExpectedCall(method string, arguments ...interface{}) (int, *Call) { m.mutex.Lock() defer m.mutex.Unlock() for i, call := range m.ExpectedCalls { @@ -287,11 +287,8 @@ func (m *Mock) Called(arguments ...interface{}) Arguments { } parts := strings.Split(functionPath, ".") functionName := parts[len(parts)-1] - return m.MethodCalled(functionName, arguments...) -} -func (m *Mock) MethodCalled(functionName string, arguments ...interface{}) Arguments { - found, call := m.FindExpectedCall(functionName, arguments...) + found, call := m.findExpectedCall(functionName, arguments...) if found < 0 { // we have to fail here - because we don't know what to do diff --git a/mock/mock_test.go b/mock/mock_test.go index 208dab7..a681623 100644 --- a/mock/mock_test.go +++ b/mock/mock_test.go @@ -2,11 +2,10 @@ package mock import ( "errors" - "testing" - "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "testing" + "time" ) /* @@ -562,7 +561,7 @@ func Test_Mock_findExpectedCall(t *testing.T) { m.On("Two", 2).Return("two") m.On("Two", 3).Return("three") - f, c := m.FindExpectedCall("Two", 3) + f, c := m.findExpectedCall("Two", 3) if assert.Equal(t, 2, f) { if assert.NotNil(t, c) { @@ -581,7 +580,7 @@ func Test_Mock_findExpectedCall_For_Unknown_Method(t *testing.T) { m.On("Two", 2).Return("two") m.On("Two", 3).Return("three") - f, _ := m.FindExpectedCall("Two") + f, _ := m.findExpectedCall("Two") assert.Equal(t, -1, f) @@ -595,7 +594,7 @@ func Test_Mock_findExpectedCall_Respects_Repeatability(t *testing.T) { m.On("Two", 3).Return("three").Twice() m.On("Two", 3).Return("three").Times(8) - f, c := m.FindExpectedCall("Two", 3) + f, c := m.findExpectedCall("Two", 3) if assert.Equal(t, 2, f) { if assert.NotNil(t, c) { @@ -1185,15 +1184,3 @@ func Test_WaitUntil_Parallel(t *testing.T) { // Allow the first call to execute, so the second one executes afterwards ch2 <- time.Now() } - -func Test_MethodCalled(t *testing.T) { - m := new(Mock) - m.On("foo", "hello").Return("world") - - found, _ := m.FindExpectedCall("foo", "hello") - require.True(t, found >= 0) - retArgs := m.MethodCalled("foo", "hello") - require.True(t, len(retArgs) == 1) - require.Equal(t, "world", retArgs[0]) - m.AssertExpectations(t) -}