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

Allow control over matching order of call expectations #152

Open
thediveo opened this issue Feb 4, 2024 · 3 comments
Open

Allow control over matching order of call expectations #152

thediveo opened this issue Feb 4, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@thediveo
Copy link

thediveo commented Feb 4, 2024

Requested Feature

Give test spec writers control over the sequence in which call expectations for a specfic 《receiver, method》are processed, in order to allow adding more specific expectations before an earlier set up "catch all" expectations. For instance, by adding MatchFirst() and MatchLast() methods to Call which then indicate order preference.

A test might want to create the mock with default calls that match any args, any time, and that forward the calls to a real implementation, such as a Docker client. While it is possible to replace the default call expectations, it is impossible to replace a single default expectation with a series of expectations matching specific args (see below).

Context("handling Docker API errors", func() {

	var rec *MockClientMockRecorder
	var sess *Session

	BeforeEach(func(ctx context.Context) {
		ctrl := mock.NewController(GinkgoT())
		sess = Successful(NewSession(ctx,
			WithMockController(ctrl)))
		rec = sess.Client().(*MockClient).EXPECT()
	})

	It("silently handles API network list errors", func(ctx context.Context) {
		rec.NetworkList(Any, Any).
			MatchFirst().
			Return(nil, errors.New("error IJK305I"))
		// ...
	})
})

Why the feature is needed

WithOverridableExpectations always replaces a specific expectation for 《receiver, method》 with a single new one, making it impossible to replace an existing default expectation with a series of expectations.

(Optional) Proposed solution

  1. Adding a placement field to Call.
  2. Adding MatchFirst and MatchLast methods that set the placement of a call, much like the min and max settings.
  3. Modify callset.add to use Call.placement to either add at the beginning of the matcher list, at the end, or default (which seems to be at the end).
@JacobOaks
Copy link
Contributor

cc: @r-hang

@r-hang
Copy link
Contributor

r-hang commented Feb 20, 2024

Hey @thediveo,

Thank you for filing the issue. My understanding of the issue is that gomock queue will queue expectations by normal means. Under WithOverridableExpectations, this queuing behavior stops and expectation are overriden by each subsequent calls -- which means it's not possible to override with queuing (e.g. series of expectations) behavior.

While I can see how overriding expectations with queuing behavior does allow users to match for call placement I think that something like a MatchFirst or MatchLast to Call doesn't quite seem to fully address the functionality that would be possible if a user could reset/override a mock's expectation on demand and then continue queuing mock expectations.

Is my description above a sufficient understanding of the issue?

@thediveo
Copy link
Author

@r-hang yes, that's also my understanding. The placement idea was initial, to get the discussion going. With some time passed, it doesn't look like correctly addressing the underlying situation.

Looking at the situation now, it could be a better solution to allow removing all previously set expectations for a particular receiver in order to register a fresh set of expectations. Kind of an override, but allowing for multiple chained expectations, where the existing override mechanism only allows for a single replacement expectation.

At the moment I'm working around the current situation by using optional args in the mock setup, where this args are the names of receiver methods that should not get default expectations. This is somewhat brittle, so I would highly appreciate a better way in the future.

For reference, here's my workaround: https://github.com/thediveo/morbyd/blob/4138a6913cae8105b41805515112e303773cec0c/client_test.go#L53 ... I'm using strings in lieu of typed constants which admittedly is more brittle than necessary.

@r-hang r-hang added the enhancement New feature or request label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants