vektra / mockery

A mock code autogenerator for Go
https://vektra.github.io/mockery/
BSD 3-Clause "New" or "Revised" License
5.93k stars 401 forks source link

unnecessary mock for type definition of func #716

Closed FLAGLORD closed 6 days ago

FLAGLORD commented 11 months ago

Description

When using the designing pattern of Option Pattern, there will be some code below:

// unexported option
type fooOptions struct{
  // some fields
  // ...
}

// Exported function type
type FooOption func(opt *fooOptions)

mockery will produce code below:

// FooOption is an autogenerated mock type for the FooOption type
type FooOption struct {
    mock.Mock
}

// Execute provides a mock function with given fields: opt
func (_m *FooOption) Execute(opt *common.fooOptions) {
    _m.Called(opt)
}

// NewFooOption creates a new instance of FooOption. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func NewFooOption(t interface {
    mock.TestingT
    Cleanup(func())
}) *FooOption {
    mock := &FooOption{}
    mock.Mock.Test(t)

    t.Cleanup(func() { mock.AssertExpectations(t) })

    return mock
}

It import a unexported struct. Is it a expected behavior? IMO, mockery should not produce a mock for fooOptions as it is not a interface.

Mockery Version

v2.34.0

Golang Version

1.20

Installation Method

Steps to Reproduce

  1. [First Step]
  2. [Second Step]
  3. [etc]

Expected Behavior

[what you expect to happen]

Actual Behavior

[what actually happened]

LandonTClipp commented 11 months ago

Mockery can mock plain functions. What's your config? I wouldn't say this is unexpected.

FLAGLORD commented 10 months ago

Thanks for your reply. This is my config:

with-expecter: false
all: true
filename: "{{.InterfaceNameSnake}}.go"
outpkg: mock{{.PackageName}}
mockname: "{{.InterfaceName}}"
dir: test/mock/{{trimPrefix .InterfaceDirRelative "pkg/"}}
packages:
  packageNameA:
    config:
      recursive: true

Is there any way to skip mock for plain functions?

Foxprodev commented 10 months ago

+1 It generates not working code for me and the only way to skip is enumerate each type name in exclude-regex

LandonTClipp commented 10 months ago

Semantically speaking, mockery is doing the correct thing as functions are capable of being mocked. However the issue is the use of an unexported struct as you mentioned. From mockery's perspective, this is expected behavior in the sense that you're asking it to generate mocks outside of the original package for a function that uses an unexported struct, which won't compile, but it's what you asked it to do.

The way to fix this in my opinion is either:

The first point should probably be added anyway, but it's a bandaid for this particular problem. The second point is the most generalizable. The third already works now but it's manually intensive. The issue is not specific to function types, you would run into the same problem if your interface referenced unexported types.

Foxprodev commented 10 months ago

Add a parameter to skip mocking functions

Maybe option to filter type signature would be more flexible. Like func.* to filter func types and interface{}|any to filter anonymous interface types or even interfaces containing something interface{.*SomeType.*}

kilianc commented 8 months ago

Mockery can mock plain functions.

where do I find this in the docs?

LandonTClipp commented 8 months ago

where do I find this in the docs?

https://vektra.github.io/mockery/latest/examples/#function-type-case

This is feature was made shortly after I took over the project but I feel like it was an unnecessary addition, and it doesn't seem like it was ever a widely-used feature anyway. While mockery can support this, I encourage people not to and instead just create your own function implementations.

The old pre-packages config semantics natively support function mocking but I never made an attempt to make it work for packages, and if it does work (like what this issue appears to be showing), then it was by accident.

iangregsondev commented 3 months ago

Hi @LandonTClipp ,

I was also having this issue, but something doesn't feel right. I have the following created in a top level go file, and it creates a mock, but if you notice, it is NOT exported.

type ringmenow func() error

According to the rules, I would have expected Ringmenow to be mocked as it is exported, but my func which is not exported (ringmenow) shouldn't be?

Is my assumption right ?

I will place the mock that is created below for visibility

Thanks in advance

Ian

// Code generated by mockery v2.43.1. DO NOT EDIT.

package svc

import mock "github.com/stretchr/testify/mock"

// Mockringmenow is an autogenerated mock type for the ringmenow type
type Mockringmenow struct {
    mock.Mock
}

type Mockringmenow_Expecter struct {
    mock *mock.Mock
}

func (_m *Mockringmenow) EXPECT() *Mockringmenow_Expecter {
    return &Mockringmenow_Expecter{mock: &_m.Mock}
}

// Execute provides a mock function with given fields:
func (_m *Mockringmenow) Execute() error {
    ret := _m.Called()

    if len(ret) == 0 {
        panic("no return value specified for Execute")
    }

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

    return r0
}

// Mockringmenow_Execute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Execute'
type Mockringmenow_Execute_Call struct {
    *mock.Call
}

// Execute is a helper method to define mock.On call
func (_e *Mockringmenow_Expecter) Execute() *Mockringmenow_Execute_Call {
    return &Mockringmenow_Execute_Call{Call: _e.mock.On("Execute")}
}

func (_c *Mockringmenow_Execute_Call) Run(run func()) *Mockringmenow_Execute_Call {
    _c.Call.Run(func(args mock.Arguments) {
        run()
    })
    return _c
}

func (_c *Mockringmenow_Execute_Call) Return(_a0 error) *Mockringmenow_Execute_Call {
    _c.Call.Return(_a0)
    return _c
}

func (_c *Mockringmenow_Execute_Call) RunAndReturn(run func() error) *Mockringmenow_Execute_Call {
    _c.Call.Return(run)
    return _c
}

// NewMockringmenow creates a new instance of Mockringmenow. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func NewMockringmenow(t interface {
    mock.TestingT
    Cleanup(func())
}) *Mockringmenow {
    mock := &Mockringmenow{}
    mock.Mock.Test(t)

    t.Cleanup(func() { mock.AssertExpectations(t) })

    return mock
}
arvenil commented 6 days ago

I'm trying to switch (again) to package config, but I get these mocks for functions generated out of blue. The old pre-package config didn't seem to generate them.

So you are saying they are not working with package config, yet somehow they get generated? While I never got them generated with pre-package config? Fishy.

LandonTClipp commented 6 days ago

@arvenil I'm adding https://github.com/vektra/mockery/pull/809 which will give you an option to disable function mocks entirely.

Adding this function mocking behavior to mockery was a mistake, does anyone really use it? I want to just remove entirely in v3.

LandonTClipp commented 6 days ago

v2.45.0 adds the config option disable-func-mocks which should clear up any issues you are having.

arvenil commented 3 days ago

Thanks! It works!