-
Notifications
You must be signed in to change notification settings - Fork 405
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
Omitting .Return()
on an expectation results in a panic and a stack trace rather than a useful error message
#729
Comments
This is a totally fair criticism, and I think it'd be easy to fix. Mockery asks testify for the return values using I'll mark this as |
Hi there @LandonTClipp! I'm new here and I'd like to give it a try. If so, can you assign it to me? Thanks! |
Hi @mateusmarquezini, thanks for volunteering. Sure I'll assign to you. If you need any pointers to get you going, even if where to look in the code base, I'm glad to help. |
Awesome @LandonTClipp! Thank you! |
Hey @LandonTClipp! I was walking through the code base trying to find where is the exact point where we should fix this, and I guess this is the place, however, the code is generated by mockery so we can't edit it. So I'm wondering now where the block of code is generated. I appreciate any help! |
Hi @mateusmarquezini, the code that generates the The string that contains the You can see that in the template used to generate the output file, there are two main cases. In one case, the method is checked to see if it has any return values in the signature, as seen here. In the second case, our method has at least one return value, so the I'm guessing that the solution here will reside 100% within that template string. So it'll be something like:
You'll also want to create a test for this by creating a new "fixture" here: https://github.com/vektra/mockery/tree/v2.37.1/pkg/fixtures, which can be something as simple as
Then, direct mockery to generate a mock for this interface in .mockery.yaml (which will output to the
|
Thank you for your amazing explanation, @LandonTClipp! It's clear now what needs to be done! Hopefully, I'll submit a PR yet this week. |
Description
Omitting the
.Return()
on an expectation results in a panic deep in a transitive dependency that does not indicate what the problem was or what the resolution might be.Mockery Version
v2.36.0
Golang Version
go version go1.21.3 darwin/arm64
Installation Method
Steps to Reproduce
When setting an expectation on a function call defined thus:
It will look something like:
(note missing
.Return(nil)
at the end of theEXPECT()
, an easy mistake to make.)Expected Behavior
In this situation I would expect an error message saying something like "Required
.Return(error)
not specified onEXPECT()
call forDoThing()
invocation on line xxx".Actual Behavior
I get a panic and a stack trace from a transitive dependency (not mockery) that gives me no idea what the error could possibly be or how to fix it:
I realise this bug is possibly testify's "fault", but I didn't install testify, I installed mockery, and not being deeply into the Go ecosystem it took me a while to work out what the relationship is.
The text was updated successfully, but these errors were encountered: