Implement proposed changes to generator

[dlwyatt]

Description

Allows a single provider function to be used which provides all of the return parameters for a mocked function, instead of needing to define a separate function per output. The older functionality still works as well.

Also adds a new RunAndReturn function to expecter calls to use this same type of provider, functionally equivalent to .Call.Return(providerFunc)

• Fixes Suggested improvement to "Return Value Provider Functions" feature #537
• Fixes mockery should allow returning values based on some custom logic #530

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Version of Golang used when building/testing:

• 1.11
• 1.12
• 1.13
• 1.14
• 1.15
• 1.16
• 1.17
• 1.18
• 1.19
• 1.20

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Updated generator_test.go and ran make test. Was not able to run make all on my system, something about the run-e2e.sh script didn't work for me. (I'm running Ubuntu in WSL on Windows 11, if it matters.)

I also ran the new build against some of my own test code, such as the samples in #537 , to make sure the generated code behaved as expected when go test was run.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[dlwyatt]

Note on the checklist; the first item says "My code follows the style guidelines of this project", but I haven't actually found any style guidelines in the repo. I tried to match the style of the code around where I made modifications.

I also didn't add any comments, as the sections of code that I was modifying already didn't contain any. I can add some comments around the new bits if you prefer.

[denizgursoy]

@dlwyatt Do you know whether it is going to be merged or know someone who we can ask if they are going to merge? Should we ask it to @LandonTClipp?

[dlwyatt]

No idea. 🙂 This is my first interaction on this project.

[LandonTClipp]

Hi folks, thanks for the contribution. I will have to review the proposed change over the coming days. I work a full-time job so I can't commit to a specific period but I do try my best so I hope to have at least a first pass review by the end of this week :)

[codecov-commenter]

Codecov Report

Base: 71.90% // Head: 72.59% // Increases project coverage by +0.69% 🎉

Coverage data is based on head (15f76b9) compared to base (8641a5b).
Patch coverage: 96.77% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #538      +/-   ##
==========================================
+ Coverage   71.90%   72.59%   +0.69%     
==========================================
  Files           6        6              
  Lines        1242     1277      +35     
==========================================
+ Hits          893      927      +34     
- Misses        305      306       +1     
  Partials       44       44              

Impacted Files	Coverage Δ
pkg/generator.go	94.19% <96.77%> (+0.23%)	⬆️
cmd/mockery.go	28.57% <0.00%> (+0.17%)	⬆️
pkg/outputter.go	35.38% <0.00%> (+7.79%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[LandonTClipp]

You know a part of me wants to stop using individual printf statements because it's difficult for me to understand what's going on. You have the right approach of trying to follow what the code historically did, but I think it'd be better for readability to use the printTemplate method instead. What do you think? Is that easily done or is printf a better fit here?

[dlwyatt]

I don't find either of them all that easy to follow, tbh. It's easier to look at the expected outputs in generator_test.go, and trust the tests to make sure all the templates and printf's are actually producing that.

[dlwyatt]

Comparing before and after on one of the test cases, this PR changes this:

// Put provides a mock function with given fields: path
func (_m *RequesterReturnElided) Put(path string) (int, error) {
	ret := _m.Called(path)

	var r0 int
	if rf, ok := ret.Get(0).(func(string) int); ok {
		r0 = rf(path)
	} else {
		r0 = ret.Get(0).(int)
	}

	var r1 error
	if rf, ok := ret.Get(1).(func(string) error); ok {
		r1 = rf(path)
	} else {
		r1 = ret.Error(1)
	}

	return r0, r1
}

into this:

// Put provides a mock function with given fields: path
func (_m *RequesterReturnElided) Put(path string) (int, error) {
	ret := _m.Called(path)

	var r0 int
	var r1 error
	if rf, ok := ret.Get(0).(func(string) (int, error)); ok {
		r0, r1 = rf(path)
	} else {
		if rf, ok := ret.Get(0).(func(string) int); ok {
			r0 = rf(path)
		} else {
			r0 = ret.Get(0).(int)
		}

		if rf, ok := ret.Get(1).(func(string) error); ok {
			r1 = rf(path)
		} else {
			r1 = ret.Error(1)
		}
	}

	return r0, r1
}

And adds this to the expecter calls:

func (_c *RequesterReturnElided_Put_Call) RunAndReturn(run func(string) (int, error)) *RequesterReturnElided_Put_Call {
	_c.Call.Return(run)
	return _c
}

[LandonTClipp]

I think we can merge this but I would like to use the printTemplate method instead just for maintainability/readability sake. If you can make that change then I'll merge this PR. This is a really good change but that's my only ask at this point. Everything else is good.

[dlwyatt]

I can try, but that's a heck of a lot more work to rewrite more of the code than what I've modified so far, and getting templates to produce exactly the same output might be pretty annoying.

[LandonTClipp]

This is great! Sorry, if I knew how much of a pain it would be maybe I could have let it slide but I do really enjoy the templates a lot more. At least to me, it's easier to reason about.

I'll ack this, lmk if there are any finishing touches you want to make before I merge.

[dlwyatt]

I need to walk away for a bit and look at it with fresh eyes later, maybe in the morning. I'll let you know if I spot anything that could use work then.

[LandonTClipp]

Sounds good, thanks for the stellar work. It's really appreciated.

[dlwyatt]

I think this is good to go. There was one function comment that was sort of inaccurate after I changed things, but I think the code itself is okay.

[LandonTClipp]

Great work, I’ll take one last look tonight and merge if everything is good. Thanks Dave.