Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Add a OneOf Matcher to match parameter as part of a list #90

Closed
abshierjoel opened this issue Sep 16, 2023 · 3 comments
Closed

Comments

@abshierjoel
Copy link

abshierjoel commented Sep 16, 2023

Problem Statement

While testing functions with multiple calls to the same method, occasionally we encounter a situation where we require multiple matches for one parameter. One approach would be to create multiple mock.EXPECT() calls for each distinct match, however, with the new gomock.WithOverridableExpectations(), the second call will override the first, regardless of any inclusion of .Times(x).

Regardless of the existing problem with gomock.WithOverridableExpectations(), in some situations it would be convenient to create multiple matches from a single EXPECT, especially if the DoAnyReturn() is complex. So I believe this would help ease-of-use.

Proposal

Include a new OneOf Matcher that will match if a given parameter belongs to the provided list. This will allow for multiple calls to be matched in one EXPECT() with TIMES(N), which temporarily helps solve the Override issue. In addition to this, being able to match one element against a list would be valuable and improve the ease of use when testing if a parameter matches within a given list.

Examples

Example A: Overriding Order

Consider the following function:

func SUT() {
    MockedFunction("Go")
    MockedFunction("Rust")
}

Assume we have created an EXPECT() for Do() and would like to later override it as follows:

mock.EXPECT().MockedFunction("Go").Return(true).Times(1)
mock.EXPECT().MockedFunction("Rust").Return(true).Times(1)

However, with gomock.WithOverridableExpectations() enabled, the 2nd EXPECT() takes precedent and the test fails. One solution to this would then be the following:

mock.EXPECT().
	MockedFunction(gomock.Any()).
	DoAndReturn(func(lang string) bool {
		switch lang {
		case "Go":
			return true
		case "Rust":
			return true
		}
		return false
	}).
	Times(2)
}

This approach allows us to use only one EXPECT call, but requires that we use an Any() Matcher and a default false case for the DoAndReturn. It would be preferable to be able to match on if the element was part of a list of acceptable parameter values, as follows:

mock.EXPECT().
	MockedFunction(gomock.OneOf([]any{"Go", "Rust"})).
	DoAndReturn(func(lang string) bool {
		switch lang {
		case "Go":
			return true
		case "Rust":
			return true
		}
		return false
	}).
	Times(2)
},

Example B: Mocking Convenience

Consider the following function used to determine which animal food you should use, based on your country:

func SUT(country string) Food {
	animal := interface.GetAnimal(country)
	food := interface.GetFood(animal)
	return food
}

// Valid animal types
func GetAnimals() {
  []Animals{
	  Gopher,
    	  Meerkat,
  	  Chinchilla,
  }
}

It may be convenient to stub the call to GetFood for any valid Animal type given. We can do this similarly to above, with a DoAnyReturn, but this again would require using an Any matcher. It would be preferable to be able to provide a list of valid matches, as follows:

func TestSUT(t *testing.T) {
    interfaceMock.EXPECT().
		GetFood({}any{Gopher, Meerkat, Chinchilla}).
		DoAnyReturn(func(animal Animal) Food {
			if animal == Meerkat {
				return Insects
			} 
			return Seeds
		}).AnyTimes()
}

Proposed implementation

I drafted a PR with my proposed implementation in #91

@r-hang
Copy link
Contributor

r-hang commented Sep 19, 2023

@abshierjoel can we close this issue now that #63 has landed?

@r-hang
Copy link
Contributor

r-hang commented Sep 26, 2023

Closing now that #63 has landed.

@r-hang r-hang closed this as completed Sep 26, 2023
@abshierjoel
Copy link
Author

@r-hang apologies on the late reply -- I've been away this past week. Thank you much!

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

No branches or pull requests

2 participants