uber-go / mock

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

Allow control over matching order of call expectations #152

Open thediveo opened 5 months ago

thediveo commented 5 months ago

Requested Feature

Give test spec writers control over the sequence in which call expectations for a specfic 《receiver, method》are processed, in order to allow adding more specific expectations before an earlier set up "catch all" expectations. For instance, by adding MatchFirst() and MatchLast() methods to Call which then indicate order preference.

A test might want to create the mock with default calls that match any args, any time, and that forward the calls to a real implementation, such as a Docker client. While it is possible to replace the default call expectations, it is impossible to replace a single default expectation with a series of expectations matching specific args (see below).

Context("handling Docker API errors", func() {

    var rec *MockClientMockRecorder
    var sess *Session

    BeforeEach(func(ctx context.Context) {
        ctrl := mock.NewController(GinkgoT())
        sess = Successful(NewSession(ctx,
            WithMockController(ctrl)))
        rec = sess.Client().(*MockClient).EXPECT()
    })

    It("silently handles API network list errors", func(ctx context.Context) {
        rec.NetworkList(Any, Any).
            MatchFirst().
            Return(nil, errors.New("error IJK305I"))
        // ...
    })
})

Why the feature is needed

WithOverridableExpectations always replaces a specific expectation for 《receiver, method》 with a single new one, making it impossible to replace an existing default expectation with a series of expectations.

(Optional) Proposed solution

  1. Adding a placement field to Call.
  2. Adding MatchFirst and MatchLast methods that set the placement of a call, much like the min and max settings.
  3. Modify callset.add to use Call.placement to either add at the beginning of the matcher list, at the end, or default (which seems to be at the end).
JacobOaks commented 4 months ago

cc: @r-hang

r-hang commented 4 months ago

Hey @thediveo,

Thank you for filing the issue. My understanding of the issue is that gomock queue will queue expectations by normal means. Under WithOverridableExpectations, this queuing behavior stops and expectation are overriden by each subsequent calls -- which means it's not possible to override with queuing (e.g. series of expectations) behavior.

While I can see how overriding expectations with queuing behavior does allow users to match for call placement I think that something like a MatchFirst or MatchLast to Call doesn't quite seem to fully address the functionality that would be possible if a user could reset/override a mock's expectation on demand and then continue queuing mock expectations.

Is my description above a sufficient understanding of the issue?

thediveo commented 4 months ago

@r-hang yes, that's also my understanding. The placement idea was initial, to get the discussion going. With some time passed, it doesn't look like correctly addressing the underlying situation.

Looking at the situation now, it could be a better solution to allow removing all previously set expectations for a particular receiver in order to register a fresh set of expectations. Kind of an override, but allowing for multiple chained expectations, where the existing override mechanism only allows for a single replacement expectation.

At the moment I'm working around the current situation by using optional args in the mock setup, where this args are the names of receiver methods that should not get default expectations. This is somewhat brittle, so I would highly appreciate a better way in the future.

For reference, here's my workaround: https://github.com/thediveo/morbyd/blob/4138a6913cae8105b41805515112e303773cec0c/client_test.go#L53 ... I'm using strings in lieu of typed constants which admittedly is more brittle than necessary.