Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

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

Open
styeung opened this issue Dec 9, 2022 · 0 comments

Comments

@styeung
Copy link

styeung commented Dec 9, 2022

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);
  });
  1. 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

#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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant