When it comes to "best practices" to mock interfaces, there's a number of things to consider, not in the least: ease of use. Over the years, I've taken to use a mock generator tool. Rather than just implementing the interface in question, GoMock supports a lot of other useful things. You can read more about it here.
The way to use it in your case would be something akin to this:
//go:generate go run github.com/golang/mock/mockgen -destination mocks/creator_mock.go -package mocks your.project.com/package Creator
type Creator interface {
CreateUser(User) (*User, error)
}
Then, in your project, just run go generate ./package/... to generate all mocks (ie all interfaces with a go:generate comment).
This will create a new package in your.project.com/package/mocks. In your unit tests, you just create mocks like this:
func TestCreateFail(t *testing.T) {
ctrl := gomock.Controller(t)
repo := mocks.NewMockCreator(ctrl)
arg := User{}
repo.EXPECT().CreateUser(arg).Times(1).Return(nil, SomeExpectedError)
obj := NewWhateverYoureTesting(repo) // pass in mock dependency
err := obj.CallYouWantToTest(arg)
require.Error(t, err)
ctrl.Finish() // checks if we received the expected number of calls
}
You can do quite a lot of more complex things with these generated mocks, like inject a custom function to control the behaviour of your mock in more detail, or add a callback to check the state of arguments, change the behaviour of the mocked function that is called based on how many times it is being called, or which arguments exactly are being passed:
func TestCreateSeveral(t *testing.T) {
ctrl := gomock.Controller(t)
repo := mocks.NewMockCreator(ctrl)
arg1, arg2 := User{ID: 1}, User{ID: 2} // 2 different users
// in case arg1 is passed, all goes well
repo.EXPECT().CreateUser(arg1).Times(1).Return(&arg1, nil)
// in case arg2 is passed, we return an error and check some fields
repo.EXPECT().CreateUser(arg2).Times(1).Return(nil, SomeError).Do(func(check User) {
// this callback will get the arguments that were passed to the repository mock
require.Equal(t, check.SomeField, "is equal to this value, set in the function we're testing")
})
obj := NewWhateverYoureTesting(repo) // pass in mock dependency
err := obj.CallYouWantToTest(arg1)
require.NoError(t, err)
require.Error(t, obj,CallYouWantToTest(arg2)) // now this should error
ctrl.Finish() // checks if we received the expected number of calls
}
I'd suggest moving the boilerplate code to set up the mocks and all to a function, and wrap everything in a test-type, just to keep your tests clean:
type testObj struct {
TestedObj // embed the type you're testing
ctrl *gomock.Controller
repo *mocks.MockCreator
}
func getTestedObj(t *testing.T) *testObj {
ctrl := gomock.NewController(t)
repo := mocks.NewMockCreator(ctrl)
return &testObj{
TestedObj: NewTestedObj(repo),
ctrl: ctrl,
repo: repo,
}
}
Then your tests look quite clean, really:
func TestCreateSeveral(t *testing.T) {
obj := getTestedObj(t)
defer obj.ctrl.Finish() // defer this to the end
// the test data
arg1, arg2 := User{ID: 1}, User{ID: 2}
// set up mock expectations & behaviour
obj.repo.EXPECT().CreateUser(arg1).Times(1).Return(&arg1, nil)
obj.repo.EXPECT().CreateUser(arg2).Times(1).Return(nil, SomeError).Do(func(check User) {
require.Equal(t, check.SomeField, "is equal to this value, set in the function we're testing")
})
// the actual test
err := obj.CallYouWantToTest(arg1)
require.NoError(t, err)
require.Error(t, obj.CallYouWantToTest(arg2))
}
All done.
What is quite useful to keep in mind that, when writing golang, it's considered good practice to define the interface alongside the type which depends on it, not next to the type(s) that end up implementing the interface. That way, everywhere something like a repository is used defines its own specific interface. The interfaces are minimal, and should only contain the methods the user will be using. An interface like you have:
type Repository interface {
Creator
Updater
Retreiver
}
looks a bit suspicious to my eye. This interface is meant to handle everything from creating, updating, and fetching data. Those are distinct operations that I prefer to have handled by separate components, so I'm not very likely to end up with a catch-all interface like this. In addition to this, because interfaces are defined along side the user, not the implementation(s), it's improbable for me to end up composing an interface is quite the same way, too. Because the code to handle creations/updates (in other words writes) and reads would be in different packages, I might end up with something like this:
package foo // read package
//go:generate go run github.com/golang/mock/mockgen -destination mocks/repo_mock.go -package mocks your.project.com/foo Repo
type Repo interface {
GetUserByEmail(string) (*User, error)
GetUserByID(uint) (*User, error)
}
And:
package bar // writes
//go:generate go run github.com/golang/mock/mockgen -destination mocks/repo_mock.go -package mocks your.project.com/bar Creator
type Creator interface {
CreateUser(User) (*User, error)
}
//go:generate go run github.com/golang/mock/mockgen -destination mocks/repo_mock.go -package mocks your.project.com/bar Updater
type Updater interface {
UpdateUser(User) (*User, error)
}
// I'd probably only use this composed interface in my tests
//go:generate go run github.com/golang/mock/mockgen -destination mocks/repo_mock.go -package mocks your.project.com/bar Repo
type Repo interface {
Creator
Updater
}
Random comments:
Some other thoughts I had when I saw the code you posted
Context
I don't see your using the context package anywhere. When dealing with a repository, or a handle to a store of any kind for that matter, 99% of the time, the interface allows for a context argument to be passed. Because of the methods being CreateUser, and GetUserByEmail, I'm assuming you're handling requests of some kind. Requests all have a context associated with it. If the connection is closed, an expensive query ought to be cancelled, rather than it being allowed to continue. The request context will be cancelled in that case, and if you pass that context through to the repository (and eventually use it when hitting the store), that cancellation is propagated automatically. For REST API's, this usually doesn't make a huge difference, but when dealing with websockets (graphQL), or streaming (protobuf), it's important to do. Similarly, when your application boots, it's always a good idea to create an application context. In your main function:
ctx, cfunc := context.WithCancel(context.Background())
defer cfunc() // cancels the context when the application shuts down
You can call the cancellation function when you receive a TERM or KILL signal, or something unexpected happens. Cancelling the application context, again assuming you passed it through when establishing the connection to the store and any other external processes/services you're relying on), it will take care of closing the connections, and freeing up the resources in a clean and efficient way.
Your interface is a bit weird
When creating or updating, you're expecting the caller to pass in an object by value, and you're returning a pointer to the same type. I'm assuming your create function tries to insert the data, and returns nil, err or &userWithIDAfterInsert, nil. Why pass a copy of the data you're trying to insert? Why not simply do this:
type Creator interface {
CreateUser(context.Context, *User) error // added context
}
Then in the implementation itself, do something like this:
id, err := r.db.Insert(ctx, user) // assuming insert returns the ID, it's a uint, so I'm assuming a SQL-style auto increment primary key
if err != nil {
return err
}
user.ID = id
return nil
Same goes for your update function. The arguments that are being passed in represent the same thing as the thing you're returning. Why faff around with 2 copies of the same thing, if you can just do it all with the same object?
