lundberg / respx

Mock HTTPX with awesome request patterns and response side effects 🦋
https://lundberg.github.io/respx
BSD 3-Clause "New" or "Revised" License
581 stars 38 forks source link

Enhance assertion output for `assert_all_called` #224

Closed sileht closed 1 year ago

sileht commented 1 year ago

When AllCalledAssertionError is raised its hard to guess which route got called and which routes didn't get called.

This change aims to provide more details by listing not called routes and by leveraging the repr of route object.

codecov-commenter commented 1 year ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (b9df437) compared to base (6f8d05f). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #224 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 20 20 Lines 2757 2755 -2 Branches 417 417 ========================================= - Hits 2757 2755 -2 ``` | [Impacted Files](https://codecov.io/gh/lundberg/respx/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg) | Coverage Δ | | |---|---|---| | [respx/models.py](https://codecov.io/gh/lundberg/respx/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg#diff-cmVzcHgvbW9kZWxzLnB5) | `100.00% <ø> (ø)` | | | [respx/router.py](https://codecov.io/gh/lundberg/respx/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg#diff-cmVzcHgvcm91dGVyLnB5) | `100.00% <100.00%> (ø)` | | | [tests/test\_mock.py](https://codecov.io/gh/lundberg/respx/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg#diff-dGVzdHMvdGVzdF9tb2NrLnB5) | `100.00% <100.00%> (ø)` | | | [tests/test\_transports.py](https://codecov.io/gh/lundberg/respx/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg#diff-dGVzdHMvdGVzdF90cmFuc3BvcnRzLnB5) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Lundberg)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

sileht commented 1 year ago

Yeah, this looks even nicer. Here is a pytest example:

    ...
    def assert_all_called(self) -> None:
        not_called_routes = [route for route in self.routes if not route.called]
>       assert not_called_routes == [], "RESPX: some routes were not called!"
E       AssertionError: RESPX: some routes were not called!
E       assert [<Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify/config.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.github/mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/commits/old-sha-one/check-runs'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/check-runs'> AND <Method eq 'POST'>>] == []
E         Left contains 6 more items, first extra item: <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>
E         Full diff:
E           [
E         -  ,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify/config.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.github/mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND <Params contains QueryParams('ref=base-sha')>>,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/commits/old-sha-one/check-runs'> AND <Method eq 'GET'>>,
E         +  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/check-runs'> AND <Method eq 'POST'>>,
E           ]

I have to use a list instead of a set as the Route object is not hashable.

sileht commented 1 year ago

Another example, if we also list called routes with:

       called_routes = [route for route in self.routes if route.called]
       assert called_routes == list(self.routes), "RESPX: some routes were not called!"
    def assert_all_called(self) -> None:
        called_routes = [route for route in self.routes if route.called]
>       assert called_routes == list(self.routes), "RESPX: some routes were not called!"
E       AssertionError: RESPX: some routes were not called!
E       assert [<Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/app/installations/12345/access_tokens'> AND <Method eq 'POST'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/user/12345/installation'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>] == [<Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/app/installations/12345/access_tokens'> AND <Method eq 'POST'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/user/12345/installation'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify/config.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.github/mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND <Params contains QueryParams('ref=base-sha')>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/commits/old-sha-one/check-runs'> AND <Method eq 'GET'>>, <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/check-runs'> AND <Method eq 'POST'>>]
E         At index 3 diff: <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>> != <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>
E         Right contains 6 more items, first extra item: <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>
E         Full diff:
E           [
E            <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/app/installations/12345/access_tokens'> AND <Method eq 'POST'>>,
E            <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/user/12345/installation'> AND <Method eq 'GET'>>,
E            <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1'> AND <Method eq 'GET'>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/pulls/1/files'> AND <Method eq 'GET'>>,
E            <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify/config.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.github/mergify.yml'> AND NOT <Params contains QueryParams('ref=base-sha')>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Method eq 'GET'> AND <Path eq '/repos/testing/name/contents/.mergify.yml'> AND <Params contains QueryParams('ref=base-sha')>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/commits/old-sha-one/check-runs'> AND <Method eq 'GET'>>,
E         -  <Route <Scheme eq 'https'> AND <Host eq 'api.github.com'> AND <Path eq '/repos/testing/name/check-runs'> AND <Method eq 'POST'>>,
E           ]

.tox/py310/lib/python3.10/site-packages/respx/router.py:106: AssertionError

Tell me which one you prefer and I will update the pull request.

lundberg commented 1 year ago

I have to use a list instead of a set as the Route object is not hashable.

👍

Tell me which one you prefer and I will update the pull request.

I'd say the first implementation aligns better with the assertion, i.e. all called == no remaining non-called

A third alternative that might give roughly the same output would be ...

assert len(non_called_routes) == 0
sileht commented 1 year ago

So it's ready 😀

lundberg commented 1 year ago

So it's ready 😀

Almost .. We should probably drop the AllCalledAssertionError class and adjust tests using that one.

lundberg commented 1 year ago

Once AllCalledAssertionError is dropped, all tests that are using it should be constrained to match part of the assertion message.

lundberg commented 1 year ago

Thanks @sileht