JosiahWitt / ensure

A balanced test framework for Go
MIT License
8 stars 0 forks source link

Mocks Path and Naming #77

Open heynemann opened 1 year ago

heynemann commented 1 year ago

Hey first of all thanks for this great mock library. I'm trying to port my mocks from mockgen to use ensure. I have two issues I need help with to do it:

  1. Mocks with Ensure do now allow me to overwrite the name of the interface, so if I have a Store that I want to create a mock it always creates a Store as mock, whereas I want to name it MockSomethingStore. This is important for me as we have conflicting names (two stores in two different services).
  2. I want to be able to specify the destination of the mocks. Instead of it creating a <package>_mocks within the internal destination I specified. Also would be great if there was an option to skip removing any mocks that are not directly controlled by ensure as this would allow me to partially migrate to ensure.

If those two make sense I can help implementing them :) Thanks for all the effort.

JosiahWitt commented 1 year ago

Thanks for the kind words!

If you'd like to retain mocks not generated by ensure mocks generate as you migrate, you could set tidyAfterGenerate: false in .ensure.yml. (See README for an example.)

In the spirit of the Go language, I'd like to avoid adding complexity to this project unless unavoidable. Today, ensure mocks generate avoids naming conflicts by scoping each package to its own mock package. For example, user.Store and account.Store would be generated in two different packages: mock_user.MockStore and mock_account.MockStore. I think that addresses your concerns about conflicting names. Is there a technical reason you need to have all your mocks in a single package? Perhaps you could achieve something similar using type aliases?

For example:

package mocks

type MockUserStore = mock_user.MockStore

func NewMockUserStore(ctrl *gomock.Controller) *MockUserStore {
  return mock_user.NewMockUserStore(ctrl)
}

type MockAccountStore = mock_account.MockStore

func NewMockAccountStore(ctrl *gomock.Controller) *MockAccountStore {
  return mock_account.NewMockAccountStore(ctrl)
}

Let me know if you have any more questions!

heynemann commented 1 year ago

I meant more in the sense of being a drop-in replacement for gomock. If that's out of scope here it's alright :) There are ways to work around it as you mentioned.

I'd rather have all mocks in the same package as we can just do mocks. and get auto-completion. Otherwise I'd have to import all of them to get the same results. Just sugar really.

If you feel like this is not something worth pursuing I'm fine with closing this issue.

JosiahWitt commented 1 year ago

I wonder if it would make sense to add an optional aliases field in .ensure.yml that is used to generate a mocks package.

For example:

mocks:
  packages:
    - path: github.com/some/path/user
      interfaces: [Store]
      aliases:
        UserStore: Store

    - path: github.com/some/path/account
      interfaces: [Store]
      aliases:
        AccountStore: Store

Could generate:

package mocks

type Aliased struct {
  UserStore    *MockUserStore    `ensure:"ignoreunused"`
  AccountStore *MockAccountStore `ensure:"ignoreunused"`
}

type MockUserStore = mock_user.MockStore

func NewMockUserStore(ctrl *gomock.Controller) *MockUserStore {
  return mock_user.NewMockUserStore(ctrl)
}

type MockAccountStore = mock_account.MockStore

func NewMockAccountStore(ctrl *gomock.Controller) *MockAccountStore {
  return mock_account.NewMockAccountStore(ctrl)
}

Where the aliased names are validated to be unique.

My only concern is it could result in multiple mocks packages being generated when you have some mocks in internal packages. For example, if user's path was instead github.com/some/path/abc/internal/user, we would end up with github.com/some/path/abc/internal/mocks for the user mock and github.com/some/path/internal/mocks for the account mock, which would be annoying if you wanted to import both. (I always find import aliases to be not ideal.) I'll have to think more about that. I guess one approach for a first pass would be to only support aliases on "top level" mocks (mocks of non-internal packages). Would that work for your use case?

Would an aliases feature be useful to you? Any thoughts / feedback?