URL match improvements #84
Conversation
|
Thanks for contributing to RESPX @SlavaSkvortsov ! I like the idea of breaking up the code for readability and easier (re)use. Though, I think we need to take the discussion in #81, by @jocke-l, into account and start using Two reasons, less "duplicated" code from HTTPX to maintain, and to align with the HTTPX api, as discussed in #79, i.e. support I'd like to see an api where one could do... @respx.mock(base_url="https://example.org")
async def test_something(respx_mock):
respx_mock.post("/foobar/", params={"foo": "bar"})...but also, if possible, even... @respx.mock(base_url="https://example.org")
async def test_something(respx_mock):
respx_mock.post(r"/foobar/(?P<slug>\w+)/", params={"foo": "bar"})Meaning, it would be nice to support both |
|
Thank you for maintaining this lib, you are doing a great job! I'll change the PR, but I don't think I can implement regular string URL support at this iteration. Maybe later :) |
|
Thanks for the update, but I think you mis-interpreted by last comment ;-) What I meant was that it would be best if this PR extends/depends on #81. It's better to keep each PR small and specific to make reviews easier and to minimize introduction of bugs. Also the commits and changelog gets clearer this way. Let's merge #81 first, and then come back to this PR, sounds good @SlavaSkvortsov ? |
|
Yeap, sounds great :) |
# Conflicts: # respx/api.py # respx/models.py # respx/transports.py # tests/test_api.py
|
This one looks ready as well. |
respx/models.py
Outdated
| url = httpx.URL(url_parts) | ||
| url, params = self._extract_params(url) | ||
|
|
||
| if self._params is not None and self._params != params: |
There was a problem hiding this comment.
QueryParams can be bool evaluated.
| if self._params is not None and self._params != params: | |
| if self._params and self._params != params: |
There was a problem hiding this comment.
Also, should we require full match on all params, or treat them as pattern matching and accept kwargs params only to be included in possibly "richer" request url params?
There was a problem hiding this comment.
Also, should we require full match on all params, or treat them as pattern matching and accept kwargs params only to be included in possibly "richer" request url params?
It's a nice question. I thought about it today. I think the second option is better
There was a problem hiding this comment.
I'm also leaning to second option, i.e. accept pattern params to be subset of request params.
Major reason is that it aligns with your proposal of empty/missing pattern params matching requests with params.
| self._params = httpx.QueryParams(params) | ||
|
|
||
| if isinstance(url, (str, tuple, httpx.URL)): | ||
| self._url = base_url.join(httpx.URL(url)) |
There was a problem hiding this comment.
Here will base_url be joined with it self, when url arg is None, due to line 388.
Maybe not a problem?
There was a problem hiding this comment.
Oh, I'll take a look
Co-authored-by: Jonas Lundberg <jonas@5monkeys.se>
|
Sorry, but this one is too outdated now as of #96. Please check out the new patterns to see if any of your logic is missing. |
Refactored URL matching, moved URL-related logic to a separate class.
The main goal was to do a params-independent URL comparing. It was hard to use the lib for GET requests (had to use regex matching)