`0.20.2` broke our tests, but I think it's good (our use case + questions + feature request)

macieyng commented 11 months ago

Hi @lundberg 👋

I want to let you know that #240 (or 0.20.2 in general) broke our tests - but I think that's good. I want to tell you how we used it and what actually broke as I think that there's a room for small improvement.

We had our mocks defined as:

respx_mock.get("https://example.com/orders?storeId=1&status=paid&status=reviewed")

And change from #240 (or 0.20.2) made us rewrite them as

respx_mock.get(
    "https://example.com/orders",
    params={
        "storeId": "1",
        "status":[
            "paid",
            "reviewed"
        ]
    }
)

I believe that up until now, we used this bug to our advantage.

I inspected what was happening under the hood in 0.20.2 and it turns out that:

First one:

Request:  <Request('GET', 'https://example.com/api/orders?storeId=1&status=paid&status=reviewed')>
Pattern:  <Scheme eq 'https'> AND <Host eq 'example.com'> AND <Path eq '/api/orders?storeId=1&status=paid&status=reviewed'> AND <Method eq 'GET'>
Match:  <Match False>

and second one:

Request:  <Request('GET', 'https://example.com/orders?storeId=1&status=paid&status=reviewed')>
Pattern:  <Scheme eq 'https'> AND <Host eq 'example.com'> AND <Path eq '/api/orders'> AND <Method eq 'GET'> AND <Params contains QueryParams('storeId=1&status=paid&status=reviewed')>
Match:  <Match True>

These are obviously two different sets of patterns to match.

As a follow up to this issue I want to ask:

Can we document this in caveat section of documentation? I think there might be some folks doing the same error as we did and this could save them hours or days of debugging.
Was this behavior expected?

Do you plan to make querystrings produce identical set of patterns?

ie: when

respx_mock.get("https://example.com/orders?storeId=1&status=paid&status=reviewed")

then

Request:  <Request('GET', 'https://example.com/orders?storeId=1&status=paid&status=reviewed')>
Pattern:  <Scheme eq 'https'> AND <Host eq 'example.com'> AND <Path eq '/api/orders'> AND <Method eq 'GET'> AND <Params contains QueryParams('storeId=1&status=paid&status=reviewed')>
Match:  <Match True>

lundberg commented 11 months ago

Hmm, that should work 🤔

Could you provide a failing test example for the first mock with the querystring.

macieyng commented 10 months ago

Hi @lundberg. Sorry for slow response. Here is a minimal setup that allow me to reproduce this in isolation.

import httpx
import respx

def send_request():
    return httpx.get("https://example.com",
        params={
            "storeId": 1,
            "status":[
                "paid",
                "reviewed"
            ]
        }
    )

def test_pass(respx_mock: respx.MockRouter):
    respx_mock.get(
        "https://example.com",
        params={
            "storeId": 1,
            "status":[
                "paid",
                "reviewed"
            ]
        }
    )
    send_request()

def test_fail(respx_mock: respx.MockRouter):
    respx_mock.get("https://example.com/orders?storeId=1&status=paid&status=reviewed")
    send_request()

This yields the following output

(respx-243) ➜  respx-243 pytest main.py 
============================================================================= test session starts ==============================================================================
platform darwin -- Python 3.11.4, pytest-7.4.2, pluggy-1.3.0
rootdir: /Users/maciejnachtygal/respx-243
plugins: anyio-4.0.0, respx-0.20.2
collected 2 items                                                                                                                                                              

main.py .F                                                                                                                                                               [100%]

=================================================================================== FAILURES ===================================================================================
__________________________________________________________________________________ test_fail ___________________________________________________________________________________

respx_mock = <respx.router.MockRouter object at 0x102926950>

    def test_fail(respx_mock: respx.MockRouter):
        respx_mock.get("https://example.com/orders?storeId=1&status=paid&status=reviewed")
>       send_request()

main.py:32: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
main.py:5: in send_request
    return httpx.get("https://example.com",
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_api.py:189: in get
    return request(
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_api.py:100: in request
    return client.request(
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_client.py:814: in request
    return self.send(request, auth=auth, follow_redirects=follow_redirects)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_client.py:901: in send
    response = self._send_handling_auth(
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_client.py:929: in _send_handling_auth
    response = self._send_handling_redirects(
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_client.py:966: in _send_handling_redirects
    response = self._send_single_request(request)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_client.py:1002: in _send_single_request
    response = transport.handle_request(request)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/httpx/_transports/default.py:228: in handle_request
    resp = self._pool.handle_request(req)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/mocks.py:181: in mock
    response = cls._send_sync_request(
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/mocks.py:212: in _send_sync_request
    httpx_response = cls.handler(httpx_request)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/mocks.py:120: in handler
    raise assertion_error
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/mocks.py:113: in handler
    httpx_response = router.handler(httpx_request)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/router.py:313: in handler
    resolved = self.resolve(request)
../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/router.py:277: in resolve
    with self.resolver(request) as resolved:
../.pyenv/versions/3.11.4/lib/python3.11/contextlib.py:144: in __exit__
    next(self.gen)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <respx.router.MockRouter object at 0x102926950>, request = <Request('GET', 'https://example.com/?storeId=1&status=paid&status=reviewed')>

    @contextmanager
    def resolver(self, request: httpx.Request) -> Generator[ResolvedRoute, None, None]:
        resolved = ResolvedRoute()

        try:
            yield resolved

            if resolved.route is None:
                # Assert we always get a route match, if check is enabled
                if self._assert_all_mocked:
>                   raise AllMockedAssertionError(f"RESPX: {request!r} not mocked!")
E                   respx.models.AllMockedAssertionError: RESPX: <Request('GET', 'https://example.com/?storeId=1&status=paid&status=reviewed')> not mocked!

../.local/share/virtualenvs/respx-243-a1MnSEUX/lib/python3.11/site-packages/respx/router.py:250: AllMockedAssertionError
=========================================================================== short test summary info ============================================================================
FAILED main.py::test_fail - respx.models.AllMockedAssertionError: RESPX: <Request('GET', 'https://example.com/?storeId=1&status=paid&status=reviewed')> not mocked!
========================================================================= 1 failed, 1 passed in 0.26s ==========================================================================
(respx-243) ➜  respx-243

Here is package setup:

(respx-243) ➜  respx-243 pip freeze
anyio==4.0.0
certifi==2023.7.22
h11==0.14.0
httpcore==0.18.0
httpx==0.25.0
idna==3.4
iniconfig==2.0.0
packaging==23.1
pluggy==1.3.0
pytest==7.4.2
respx==0.20.2
sniffio==1.3.0

and python version

(respx-243) ➜  respx-243 python --version
Python 3.11.4

lundberg commented 9 months ago

I tried your reproduced setup, but modified the failing test to mock the same url as in send_request, i.e. https://example.com?..., and then both tests works. Anything I'm missing?

lundberg commented 4 months ago

Doesn't seem to be any problem .. please re-open if there's still an issue.

lundberg / respx

`0.20.2` broke our tests, but I think it's good (our use case + questions + feature request) #243