Design Proposal: integration of `github.com/matryer/moq`-style mocks

[LandonTClipp]

matryer/moq#208 was submitted to the maintainers of matryer/moq suggesting integrating the moq-style mocks into mockery. This suggestion seems well-received due to the benefits we could gain from combining the two projects' strengths.

This GitHub issue serves as a design proposal, discussion, and feature tracking page.

Background

vektra/mockery style of mocks rely on argument matching behavior provided from testify/mock. Expectations are created by mapping argument values to return values and side effects. Many valid criticisms have been raised about various pitfalls of this kind of expectation system. There are various alternate strategies for defining mock objects, and it's the goal of this project to combine the strengths of these alternate strategies with the strengths implemented in mockery itself.

mockery benefits

Speed

Mockery has undergone a huge rework over the last year with the new packages feature. Historically, mockery (and basically all other mock implementations) relied heavily on using go:generate statements to generate one mock per instantiation of mockery. This was extremely inefficient because it meant the packages.Load call required to parse the AST had to parse the package and all of its dependencies many times. This was an O(n*m) operation (where n is the number of mocks being generated, and m is the number of dependent packages). The rather trivial insight was to simply collect all packages to parse within a single list and pass all of them into packages.Load at once, which removed m from the equation. The new configuration model allows mockery to have this singular list of packages available within a single process.

Configuration inheritance

The other benefit of the configuration model is that allows inheritable configuration. Configuration can be specified at the top-level defaults, at the package level, and at the interface level, which parameters being inherited at each level. This is a powerful way to differentiate mock generation based on individual needs.

Templating

The configuration also takes advantage of Go templating, which allows users to call upon variables that are only known at runtime. Templating functions provide more power and flexibility to users to alter template variables.

mockery criticisms

1. This type of mocking encourages increased coupling between tests due to the fact that it's easy to fall into a habit of asserting how the mock was called, which is bad practice (it asserts implementation details).
2. Despite the great effort in adding type-safe expectation methods (Add mock generation with expecter #396), the necessity of specifying mock.Anything in mock objects means that the expecter's argument types have to be any. This means that argument types are not a compile-time safety guarantee but rather runtime.
3. There is a huge amount of logic in testify that goes into correctly mapping method calls to their corresponding expectations. While testify does its job perfectly well, its clear that the complicated details of argument-matching necessitates a huge amount of logic to support it. A different mocking model could greatly simplify the generated code.

matryer/moq benefits

Simplicity

These style of mocks provide an incredibly simple way of defining mocks: you define the function to be run for your mock call. No argument matching, no assertions on how the mock was called. This simplicity encourages the programmer to not do too much with the mock, which reduces coupling. It doesn't make it impossible to do nasty things like call count and argument value assertions, but it makes it somewhat harder to do.

Type safety

moq objects are strictly type safe, which means test compilation will fail immediately if your mock doesn't satisfy the interface no matter what.

matryer/moq criticisms

Speed

Like all other mock projects besides mockery, moq relies on go:generate to instantiate one moq process per mock object. This is highly inefficient for the reasons aforementioned.

Configuration model

Its configuration model is entirely CLI parameter based. There are no config files, no inheritable config, no use of Go templating.

Proposal

Configuration syntax

moq-style mocks can be easily called upon in mockery by yaml such as this:

with-expecter: true
all: true
packages:
    github.com/my/repo:
        config: 
            style: moq
    github.com/my/other-repo:
        config: 
            style: mockery # this would be the default

Of course, style could be specified anywhere: at the top-level (defaults), in an environment var, in the CLI parameter, at the package level, or at the interface level.

Generated code

To maintain backwards compatibility with all existing usage of moq, the generated code would be identical to that of github.com/matryer/moq, minus any necessary differences to comments within the mock file. For those unfamiliar, this provides an example of the mock moq generates.

Given:

// Person represents a real person.
type Person struct {
	ID      string
	Name    string
	Company string
	Website string
}

// PersonStore provides access to Person objects.
type PersonStore interface {
	Get(ctx context.Context, id string) (*Person, error)
	Create(ctx context.Context, person *Person, confirm bool) error
}

This is generated.

type PersonStoreMock struct {
	// CreateFunc mocks the Create method.
	CreateFunc func(ctx context.Context, person *Person, confirm bool) error

	// GetFunc mocks the Get method.
	GetFunc func(ctx context.Context, id string) (*Person, error)
...

You then define each function individually in your test as such:

func TestSomethingThatUsesPersonStore(t *testing.T) {

    // make and configure a mocked PersonStore
    mockedPersonStore := &PersonStoreMock{
        CreateFunc: func(ctx context.Context, person *Person, confirm bool) error {
            panic("mock out the Create method")
        },
        GetFunc: func(ctx context.Context, id string) (*Person, error) {
            panic("mock out the Get method")
        },
    }

    // use mockedPersonStore in code that requires PersonStore
    // and then make assertions.

}

File placement

Where the mock files are physically placed will be according to the dir parameter, just as in the current system.

Code implementation

mockery has a concept of an Outputter, which is the object that determines where a mock should physically reside, and a Generator which creates the mock code. The Outputter should be modified to take an interface value in its constructor:

type CodeGenerator interface {
    io.WriterTo      // writes the generated mock to a writer (typically a file)
    Generate() error // generates the mock in-memory
}

Both mockery's current implementation, and moq, would implement this interface. The outputter will be provided the proper implementation here based off the style specified.

packages.Load changes

moq currently calls packages.Load based off filesystem path: https://github.com/matryer/moq/blob/c59089b2cd89d975f3d44924f04f37591fbd701d/internal/registry/registry.go#L31

This will need to be modified to integrate with the Interface struct that contains the AST information about each interface discovered in the package. This struct is generated using the information returned from here.

[breml]

Hi @LandonTClipp
Thanks for the proposal and the solid preparation work. I am not yet familiar with the ins and outs of mockery, so I am not yet able to comment on all the aspects of the proposal. But still let me share some thoughts:

It doesn't make it impossible to do nasty things like call count and argument value assertions, but it makes it somewhat harder to do.

As a experienced user of moq, I do not agree with this observation. Count assertions are extremely simple, e.g. len(mockedPersonStore.CreateCalls()) for the mock in the proposal. For argument assertions, it is my observation, that the impact of the arguments on the test result is in most cases less important than the result, that is returned from the mock. That being said, if a mock is just called once (per test execution) again, it is straight forward to have the necessary assertions in the function body of the mocked function. The big advantage in this case of moq is, that the assertions look the same as all the other assertions (in contrast to the clumsy for, that is required by testify with EXPECT().
Maybe the most "challenging" thing for most users that work with moq generated mocks are the proper handling of multiple calls to the same mock in a single test case. For this, the approach with queues served us well (see matryer/moq#187 (comment), unfortunately, this tip did not yet make it into the README.md).

The other benefit of the configuration model is that allows inheritable configuration. Configuration can be specified at the top-level defaults, at the package level, and at the interface level, which parameters being inherited at each level. This is a powerful way to differentiate mock generation based on individual needs.

and

Its configuration model is entirely CLI parameter based. There are no config files, no inheritable config, no use of Go templating.

I think this critic is debatable as well. As long as there is no need for just an other config file, that pollutes my repository, since everything that is needed to generate the mocks fits into the go:generate line is a good thing in my opinion. It gets even worse in my opinion, if the config files are spread through out the repository and are then combined in some sort of Configuration inheritance, since this can hurt maintainability. In regards to inheritance in config files, it might be worth to have a look at cue and the reasoning behind, why cue does not have any sort of inheritance (source, e.g. core developer Marcel van Lohuizen in "Override-style inheritance is the biggest source of complexity in configuration." in https://www.coss.community/cossc/ocs-2020-breakout-marcel-van-lohuizen-google-17i0).

style

In regards to the possible values for the configuration attribute style, I wonder if it does make sense to have the "product" names there or if we should try to find terms, that describe best the two different styles of mocks.

Generator

I wonder, if the generator could be made "generic" in the sense that it just uses Go templates again to generate the final result. Similar to how it is done in https://github.com/hexdigest/gowrap. Basically mockery would just collect the necessary information (state), that is needed to generate the mocks and pass this to the Generator, which then basically loads a Go template where the final result is generated based on the state that is passed.

I hope, this thoughts are helpful for you.

[LandonTClipp]

I am not yet familiar with the ins and outs of mockery, so I am not yet able to comment on all the aspects of the proposal.

I myself am not super familiar with moq so it's great to have you in this discussion :D

Maybe the most "challenging" thing for most users that work with moq generated mocks are the proper handling of multiple calls to the same mock in a single test case. For this, the approach with queues served us well (see matryer/moq#187 (comment), unfortunately, this tip did not yet make it into the README.md).

I was a bit confused as to why this would be a "challenge," but what you suggested (either an external counter or a queue of return values) is what I thought would be done. In fact, the reason why I like that method more is precisely because it's ugly/manual. I've found it to generally be (like 80% of the time) bad practice, so I feel the mock framework should discourage it by default due to the increased coupling it creates. The main criticisms that have been raised is that mockery encourages this sort of coupling due to how easy it is to say .EXPECT().Foo(1).Return(1).Once().

I think this critic is debatable as well. As long as there is no need for just an other config file, that pollutes my repository, since everything that is needed to generate the mocks fits into the go:generate line is a good thing in my opinion.

I agree, on the surface this is a matter of taste. I prefer centralized config files, some people prefer go:generate. In this case, our hand is forced because go:generate was in fact the sole reason for poor performance. So in mockery, we'll never go back to recommending go:generate. It's a trade off that I feel is worth it for the 5x increase in performance I've measured.

It gets even worse in my opinion, if the config files are spread through out the repository and are then combined in some sort of Configuration inheritance, since this can hurt maintainability.

This had been suggested, but I rejected the idea on the grounds that it didn't seem like a highly requested feature and there are ways around the lack of inheritable config files. So as it stands, the only inheritance is your typical 3 sources of environment, CLI, and config files (and of course the mappings within the config file itself).

In regards to the possible values for the configuration attribute style, I wonder if it does make sense to have the "product" names there or if we should try to find terms, that describe best the two different styles of mocks.

You are probably right. Maybe: expectation and function-attribute? Open to other ideas.

I wonder, if the generator could be made "generic" in the sense that it just uses Go templates again to generate the final result. Similar to how it is done in https://github.com/hexdigest/gowrap.

That's a great idea. We could probably structure a new kind of generator that does just that. Unfortunately it would be very hard to port over the current Generator that creates expectation mocks because it has a lot of in-memory Sprintf-s that construct various parts of the mock. I've been pushing aggressively to move over to a singular template string but that work isn't completed yet. Regardless, we should 100% design it in this way. The other really cool thing about this idea is that we could allow users to define their own templates to create their own style of mocks. At that point, mockery becomes this sort of meta-mocking thing and opens up the door for lots of interesting paths.