Feature/dynamic response #165
Conversation
```python
def dyn_resp(req, resp):
# Can make the response dynamic based on the request
return {'key': 'value'}
pook.get('https://example.org/', persist=True, response_json=dyn_resp)
```
|
@h2non have you had a chance to look at this PR? |
sarayourfriend
left a comment
sarayourfriend
left a comment
There was a problem hiding this comment.
Thanks for the PR! And apologies for the late review, I've been unplugged for a while.
With the tests, because this feature is implemented in each of the interceptors, it would be good to add it to the StandardTests base class. That'll make sure it's tested for all the interceptors. You can definitely keep the tests in mock_test if you like and you feel they aren't redundant to any added in StandardTests 🙂
I'll see about pushing a change later this weekend or next week unless you get around to it first.
Cheers 🙂
src/pook/response.py
Outdated
| self: ``pook.Response`` current instance. | ||
| """ | ||
| if hasattr(body, "encode"): | ||
| if isfunction(body): |
There was a problem hiding this comment.
Is there a reason to prefer isfunction over callable here? isfunction returns false for objects with __call__.
There was a problem hiding this comment.
not really, no. it was more so I wasn't aware/forgot about callable. (been a while since I did heavy python coding ).
but yeah callable makes better sense to allow for __call__ support.
src/pook/response.py
Outdated
| return self | ||
|
|
||
| def fetch_body(self, request): | ||
| if isfunction(self._body) or hasattr(self._body, 'can_fetch_body'): |
There was a problem hiding this comment.
Wondering if this would be equivalent (if can_fetch_body could be excluded with this approach?)
| if isfunction(self._body) or hasattr(self._body, 'can_fetch_body'): | |
| if callable(self._body): |
There was a problem hiding this comment.
I'll do some tests. The main issue I was having was that I couldn't easily "detect" if the object was the right type, hence the can_fetch_body marker.
tests/unit/mock_test.py
Outdated
| return b"hello from pook" | ||
|
|
||
| mock = Mock( | ||
| url='https://example.com/fetch', |
There was a problem hiding this comment.
Top here that we tend to use the url_404 fixture for URLs. That way, if for some reason the test configuration causes the interceptor not to work, then the status == 200 assertion will fail and we'll get a hint 🙂 Theoretically https://example.com/fetch shouldn't ever give us back a body of hello from pook, but still a good assurance 😅
sarayourfriend
left a comment
sarayourfriend
left a comment
There was a problem hiding this comment.
Sorry, meant to request changes!
|
I added a fixup commit with a bunch of changes and more encompassing tests (in the StandardTests). Which was good as it exposed some issues with the urllib implementation. And I also added a persist() test which exposed another bug where the mock was only usable one time (thus defeating the purpose of this while PR). And I reworked the tests in the mock_tests to actually test that the correct _body was set / wrapped (For JSON). Please look thorugh my commit and see if there is anything else you want tweaked. If it all looks good let me know and I can squash the commits down before you merge anything. |
2 participants
Description
This PR adds support for passing a function for the response_body/response_xml/response_json.
This allows producing dynamic responses for mock.
Fixes #164
PR Checklist