golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.32k stars 607 forks source link

Providing functional argument to mocked method #324

Open MattNolf opened 5 years ago

MattNolf commented 5 years ago

I am encountering an issue mocking method calls with a functional argument. It appears that gomock is not matching the argument, and so failing with an unexpected call.

Tests are failing with the following:

Unexpected call to *gmt_mock.MockConsumer.Consume([some-name 0x10f8cf0]) at because: 
            Expected call at /Users/matthewnolf/go/src/github.com/mattnolf/gomock-test/main_test.go:20 doesn't match the argument at index 1.
            Got: 0x10f8cf0
            Want: is equal to 0x10f8cf0

With argument at index 1 being an aliased function.

I have provided an example scenario below.

// ConsumerHandler is a function to handle consumed messages
type ConsumerHandler func(id string) error

// ConsumerOption is a function to inject provided option into the consumer
type ConsumerOption func(consumer Consumer) error

// WithSomeName is a ConsumerOption
func WithSomeName(handler ConsumerHandler) ConsumerOption {
    return func(consumer Consumer) error {
        return consumer.Consume("some-name", handler)
    }
}

// Consumer is used to consume messages from some broker
type Consumer interface {
    Consume(name string, handler ConsumerHandler) error
}

// an implementation of Consumer
type mockConsumer struct {}
func (mc mockConsumer) Consume(name string, handler ConsumerHandler) error {
    return nil
}

// Start begins the consumer
// example invokation:
//  mockHandler := func(id string) error {
//      return nil
//  }
//
//  Start(consumer, WithSomeName(mockHandler))
func Start(c Consumer, opt ConsumerOption) {
        opt(c)
}

And the failing test

func TestStart(t *testing.T) {
    t.Run("test the consumer runs", func(t *testing.T) {
        ctrl := gomock.NewController(t)

        mockHandler := func(id string) error {
            return nil
        }

        mockConsumer := mock.NewMockConsumer(ctrl)
        mockConsumer.EXPECT().Consume("some-name", mockHandler)

        gmt.Start(mockConsumer, gmt.WithSomeName(mockHandler))
    })
}

mockConsumer.EXPECT().Consume("some-name", mockHandler) is failing, as gomock does not match mockHandler as the provided argument.

Here is the test repository

tscott0 commented 5 years ago

According to the Go language spec:

Slice, map, and function values are not comparable. However, as a special case, a slice, map, or function value may be compared to the predeclared identifier nil.

I suspect this is why it doesn't work here. There might be workarounds using the reflect package but that's not great.

As a workaround you could implement a stub Consume function (using Do function) and then check your function is called that way. Hope this helps!

MattNolf commented 5 years ago

@tscott0 Perhaps, but it's possible to expect & assert on slices/maps so seems strange that functions would be different.

Will take a look at Do() as a workaround in the meantime 👍

tscott0 commented 5 years ago

@MattNolf I didn't know expect and assert worked for slices/maps but that's good to know. I was curious so I did some digging...

Under the hood it's using reflect.DeepEqual.

What I didn't realise is that you can't compare functions with DeepEqual. They are only equal if they are both nil. See docs here. This explains it but there still might be a solution that doesn't use DeepEqual so I think the issue should remain open. Good luck!

codyoss commented 5 years ago

Thank you @MattNolf for your detailed report, that is very useful. I will look into this...

nicesj commented 4 years ago

Dear all, Unfortunately, I have this issue. Do you have any updates? And is there any way to avoid this issue temporarily?

cvgw commented 4 years ago

I don't think this one is fixable as comparison of functions has no guarantee at the language level in Golang.

I propose that passing a function argument to a mocked method should panic.

Instead, the correct usage should be with gomock.AssignableToTypeOf which currently works.

There is no way to reliably compare functions so this should result in an error IMO.

@nguyenfilip @codyoss WDYT?

See following links for more details:

minicuts commented 4 years ago

@cvgw I agree, this would be very hard to support. Panicing sounds like the best short term behavior change.

zachwalton commented 3 years ago

Hope it's ok to post a workaround; have been dealing with this for a while for variadic option functions and finally found a workable solution.

@MattNolf extending your test:

https://github.com/zachwalton/gomock-test/commit/04033d4265da8cc0d4acfb1c2307963b511916c2

You can build a custom matcher that:

  1. Defines the expected function
  2. Intercepts & casts the argument
  3. Compares the 2 functions in whatever way works best for your test case

(3) could be reflection, but for my purposes (building and returning a variadic options struct), it was enough to execute both functions and compare the return values.

adu-crypto commented 2 years ago

As a workaround you could implement a stub Consume function (using Do function) and then check your function is called that way.

this worked for me. we just need to use go.Any() to match the function argument but do nothing and use Do() to do what the method should have done.