matryer / moq

Interface mocking tool for go generate
http://bit.ly/meetmoq
MIT License
2k stars 127 forks source link

Pass counter to the generated methods (optionally, with -count flag) #160

Closed umputun closed 3 years ago

umputun commented 3 years ago

Very often I need to alter the return value of the called mock function based on the sequence number (counter). Currently, it is possible to do this by maintaining an external count and incrementing it in the call itself. I mean smth like this:

evCount:=0
p1 := &ProviderMock{
        EventsFunc: func(ctx context.Context) <-chan ProviderID {
                        evCount++
                        if evCount == 2 {
                                res := make(chan ProviderID, 2)
                                return res
                        }
            res := make(chan ProviderID, 1)
            res <- PIFile
            return res
        },
    }

Another way to achieve it is to use the provided FuncCalls() method, I.e. len(FuncCalls()) but this approach requires to split declaration and implementation, I.e one needs to declare mock first and add the function after this:

p1 := &ProviderMock{}
p1.EventsFunc: func(ctx context.Context) <-chan ProviderID {
     evCount := len(p1.EvensCalls())
     ....
}

My proposal is to add an extra flag, something like -count adding a count parameter to each generated mock and the usage will be as simple as this:

p1 := &ProviderMock{
        EventsFunc: func(ctx context.Context, count int) <-chan ProviderID {
                }
}

Let me know what you think of this idea and I could try to implement it if you like it.

breml commented 3 years ago

I am personally not convinced, that the argument of having to split the declaration and the implementation is worth the additional complexity of the proposed flag. If you don't like the assignment of the functions one by one, you could even go for:

var p1 *ProviderMock
p1 = &ProviderMock {
     EventsFunc: func(ctx context.Context) <-chan ProviderID {
          evCount := len(p1.EvensCalls())
     ....
     },
}
umputun commented 3 years ago

well, this doesn't resolve the split, i.e. declaration in your example is still a separate step from the actual implementation.

Personally, I think the simpler "user interface" is worths it, as we get rid of this artificial split and won't need the len call for something that essential and useful. From my experience, many non-trivial tests need some sort of state, and having this count easily accessible will make usage of moq more pleasant. I don't think the implementation will add any significant amount of code either.

breml commented 3 years ago

In non trivial test, where my mock needs to respond differently in multiple subsequent calls (where the number of calls would become handy), I normally use a pattern with a list (slice) of responses (could also be a list of functions that return the necessary response values) where on every request, the first item is taken from the list. This looks more or less like this:

type Response string
responseQueue := []State{ "initial", "running", "finished" }

p1 := &ProviderMock {
    EventsFunc: func(ctx context.Context) Response {
        is.True(len(responseQueue) > 0) // EventsFunc was called when queue was depleted
        // Pop response
        resp := responseQueue[0]
        responseQueue = responseQueue[1:]

        return resp
    },
}

In my case, the responseQueue is often part of my test table. I really try to avoid any complex state handling logic in my mock, because for me, the mocks should be as simple as possible and easy to reason about. After all, we are not testing the code of the mock but the SUT (system under test), which depends on the mocks in order to perform its task.

is in the above example refers to https://pkg.go.dev/github.com/matryer/is

umputun commented 3 years ago

thx, this is an interesting approach.

I have tried to implement POV and it wasn't hard to make it generate what I wanted. However, I missed an essential issue - as soon as I add an extra parameter count to the generated mock signature, the mock doesn't match the interface anymore. This is a fairly fundamental problem that kills the idea altogether. Probably I could figure some workaround, but it is getting ugly fast, so I would close the ticket.

sudo-suhas commented 3 years ago

Thank you @breml for the helpful suggestions 🙂