Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate typed On_X functions to make updating mocks in tests faster #375

Closed

Conversation

dbravender
Copy link

This PR adds a new --generate-on-functions option which, if enabled, will generate typed On_ methods to make refactoring tests easier after an interface is changed.

Because mock.On accepts any parameters mismatching types aren't detected until runtime which can make refactoring tests after an interface has changed non-trivial. If you use the generated On_ functions with this change refactoring tests can be much faster because the arguments and return values are checked by the compiler.

Copy link

@lyoungblood lyoungblood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@LandonTClipp
Copy link
Contributor

I really like the idea and it's something that's been heavily requested, but I have rejected the idea on the grounds that it introduces feature creep. This issue is a characteristic of testify and I hesitate to start introducing features that bifurcate mockery into two distinctly different behaviors.

It would be more appropriate to introduce type safety into v3 where the default and only behavior is to be type safe. Otherwise I feel it's asking for trouble and creates a large maintenance burden down the line.

So yes I really like the idea and wish mockery was type safe, but I don't feel comfortable introducing it in the current major version.

@dbravender
Copy link
Author

Thanks for looking at this PR @LandonTClipp.

The reason I did this is because refactoring code and updating the corresponding tests with the dynamic On("FunctionNameAsAString"... is error prone and was consuming a lot of time. It felt like I was programming in a type-safe language but testing in a dynamic language since I have to run the tests to see if the updated mock calls are correct. When the mock calls are incorrect the errors from mock make it difficult to track down what the actual issue is. The cycle time is much faster if my editor and the Go compiler can clearly and immediately show me where the mistake is. I've been faster and have had a more pleasant time refactoring code that uses On_* functions in the tests.

I really like the idea and it's something that's been heavily requested, but I have rejected the idea on the grounds that it introduces feature creep. This issue is a characteristic of testify and I hesitate to start introducing features that bifurcate mockery into two distinctly different behaviors.

I understand the desire to not add more features to maintain. I intentionally made this behavior optional and disabled by default so only users who opt-in by specify --generate-on-functions will have the additional type-safe On_* functions generated. Given how much it's helped me I would love for others to have the chance to use this so I'd be willing to help maintain this feature.

It would be more appropriate to introduce type safety into v3 where the default and only behavior is to be type safe. Otherwise I feel it's asking for trouble and creates a large maintenance burden down the line.

I think bifurcating the behavior is inevitable in Go 1. If users want to continue to use mock-specific features such as mock.Anything or mock.AnythingOfType they will still need to use the dynamic On("FunctionNameAsAString"...) functions. I updated our tests to only use explicit calls with On_* functions and it wasn't too much work (outside of tracking down stretchr/testify#1063). Maybe generics in Go 2 will make it possible to have type-safe calls even with those mock placeholders.

@LandonTClipp
Copy link
Contributor

Thank you for your thoughts, I will give it some more consideration. If you are willing to stay active and maintain the feature that will help. I do see the clear value it would provide, so let me think it over for a bit.

@aljorhythm
Copy link

aljorhythm commented Jun 28, 2021

can we generate wrappers around .On ?

type SomeInterface interface {
    SomeMethod func(int) err
}
# generated

func (m *MockSomeInterface) On_SomeMethod(int) *Mock.call {
    return m.On("SomeMethod", int)
}

@dbravender
Copy link
Author

can we generate wrappers around .On ?

@aljorhythm The example you provided is nearly identical to what this PR adds.

@dbravender
Copy link
Author

#396 looks like it solves this problem in a more flexible way.

@LandonTClipp
Copy link
Contributor

I am going to close this in favor of #396 because I like that schema a lot more.

@dbravender dbravender deleted the generate-typed-on-functions branch September 19, 2021 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants