Ignore some arguments

[rekby]

As first thanks for your program. Sorry for my english.

Is any way to ignore some arguments in Expect?

for example: i want mock method, that will receive *http.Request.

And I want check for this argument will not nil, and nothing more.

Or many method receive context.Context argument and, but context may renew internally (for example add value or timeout) and it will not same object, that I pass to checked method.

Now i see only way - Set function and do manual create function budy.

I want some syntax like: transport.RoundTripMock.When(&http.Request{}).NotNil(0).Then(&http.Response)

for internally mock method will check about first argument not nil and ignore real value, that I passed.

And few functions like NotNil:

• NotNil
• Any
• CheckBy - for function, which will check only that arg and bool result.

And other parameters will work as now: return result, when receive expected parameters.

Or syntax like: transport.RoundTripMock.When(&http.Request{}).Adv(NotNil(0)).Then(&http.Response)

[hexdigest]

Hi @rekby

I'll think about it but what I can say for sure is that the things you asking for require interface{} arguments which is not going to happen in minimock. Because one of the main features of minimock is that all generated code is statically typed and being type checked by the compiler and being auto-completed by IDE and editors and being analyzed by linters and so on and so forth.

What can be done here is special helper for each argument like: transport.RoundTripperMock.WhenRequest.NotNil.Then(&http.Response{}, nil) or transport.RoundTripperMock.WhenRequest.AnyOf(&http.Request{1}, &http.Request{2}).Then(&http.Response{}, nil)

But this is a lot of work and I think after we have that It won't be enough for you anyways :) So why not just use Set() ?

Regarding the context.Context I also want to remind you that context.Context is an interface which can be mocked so you alter context with WithValue/WithDeadline/WithTimeout to return you some implementation that you can wait in Expect() helper and have much better unit test in result.

I also don't think that assertion for non-nil Request in round tripper is a good idea. Because it means that you didn't check for the error when creating http.Request and instead there should be a test for handling this error correctly.

[rekby]

Hi, @hexdigest .

for example code:

package main

import (
    "context"
    "testing"
    "time"
)

type Summer interface {
    Sum(ctx context.Context, a, b int) int
}

func limitedTimeSumm(ctx context.Context, summer Summer, a, b int) int {
    ctx, _ = context.WithTimeout(ctx, time.Millisecond)
    return summer.Sum(ctx, a, b)
}

func TestLimitedTimeSumm(t *testing.T) {
    summerMock := NewSummerMock(t)
    summerMock.SumMock.Set(func(_ context.Context, a int, b int) (i1 int) {
        if a == 1 && b == 2 {
            return 3
        }
        if a == 2 && b == 3 {
            return 3
        }
        panic("unexpected")
    })

    res := limitedTimeSumm(context.Background(), summerMock, 1, 2)
    if res != 3 {
        t.Error(res)
    }
}

I can't use when, then syntax - becouse mock always receive context other, then I pass to func limitedTimeSumm.

And I don't real need value of context in mock. In my case I use context for logging and context pass to every method. Sometime method add values to context (request_id, some detail for repeat in next log lines - for better read log) and pass modified context to outer functions. This is blocker to use When/Then syntax. And need write full mock functions for every mocked method (a lot of boilersplate code).

Condition for only one argument I think not comfort because in example above: it will defficult mock to combinations a=1,b=2 and a=1,b=3.

I propose some method, which will additional options for every When condition. It can be statical generated method with same argument count, which in when method and must have option for every argument (or skip method call).

Or it can receive variadic description fields - for apply arguments for only need args.

As variant: it can generate arguments as structs For example: SummerSumCtxArgParam and use it:

summerMock.SumMock.When(nil, 1, 2).Adv(SummerSumCtxArgParam{Ignore:true}).Then(3)

SummerSumCtxArgParam will implement some interface for example MinimockAdvParam or SummerSumAdvParam and Adv method will receive variadic of this interfaces instead of interface{}

[deadok22]

We could look into introducing match funcs. Currently the function for matching the expectation is deep equality of all arguments.

The API may look like:

expectedRequest := func(r *http.Request) bool { ... }
transport.RoundTripperMock.When(....).WithArg0Matching(expectedRequest).Then(...)

Or:

expectedRoundTripperArgs := ... // a func returning bool that matches all args in some way
transport.RoundTripperMock.WhenMatches(expectedRoundTripperArgs).Then(...)

Or we could employ fluent interfaces to specify matchers for all args in order:

expectedWhence := func(whence int) bool { ... }
seeker.SeekMock.WhenArgs().OffsetIs(42).WhenceMatches(expectedWhence).Then(...)

That will likely make the mocks considerably bigger, though. So it's probably worth adding a separate option for enabling this behavior.

So far I guess this is the only thing I miss comparing to mockery.

And +1 to keeping things statically typed. It's what I like about minimock the best.

[hexdigest]

@deadok22

This

expectedRoundTripperArgs := ... // a func returning bool that matches all args in some way
transport.RoundTripperMock.WhenMatches(expectedRoundTripperArgs).Then(...)

is not very much better than Set:

transport.RoundTripperMock.RoundTripMock.Set(func(r *http.Request) (*http.Response, error) {
  //implement whatever complex matching behaviour you want here
})

@rekby

1. I don't think that with this notation summerMock.SumMock.When(nil, 1, 2).Adv(SummerSumCtxArgParam{Ignore:true}).Then(3) you'll get less boilerplate code than with simple Set() which allows you do do literally anything.
2. Can you provide an implementation of the mock you would like to see by altering current implementation of the generated code? Compare how it reduces the boilerplate and then I'll think how can we generate it.

[deadok22]

@hexdigest I'm not saying it's very convenient, but it's a lot better than Set: it allows to leverage expectations.

With Set one has to implement assertions around things like invocation count themselves, implement different behaviors on different inputs, etc. One other important use-case that API would cover is asserting arguments' properties like "the first arguments' field X matches match the 3rd argument of the same invocation".

[rekby]

Can you provide an implementation of the mock you would like to see by altering current implementation of the generated code? Compare how it reduces the boilerplate and then I'll think how can we generate it.

Yes, I can. It can take few days.

[rekby]

@hexdigest Example: https://gist.github.com/rekby/edc062095eca7ef7dc99be17d6717466

On page: https://gist.github.com/rekby/edc062095eca7ef7dc99be17d6717466/revisions You can see my changes in mock file.

[rekby]

This interface with user dfained comparer function let custom check of arguments.

For example - when I send *http.Request to mock of httpclient - I want check only few parameters (url, method etc), and I can set these parameters in want object and custom comparer. And I don't need custom full request.

[rekby]

Complex work with some params (ignore or global check for valid: valid context, not nil, etc) allow check if all expected calls of mock did. With SetFunc it need additional code.

[rekby]

I need it for test my project https://github.com/rekby/lets-proxy2/ Near every method contain context param - for logging with query context, stop work by timeouts and etc.

Will you OK if I add this feature myself by PR?

[hexdigest]

I'll be able to review it and respond not earlier than 12 of may

чт, 2 мая 2019 г., 11:40 Timofey Koolin notifications@github.com:

Will you OK if I add this feature myself?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gojuno/minimock/issues/30#issuecomment-488592943, or mute the thread https://github.com/notifications/unsubscribe-auth/AALEA6H4MA7ID4GQ7PNXPSDPTKSGXANCNFSM4HH5KTVQ .

[ruz]

Hello, we also want similar thing, but our idea to address this issue is very different. Our points are:

• we really like that XxxMock.Expect(...) has the same signature as Xxx function and we don't want to lose this
• most of our problems come from context.Context as first argument
• thus our developers just don't check arguments of calls
• overriding XxxFunc ( XxxFunc = func (...)) to check arguments is too wordy and less declarative, less readable
• we believe that for each mocked function we can implement one custom arguments "comparer"
• if we can not write one "comparer" then we can use XxxMock.Set(...) or XxxFunc = func (...)

So our idea is that it would be really cool to be able to setup checker:

XxxArgumentsChecker = func (t *testing.T, expected XxxArgumentsStruct, got XxxArgumentsStruct) (bool) {...}

This function will be called instead of DeepEqual.

What do you think?

[shoenig]

@ruz If I understand correctly, in the typical case where you just want to ignore a context.Context argument that appears first, you'd be providing an implementation that is something like,

// pretend our func has signature (a context.Context, b string, c Something)

XxxMock.XxxArgumentsChecker = func(t *testing.T, exp XxxArgumentsStruct, got XxxArgumentsStruct) bool {
    // ignore ctx (Arg0)

    // compare a
    if !exp.Arg1.DeepEqual(got.Arg1) {
        return false
    }

    // compare b
    if !exp.Arg2.DeepEqual(got.Arg2) {
        return false
    }

    return true
}

[ruz]

Yep. Very close. It's even questionable that the function should have return value or not, it can be:

XxxMock.XxxArgumentsChecker = func(t *testing.T, exp XxxArgumentsStruct, got XxxArgumentsStruct) {
    // ignore ctx (Arg0)
    exp.Arg0 = nil
    got.Arg0 = nil

    // compare the rest, require or assert or whatever
    require.DeepEqual(t, exp, got)
}

This also allows you to compare complex values partially, for example we can compare some fields in the context.

[rekby]

I like idea with ArgumentChecker func. I think it will good if return value will one of values: minimock.DefaultChecker, minimock.Accept, minimock.Reject:

minimock library.go

type CheckerResult int
const (
   DefaultArgChecker CheckerResult = iota
   CheckerAccept
   CheckerReject
)

then

XxxMock.MyFuncArgumentsChecker = func (got *MyFuncArgumentsGot, exp *MyFuncArgumentsExpected)(result minimock.CheckerResult){
  if got.arg1 == 1 {
    return minimock.CheckerAccept
  }
  if got.arg2 < 0 || got.arg2 > exp.arg2 {
    // reject function
    return minimock.CheckerReject
  }
  if exp.Ctx == context.Background() && exp.Ctx != nil { // ignore exist context
    // force modify argument after custom check for accept by default checker and use default checker for other params
    got.Ctx = context.Background()
  }
  // default result will zero value and default checker will call
}

and I want different type for Got and Expected params structs - for prevent mistake of it placement.

[ruz]

I'm ok with either syntax, some may be impossible or impractical.

Not clear how to define these functions. I believe setting them for every instance would be too wordy. Do you agree?

On the other hand. If you declare all checkers for a mocked interface in one place far from test code then it may become not obvious.

Wonder how to get approve of concept from @hexdigest .

[hexdigest]

Hi everyone!

@ruz I definitely like the concept of "checkers" more than previously proposed solutions to this problem (well similar solution with "matchers" was proposed by @deadok22 ). But I'm still in doubt about the benefit of using checkers comparing to the Set helper.

Let's look at the code. Set goes first:

mock.DoMock.Set(func(ctx context.Context, arg1, arg2 string) (Result, error) {
  //Do whatever you want here
  //you can even return Result that depends on arg1/arg2
return Result{}, nil
})

Using "checker" it will be something like:

mock = mock.DoMock.WithArgumentsChecker(func(got, want *DoFuncArguments) error {
  //You can only compare arguments here
  //No way to return some result
})
mock.DoMock.Return(Result{}, nil) //we still need something to return, right?

Is it worth it? I'm not even talking about the fact that you can easily create an instance of expected Context in the beginning of your test and pass it to Expect(expCtx, arg1, arg2) to get a higher quality test. This is what I as a user of minimock personally do and don't have any problems with this approach.

However there's still a "problem" of checking only some parameters of http.Request mentioned by @rekby but honestly I think that it's just a matter of good code composition and writing tests that actually test something.

Also bringing the concept of "checkers" raise some questions like:

1. Should checker affect When/Then. If checker ignores Context and I pass some mocked context.Context interface to When(expCtx) should When match in this case or not?
2. If checker is set but there were no calls to the method should we fail such test?
3. If checker is set but there's also a call to Set() for the same method what should we do?

Finally what I like about current implementation is that it's pretty straightforward:

1. Use Expect/Return/When/Then when you either want a good test or when you're testing simple funcs
2. Use Set when you want to cut some corners or when you want some sophisticated mock behavior.

What people ask in this thread is a better way to cut corners. Maybe instead we should update documentation with some real examples of handling context.Context arguments or mocking httpclient.Do metod?

[hexdigest]

If you guys still insist that Set is too wordy I'm ~ok with the following "checker" signature: func(want, got *DoFuncArguments) error

This will allow us to have:

1. standard output from minimock
2. one structure for got/want (I don't like @rekby's proposal about two identical types with different names)
3. order of the want/got args is the same as in famous stretchr/testify toolkit.

[rekby]

However there's still a "problem" of checking only some parameters of http.Request mentioned by @rekby but honestly I think that it's just a matter of good code composition and writing tests that actually test something.

Not composition only. For example - I want test, which call func with some internal timeout (and change context for next functions - example with limitedTimeSumm above) or add some details for logging and save logger to context. I can't pre-create context out of functions because it will different objects and will not match with standard comparer.

[rekby]

My be add default version of mocked function to call from set?

obj.DoMock.Set(func(ctx context.Context, args)(results){
if ctx != nil {
  ctx = context.Background()
}
return obj.DoMock.Default(ctx, args) // first variant
return ObjMockTypeDoMockDefault(obj, ctx, args) // second variant
}

It will allow:

1. Do own checkrs with higher priority
2. If add function similar to IsMatched - for check arguments without call count, return result, etc - allow do self checkers with lower priority
3. Ignore some arguments and allow use When/Then logic - for other arguments

[ruz]

Your example with checker is slightly incomplete, it will look like this:

mock = mock.DoMock.WithArgumentsChecker(DoChecker)
mock.DoMock.Expect(expCtx, arg1, arg2).Return(Result{}, nil)

Set() can be used, but it's not that declarative as Expect/Return pairs and other helpers minimock has. I'm also considering something like the following approach:

func MyChecker(
    t *testing.T, Result, error, arg1, arg2 string,
) (
    func(ctx context.Context, arg1, arg2 string) (Result, error),
) {
    return func....
}
mock.DoMock.Set(MyChecker(t,  Result{}, nil, "arg1", "args2"))

I find above a bit awkward. Ability to define Return after Set can be helpful:

mock.DoMock.Set(MyChecker(t, "arg1", "args2")).Return(Result{}, nil)

Can this be done? It would be very close to my original idea. Expectation series wouldn't work, but who cares.

We can not easily create instance of Context, it may have Jaeger spans with random ids, logger with fields attached and so on. It's painful to maintain such context in tests. Writing tests shouldn't be painful.

My answers to the questions: 1) when/then is in trouble, need thoughts. we don't use when/then because of Context :) 2) checker can not "answer" call so it doesn't mark method as expected to be called 3) Set will probably clear checker as functionality of two conflicts

We want better way to cut corners as now we cut them by skipping Set and Expect. This is whole blocks cutting, not just corners. Documentation is needs love for sure.

[hexdigest]

@rekby In your example you already used Set which you find wordy and hard to write. If you already used why do you need Default? Also using Default assumes that along with set you used When/Then or Expect/Return. So correct complete example would be something like:

obj.DoMock.Expect(nil, arg).Return(something, nil).Set(func(ctx context.Context, args Args)(result, error){
  return nil, errors.New("something went wrong")
})

Which I find highly unclear. Because how you can tell what actually is going on here what expectations and what are return params (especially if you don't call Default at the end of the closure)):

1. I used expect but it's not something I expect actually
2. I called Return but it's not something I'm going to return
3. I used Default but I don't know what

You can't do it currently which makes minimock behavior straightforward.

[hexdigest]

@ruz @rekby What if along with Expect we have Inspect helper:

mock.DoMock.Inspect(func(ctx context.Context, arg1, arg2 string) error {
  //Inspect arguments and return error if something is wrong
  return nil
}).Return(Result{}, nil)

or

mock.DoMock.Inspect(func(tester minimock.Tester, ctx context.Context, arg1, arg2 string) {
  //Inspect arguments and return error if something is wrong
  require.NotNil(tester, ctx)
  deadline, ok := ctx.Deadline()
  assert.True(tester, ok)
  assert.NotEmpty(tester, deadline)
}).Return(Result{}, nil)

?

[ruz]

As I said above I'm ok with Set() that can be followed by Return() call without overriding Set(). Looks like Inspect will do exactly this. Am I correct?

What's the purpose of return value (error)? Just sugar?

I don't think you should pass minimock.Tester inside Inspecting function. Users can pass closure over testing.T.

So far it looks good for my purpose, and I can implement re-usable checkers using the following code:

func DoChecker(
  t *testing.T, earg1, earg2 string,
) (
  func(gctx context.Context, garg1, garg2 string),
) {
  return func(gctx context.Context, garg1, garg2 string) {
    require.NotNil(t, gctx)
    require.Equal(t, earg1, garg1)
    require.Equal(t, earg2, garg2)
  }
}

mock.DoMock.Inspect(DoChecker(t, "foo", "bar")).Return(Result{}, nil)

This solves my issues.

[hexdigest]

@ruz

I'm ok with Set() that can be followed by Return() call without overriding Set().

Set can't be followed by Return because it leads to unclear behavior:

1. You can return result from Set
2. You set results with Return

It's not clear what results to expect here.

Looks like Inspect will do exactly this. Am I correct?

Not really, you could use Inspect with Return with no problems because minimock will ensure that you didn't use Set before or after Inspect like it does with When/Then/Expect/Return to avoid unclear behavior.

minimock.Tester is a wrapper around testing.T which protects testing.T from concurrent use and makes it concurrent safe (because closure that you pass to Set can be run concurrently from the tested code).

What's the purpose of return value (error)? Just sugar?

I wanted to propose one of the of the solutions actually not both. If we continue with the first option (where closure returns an error) then error will be handled by minimock and it will provide clear message why Inspect failed when running your mock.

If we continue with the second solution where minimock.Tester becomes the first argument to the closure then there's no need in error because you can fail test at any moment using tester object.

I personally like first solution better because func that we pass to the Inspect has exactly same arguments as the method we're mocking. However making assertions using other testing (like stretchr/testify in my example) frameworks won't be possible.

[ruz]

I love Inspect + Return very much. One thing to decide is signature of the func you pass as argument to Inspect.

I personally like first solution better because func that we pass to the Inspect has exactly same arguments as the method we're mocking. However making assertions using other testing (like stretchr/testify in my example) frameworks won't be possible.

I don't understand claim that "assertions using other testing frameworks won't be possible."

func TestSome(t *testing.T) {
  ...
  mock.DoMock.Inspect(func(ctx context.Context, arg1, arg2 string) {
    require.Equal(t, "some", arg1)
    require.Equal(t, "emos", arg2)
  }).Return(Result{}, nil)
  ...
}

What wrong with this?

I would really prefer arguments to match original function we mock (no tester object), return value to be empty (no error). I would use require check if I want test to stop after fail. I would use asserts if I want to continue execution.

What don't I see?

[hexdigest]

@ruz

What wrong with this?

The problem with this code is that if you test concurrent code and Do method is being called from within several goroutines then you may end with panic due to *testing.T methods are not concurrent safe. Such code requires some effort to be safe like wrapping testing.T with mutexes or using channels to send results of assertions. But in terms of readability in simple cases it's much better of course.

[deadok22]

I've built a prototype supporting the following syntax today:

//go:generate go run minimock.go -g -i Target -o practice/ -s _mock.go

type Target interface {
    Shoot(ctx context.Context, projectile string) error
}

func Test_Target_Practice(t *testing.T) {
    m := practice.NewTargetMock(t)

    const projectile = "projectile"

    ctx, cancel := context.WithTimeout(context.Background(), time.Second)
    cancel()

    m.ShootMock.
        When(
            m.MinimockArg.MatchedContext(func(ctx context.Context) bool {
                _, deadlineIsSet := ctx.Deadline()
                return deadlineIsSet
            }),
            projectile,
        ).
        Then(nil)

    require.NoError(t, m.Shoot(ctx, projectile))

    m.MinimockFinish()
}

This is similar to how Java mocking frameworks do things. The Matched.* funcs return zero values of the method parameter types and register the argument match func. Match funcs can be mixed with value specifications so long as there's no conflicts. We can trivially add Any.* funcs that'd just call corresponding Matched.* with a func accepting anything.

Here's what an implementation of When from the example above looks like:

// When sets expectation for the Target.Shoot which will trigger the result defined by the following
// Then helper
func (mmShoot *mTargetMockShoot) When(ctx context.Context, projectile string) *TargetMockShootExpectation {
        if mmShoot.mock.funcShoot != nil {
                mmShoot.mock.t.Fatalf("TargetMock.Shoot mock is already set by Set")
        }
        mm_m := TargetMockShootMatchers{}
        mm_e := TargetMockShootParams{}
        mmShoot.mock.MinimockArg.matchers.
                Expectation(mmShoot.mock.t, "TargetMock.Shoot").
                Next(ctx, &mm_m.ctx, &mm_e.ctx).
                Next(projectile, &mm_m.projectile, &mm_e.projectile).
                Done()
        mm_me := &TargetMockShootExpectation{mock: mmShoot.mock, params: &mm_e, matchers: &mm_m}
        mmShoot.expectations = append(mmShoot.expectations, mm_me)
        return mm_me
}

I plan to bring it to a fully working state, share it here so you all can try it out, and then provide a pull request after I'm done with tests and cleaning it all up.

Let me know what you think.

[ruz]

@hexdigest

What wrong with this?

The problem with this code is that if you test concurrent code and Do method is being called from within several goroutines then you may end with panic due to *testing.T methods are not concurrent safe. Such code requires some effort to be safe like wrapping testing.T with mutexes or using channels to send results of assertions. But in terms of readability in simple cases it's much better of course.

func TestSome(t *testing.T) {
  mc := minimock.NewController(t)
  defer mc.Finish()
  ...
  mock.DoMock.Inspect(func(ctx context.Context, arg1, arg2 string) {
    require.Equal(mc, "some", arg1)
    require.Equal(mc, "emos", arg2)
  }).Return(Result{}, nil)
  ...
}

Safer? Other wrapper can be used or some other means. Don't think it's concern of a mocking tool.

How do we move forward?

[hexdigest]

@ruz

Safer? Other wrapper can be used or some other means. Don't think it's concern of a mocking tool.

Yes it's safer. I agree that it's not a concern of the minmock.

How do we move forward?

If everyone is ok with suggested notation:

mock.DoMock.Inspect(func(ctx context.Context, arg1, arg2 string) {
    require.Equal(mc, "some", arg1)
    require.Equal(mc, "emos", arg2)
  }).Return(Result{}, nil)

I can implement it on a weekend.

[hexdigest]

@deadok22

m.ShootMock.
        When(
            m.MinimockArg.MatchedContext(func(ctx context.Context) bool {
                _, deadlineIsSet := ctx.Deadline()
                return deadlineIsSet
            }),
            projectile,
        ).
        Then(nil)

I don't really understand how it will work. In your example m.MinimockArg.MatchedContext will be calculated before the tested code

How ctx from m.Shoot(ctx, projectile) is going to be passed to the func argument of the m.MinimockArg.MatchedContex to perform a matching?

[deadok22]

Please see this branch in my fork. I haven't implemented support for all features yet, but basic When/Then works - please check it out. Usage examples can be found here.

[rekby]

@deadok22, hello. I saw your branch. I think the idea can work, with some changes:

1. MinimockArg need have functions like MatchCtx, MatchProjectfile, etc - for set matchers by argument name, becouse order of calc params before call When undefined + may be few arguments with same type.
2. Clean matchers after every When.

And I don't very like the idea becouse it is not trivial to understand how it work.

I more like variant with argument checker https://github.com/gojuno/minimock/issues/30#issuecomment-505097913

[deadok22]

@rekby not knowing the order of parameters before the When call is a given, but I tried to handle it the best I can - see a test case capturing this. Please let me know if there are cases I've missed.

Generating a struct per parameter name would be pretty wasteful - the mocks for large interfaces will be huge.

What do you mean by cleaning matchers after every when? I guess an assertion there's no matchers would be a good idea, though.

[hexdigest]

@ruz Please check out the new Inspect helper.

@rekby @deadok22

I'm still not convinced about any suggested approaches regarding matchers/checkers. Here are my thoughs:

1. Matchers are helpful only with Then helper
2. @deadok22 proposal with When(m.MinimockArg.MatchedContext... is not clear and I want to keep minimock clear and simple to use. More concretely I don't like that m.MinimockArg.MatchedContext returns something that When actually doesn't expect.
3. In general I don't like any proposals regarding per-argument matchers because it will be hard to explain these concepts in documentation and actually they don't save anyone from writing boilerplate code.

What we can do here is to introduce new WhenMatch helper that will accept a matcher func with bool result:

m.ShootMock.WhenMatch(func(ctx context.Context, r Request) bool {
  _, ok := ctx.Deadline()
  return ok
}).Then(&Result{}, nil)

@deadok22 what do you think about this kind of matcher func?

[deadok22]

More concretely I don't like that m.MinimockArg.MatchedContext returns something that When actually doesn't expect.

Yeah, that's not intuitive unless you're used to Java mocking frameworks that do exactly this.

The WhenMatch approach is inconvenient when you want to use deep equality matchers for all arguments but one, which is almost always the case for interfaces with a context.Context argument.

[hexdigest]

I agree that per-argument matchers can be helpful but I just don't see any clear approach to implement this functionality. With per-argument matchers there will be lots other problems and the main one is naming. If names of the matchers derive from arguments' names then we will have problems with methods where arguments names are not defined and this is pretty typical. Also if you already have some tests that utilize this kind of mock and then you decide to name the arguments you will break your tests after mocks are regenerated. If names of the matchers derive from argument's types then we will have all other sorts of problems like same types names defined in different packages or same types used for different arguments of the same method and so on.

The only clear and straightforward thing here is the order of arguments but it doesn't seem convenient in terms of the generated matchers's names like WhenMatch1().Then().WhenMatch2().Then()

[deadok22]

I gave it some more thought and I agree with you: neither parameter names nor their (static) types can be used to generate good mocks. That leaves us with an option of naming per-argument APIs using names like Arg0, Arg1, etc. By the way, we can still reuse code per argument type. I'll hack on this when I have time and share what I come up with.

[rekby]

I like WhenMatch WhenMatch https://github.com/gojuno/minimock/issues/30#issuecomment-512769739 from @hexdigest

[vlanse]

BTW, looks like separate case with context.Context is rather important. Given that golang function/methods has ctx argument very often and no one will guarantee that ctx will be always passed along as-is w/o any wrapping, current When/Then approach becomes impractical, and the only alternative is to use Set to skip ctx argument validation, but Set gives too much overhead in terms of lines of code, which means that test code become bloated with useless comparisons.

So, I suggest to add just one separate helper to pass "any context" to mock, something like this

o.MethodMock.When(minimock.AnyContext(), arg1, arg2).Then(ret1, ret2)

Fix could be rather trivial, IMO it will be enough to patch minimock.Equal() to achieve the result.

@hexdigest what do you think?

[hexdigest]

Hey folks,

Please check the latest v3.3.1. It has multiple issues fixed plus there's a minimock.AnyContext helper to solve the most annoying issue with When/Expect where one of the arguments is context. More details can be found in the updated documentation: Mocking context section.