golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.26k stars 608 forks source link

Proposal: Provide an option where every invocation of `EXPECT()` of a mock should override any previous invocations. #685

Open styeung opened 1 year ago

styeung commented 1 year ago

Hi team,

The proposal here is to allow for an option to bring GoMock closer in line with other mocking libraries that exist for other languages. In particular, the GoMock behavior of ordering mock expectations in a FIFO manner makes it difficult to bundle default mock expectations into a helper or testify Suite's BeforeTest block.

The problem

For example, the following set up won't work:

type ExampleTestSuite struct {
    suite.Suite

    repoAMock *MockRepositoryA
    repoBMock *MockRepositoryB
    repoCMock *MockRepositoryC

    sut *Controller
}

func TestExampleTestSuite(t *testing.T) {
    suite.Run(t, new(ExampleTestSuite))
}

func (s *ExampleTestSuite) SetupTest() {
    ctrl := gomock.NewController(s.T())
    s.repoAMock = NewMockRepositoryA(ctrl)
    s.repoBMock = NewMockRepositoryB(ctrl)
    s.repoCMock = NewMockRepositoryC(ctrl)

    s.sut = NewController(s.repoAMock, s.repoBMock, s.repoCMock) // subject under test

    // set up mock defaults
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("foo", nil)
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("bar", nil)
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("baz", nil)
}

func (s * ExampleTestSuite) TestGather_HappyPath() {
    res, rr := s.sut.Gather()
    s.Require().Equal("foo, bar, baz", res)
    s.Require().NoError(err)
}

func (s *ExampleTestSuite) TestGather_RepoAFails() {
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

func (s *ExampleTestSuite) TestGather_RepoBFails() {
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

func (s *ExampleTestSuite) TestGather_RepoCFails() {
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

Instead, one must do something like this:

type ExampleTestSuite struct {
    suite.Suite

    repoAMock *MockRepositoryA
    repoBMock *MockRepositoryB
    repoCMock *MockRepositoryC

    sut *Controller
}

func TestExampleTestSuite(t *testing.T) {
    suite.Run(t, new(ExampleTestSuite))
}

func (s *ExampleTestSuite) SetupTest() {
    ctrl := gomock.NewController(s.T())
    s.repoAMock = NewMockRepositoryA(ctrl)
    s.repoBMock = NewMockRepositoryB(ctrl)
    s.repoCMock = NewMockRepositoryC(ctrl)

    s.sut = NewController(s.repoAMock, s.repoBMock, s.repoCMock) // subject under test
}

func (s *ExampleTestSuite) TestGather_HappyPath() {
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("foo", nil)
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("bar", nil)
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("baz", nil)

    res, err := s.sut.Gather()
    s.Require().Equal("foo, bar, baz", res)
    s.Require().NoError(err)
}

func (s *ExampleTestSuite) TestGather_RepoAFails() {
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("bar", nil)
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("baz", nil)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

func (s *ExampleTestSuite) TestGather_RepoBFails() {
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("foo", nil)
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("baz", nil)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

func (s *ExampleTestSuite) TestGather_RepoCFails() {
    s.repoAMock.EXPECT().Get(gomock.Any()).Return("foo", nil)
    s.repoBMock.EXPECT().Get(gomock.Any()).Return("bar", nil)
    s.repoCMock.EXPECT().Get(gomock.Any()).Return("", assert.AnError)

    _, err := s.sut.Gather()
    s.Require().Error(err)
}

It's not hard to imagine that the test body can really blow up once the number of dependencies one needs to mock out grows in size.

Examples from other languages

Some popular mocking frameworks in other languages support setting default mock behaviors, which helps clean up the test code:

  1. gMock for C++ - "newer rules override older ones."
  2. Mockito for Java - "Stubbing can be overridden: for example common stubbing can go to fixture setup but the test methods can override it."
  3. Jasmine for Javascript

    beforeEach(function() {
        this.propertySpy = spyOnProperty(someObject, "myValue", "get").and.returnValue(1);
    });
    
    it("lets you change the spy strategy later", function() {
        this.propertySpy.and.returnValue(3);
        expect(someObject.myValue).toEqual(3);
    });
  4. Rspec for Ruby

      describe '#active?' do
        let(:envrc) { '/Users/pivotal/workspace/loggregator/.envrc' }
    
        before do
            allow(FileTest).to receive(:exist?).and_return(false)
        end
    
      it 'returns true when .envrc contains GOPATH' do
        allow(FileTest).to receive(:exist?).with(envrc).and_return(true)
        allow(IO).to receive(:read).with(Pathname(envrc)).and_return('export GOPATH=/foo/bar')
        expect(subject.active?).to eq(true)
      end

Proposal

Provide an option where every invocation of EXPECT() of a mock should override any previous invocations.

How to address returning different values based on input?

Use DoAndReturn to handle different args

Example Pull Request

https://github.com/golang/mock/pull/686

Alternatives Considered

There have been several previous issues filed that proposed a reverse ordering strategy for mock expectations:

However, a couple usability problems arise with a reverse ordering strategy. One is that if one were to put a mock expectation in a SetupTest that runs before each test, it's implied that one must append AnyTimes() to the end of it. This is because if one were to leave that out, the gomock controller would continue to wait for for that expectation, even if one were to "override" it within the test block.