vektra / mockery

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

Initialize/setup mock via a constructor param #784

Closed Thiht closed 3 weeks ago

Thiht commented 3 weeks ago

Description

Hi! Would you consider adding a way to setup the mocks directly via the mock constructor? I have something like this in mind:

Suggestion 1: with a variadic, to keep retro-compatibility

func NewFoo(t interface {
    mock.TestingT
    Cleanup(func())
}, setup ...func(*Foo)) *LeaveStore {
    mock := &Foo{}
    mock.Mock.Test(t)

    for _, s := range setup {
        if s != nil {
            s(mock)
        }
    }

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

    return mock
}

Suggestion 2: no variadic, assuming a new .mockery.yml configuration (something like with-constructor-setup)

func NewFoo(t interface {
    mock.TestingT
    Cleanup(func())
}, setup func(*Foo)) *LeaveStore {
    mock := &Foo{}
    mock.Mock.Test(t)

    if setup != nil {
        setup(mock)
    }

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

    return mock
}

Rationale

We're using table driven testing and declare our mocks in the following way:

tests := []struct {
    name   string
    fooExp func(s *mocks.Foo)
    // ...
}{
    {
        name: "test1",
        fooExp: func(s *mocks.Foo) {
            s.EXPECT().DoSomething().Return(nil)
        }
    },
    {
        name: "test2",
        // fooExp not needed -> zero value
    }
}

for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        foo := mocks.NewFoo(t)
        tt.fooExp(foo) // possible panic

        // ...
    })
}

This forces us to either:

  1. declare fooExp: func(s *mocks.Foo) {} everywhere even if we don't need it, because the zero value is nil
  2. add a nil-check in the t.Run around tt.fooExp(foo)

both of these are easy to do, bu easy to forget, and it's a bit of a hindrance on the whole table driven testing experience.

A constructor param solves the issue with no drawback, and it's in my opinion pretty in line with what a constructor should do:

tests := []struct {
    name   string
    fooExp func(s *mocks.Foo)
    // ...
}{
    {
        name: "test1",
        fooExp: func(s *mocks.Foo) {
            s.EXPECT().DoSomething().Return(nil)
        }
    },
    {
        name: "test2",
    }
}

for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        foo := mocks.NewFoo(t, tt.fooExp)

        // ...
    })
}

What do you think? I can make a PR quickly if you're ok with the idea.

Mockery Version

v2.43.2

Golang Version

go version go1.22.3 darwin/arm64

Installation Method

LandonTClipp commented 3 weeks ago

Why not just have fooExp be fooExp: func(t *testing.T) *mocksFoo that returns the instantiated mock with expectations already added?

foo := tt.fooExp(t)
Thiht commented 3 weeks ago

It's a pretty good solution that I did not think of (thanks for the suggestion, I'll probably use that if this doesn't get added!). I like that it keeps the mock initialization and setup together.

The only disadvantage I see is that it's a bit more verbose (2 more lines per expect function so if the table has >10 tests it can pile up) but I can live with that:

fooExp: func(m *mocks.Foo) {
    m.EXPECT().DoSomething().Return(nil)
}

// vs

fooExp: func(t *testing.T) *mocks.Foo {
    m := mocks.NewFoo(t)
    m.EXPECT().DoSomething().Return(nil)
    return m
}
LandonTClipp commented 3 weeks ago

That's great. FWIW add a nil-check in the t.Run around tt.fooExp(foo) is not a bad option in my opinion. This would probably be my preferred solution. But yeah, if there is an easy way around your problem, I prefer not to add to the mock objects.