gojuno / minimock

Powerful mock generation tool for Go programming language
MIT License
574 stars 38 forks source link

Skip argument check #89

Closed zcolleen closed 2 months ago

zcolleen commented 5 months ago

Hey @hexdigest ! This issue is related to https://github.com/gojuno/minimock/issues/30 . First of all thank you for minimock.AnyContext , i found it very useful. But sometimes you want to skip other arguments. I know i can do something like mock.NewMock(controller).Mock.Inspect(func(arg1 string) { assert.Equal(arg1, 1) } ).Return(...) but i find it annoying to check every argument with Inspect rather then just to skip one.

You were discussing about checkers and you found it too complex and not clean. My proposal is much easier, maybe we could just add SkipMockNameArg1Name to api, so it would look like mock.NewMock(controller).Mock.Expect().SkipMockArg1Name().SkipMockArg2Name().Return(...).

To make Expect(When) arguments cleaner, we could generate some stub variables like MockNameAnyArg1Name that will be default value of the type of the argument. Technically they will do nothing, using them will just make Expect code cleaner (that is my attempt to solve your concern about "you put into Expect not what you actually expect")

What do you think?

hexdigest commented 4 months ago

Hey @zcolleen,

I think rather than adding Skip heplers it's better to add Expect helpers, e.g.: mock.NewMock(c).SomeMock.ExpectParam1(p1).ExpectParam2(p2).Return()

The only problem I see here is that when you mock functions with no param names defined it wouldn't be very readable because minimock generates simple param names inferred from param types and if you name/rename params than your tests won't compile.

Alternatively we can just use numbers: mock.NewMock(c).SomeMock.Expect1(p1).Expect2(p2).Return()

But this is also not very readable.

zcolleen commented 4 months ago

To add some readability we could name it with ExpectParamTypeNameParam1 . For example if params type is context.Context we could name it with ExpectContextParam1 .

I will bring pr on this issue, if you dont mind

zcolleen commented 4 months ago

@hexdigest hey , i brought pr of this feuture https://github.com/hexdigest/gowrap/pull/82 (depends on https://github.com/hexdigest/gowrap/pull/82) . I wasnt able to make it as i said with paramTypeName because param could be variadic or doesnt have type name at all, so i did it as you suggested ExpectParam1 .

if you name/rename params than your tests won't compile

Your tests would compile even if you name/rename arguments because mock still implements mocking interface, the only problem i see is that if you rename interface param names and dont regenerate mocks than you have inconsistent ExpectParam function names but i actually dont think this is a big issue.