Added mocking forever support #72
Conversation
tasomaniac
force-pushed
the
taso/mock_forever
branch
from
f8a321f to
0722730
Compare
In the original code, the requests list/queue is processed when new requests are executed. And earlier mocks are removed on-the-fly when hiroaki responds. It is actually not possible to compare and remove previous matchers immediately when they're added since they don't implement `equals` functionality. With this approach here is some principles I put into place: - Regular `respond` always takes priority regardless of being added before or after. - The most recently added `respondForever` takes precedence. As mentioned above, I cannot clear previous matchers immediately when a new one is added. I could potentially clear previous one when a `respondForever` is matched successfully. I figured that this is not a good idea since 1. it is not trivial, 2. there might be cases where previously added matcher can be useful for other requests even though it is matched for one request.
tasomaniac
force-pushed
the
taso/mock_forever
branch
from
0722730 to
21d31d2
Compare
| dataSource.getNews() | ||
| } | ||
|
|
||
| server.verify("v2/top-headlines").called() |
There was a problem hiding this comment.
This test should verify that error is returned on first call, then success with the provided body for subsequent calls. We're just verifying that the endpoint is called here 🤔
There was a problem hiding this comment.
There is a single call here. So no error at all. I want to demonstrate the behavior where calling it The 2nd times overrides the existing. So error is ignored.
There was a problem hiding this comment.
I would expect this to return error on a first match, then subsequent calls would return the provided success forever 🤔 that's what "then" wording leads to understand and I'd like to achieve.
There was a problem hiding this comment.
Fair point. Could it be fine to come up with different naming?
Calling forever 2nd time should override the first. This makes it fundamentally different than the original behavior. Let me think about it more and how this can be made obvious.
If you can think of any alternative, it would be awesome.
There was a problem hiding this comment.
I think the naming is great and I like the feature, I'd aim for matching the behaviour I describe. That would make it coherent, as in living up to the "then" prefix for ordering matched / dispatched requests. That's what makes the whole thing very idiomatic on usage, imho. As you say, making it "obvious".
My expectations for this feature would be:
-
If you add any number of
thenRespondbeforethenRespondForever, those must be matched first and removed.thenRespondForeverdoes not get removed from the queue when matched / dispatched, so it can keep matching / dispatching. -
As soon as a
thenRespondForeveris declared, furtherthenRespondcalls declared after it would have no effect. This would be clarified in docs, sincethenRespondForeverhas a wider priority given to its own semantics. It's "forever". -
Same for multiple calls to
thenRespondForever, I'd suggest matching just for the first one found on the queue so it matches previous behaviour also. -
We can add an additional
resetQueueparam tothenRespondandthenRespondForever(same for the dispatch variants) so you can reset the queue and your newly added matcher becomes the first one in the queue. That would be used for cases where you want to assert multiple scenarios under the same test. In my opinion it's always better to write different tests for this, so they're atomic in terms of error reporting once they start failing. Still it's fine to provide this feature for flexibility imho, since every project has different requirements and people uses to work in many different ways, so I'd rather not being super opinionated on this. Some scenario for the sake of the example:- User configures matchers for one request, one
thenRespond(), onethenRespondForever(). thenRespond()gets matched when first endpoint call occurs, user verifies.- 3 more endpoint calls occur,
thenRespondForever()is matched for all those. User verifies. - Then user decides she/he wants to queue a new
thenRespond()(or a newthenRespondForever()) and make further calls, then verify. The user could use theresetQueueparam for this.resetQueueparam would be defaulted to false.
- User configures matchers for one request, one
Thoughts? Do you see any flaws? 🤔
There was a problem hiding this comment.
Sorry for super late reply. I think I will give it another go.
I think this is going to be very difficult because technically, it is impossible to compare matchers.
Imagine a scenario where thenRespond and thenRespondForever has slightly different matchers matching the same request. This also depends on the request, that's why all should be recorded and then decided at the time of the actual request.
Queue logic is much easier since they are processed and removed sequentially.
If you already have an idea on the data structure, please let me know. I think I will first right the tests according to these expectations. And then see how we can do this.
| dataSource.getNews() | ||
| } | ||
|
|
||
| server.verify("v2/top-headlines").called() |
There was a problem hiding this comment.
Same here, we should verify the second one is the one responded for N calls and the first one is ignored
| dataSource.getNews() | ||
| } | ||
|
|
||
| server.verify("v2/top-headlines").called() |
There was a problem hiding this comment.
This doesn't seem to be verifying what's required here?
📌 References
Fixes #68
🎩 What is the goal?
This PR adds 2 new functions
thenRespondForeverandthenDispatchForeverto have a behavior where the mocking mechanism is persistent until the server is shut down.💻 How is it being implemented?
In the original code, the requests list/queue is processed when new requests are executed. And earlier mocks are removed on-the-fly when hiroaki responds. It is actually not possible to compare and remove previous matchers immediately when they're added since they don't implement
equalsfunctionality.With this approach here is some principles I put into place:
respondalways takes priority regardless of being added before or after.respondForevertakes precedence. As mentioned above, I cannot clear previous matchers immediately when a new one is added. I could potentially clear previous one when arespondForeveris matched successfully. I figured that this is not a good idea since 1. it is not trivial, 2. there might be cases where previously added matcher can be useful for other requests even though it is matched for one request.📱 How to Test
Tests added testing various behavior.
No changes to existing tests verify that we don't have any behavior change.