Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow reusing settings from a MockRouter instance on MockRouter.__call__ #181

Open
flaeppe opened this issue Nov 5, 2021 · 5 comments · May be fixed by #186
Open

Allow reusing settings from a MockRouter instance on MockRouter.__call__ #181

flaeppe opened this issue Nov 5, 2021 · 5 comments · May be fixed by #186
Labels
enhancement New feature or request

Comments

@flaeppe
Copy link
Contributor

flaeppe commented Nov 5, 2021

I'm wondering if it could be possible to support reusing some settings of a MockRouter instance, when invoking __call__. Basically that'll allow me to specify a MockRouter with defaults, and for specific test cases flip the assert_all_called-flag.

Consider the code below. I want base_url and assert_all_called to have its default, unless they're overriden by a new __call__ invocation

import httpx
import respx
default = respx.mock(base_url="http://some.where/", assert_all_called=False)
default.get(path="/here/")

with default(assert_all_called=True):
    httpx.get("http://some.where/here/")  # Raises: RESPX: <Request('GET', 'http://some.where/here/')> not mocked!

@default(assert_all_called=True)
def test_1():
    httpx.get("http://some.where/here/")  # Raises: RESPX: <Request('GET', 'http://some.where/here/')> not mocked!

test_1()

# This should still have the default `assert_all_called=False`
@default
def test_2():
    httpx.get("http://some.where/here/")  # This works

test_2()

Currently, this gives me an error whenever I pass in new kwargs to default(test_1 and with default). As __call__ is just creating a new MockRouter instance from scratch.

I'm realising as I write this that support for this might appear better through some additional MockRouter method (e.g. MockRouter.replace()) instead of changing __call__.

@lundberg
Copy link
Owner

lundberg commented Nov 8, 2021

This is a great suggestion and enhancement of the router settings.

I think this could be solved by adding a .configure(...) method to the router and include the assertions and base_url settings in the snapshot/rollback functionality. That would allow the user to alter these settings within a specific test case and automatically be rolled back to previous configuration when exiting mocked context.

Something like this:

my_mock_api = respx.mock(assert_all_called=False)
...

@my_mock_api
def test_somthing_else():
    ...
    # won't assert that all routes have been called when exiting this test case

@my_mock_api
def test_something():
    my_mock_api.configure(assert_all_called=True)
    ...
    # will assert that all routes have been called when exiting this test case

@lundberg lundberg added the enhancement New feature or request label Nov 8, 2021
@Mojken Mojken linked a pull request Nov 11, 2021 that will close this issue
@lundberg
Copy link
Owner

lundberg commented Nov 11, 2021

I gave this a try a few days ago @flaeppe, and got stuck on a "problem" with the base_url.

How it works is that the base URL is turned into a bases pattern by the router, and in turn merged to each added route.

We need a way to alter/clone any existing routes' patterns if they have previous bases. This is also mentioned in #186.

About the configure api, I'm now leaning towards your first idea of allowing call on the router instance to re-configure all settings. Actually, it already works like this, except that any existing routes are not cloned to the new router instance.

@flaeppe
Copy link
Contributor Author

flaeppe commented Nov 12, 2021

About the configure api, I'm now leaning towards your first idea of allowing call on the router instance to re-configure all settings. Actually, it already works like this, except that any existing routes are not cloned to the new router instance.

Cool, yeah. Some additional Config object instance that could be attached as a "head" for patterns, together with having the bases as some sort of "swappable component" might work? I'm really just babbling now, haven't dug in to how that would actually pan out.

How it works is that the base URL is turned into a bases pattern by the router, and in turn merged to each added route.

One initial idea could be to "postpone" merging until matching/comparison happens, though that will come with a performance hit, not sure how severe that would be though. Or if it could be mitigated with some @cached_property or similar.

But if I'm interpreting you right here, the problematic part is that bases are merged with each route and we have no way of "rewinding" that merge? So that we can only edit the bases patterns.

@lundberg
Copy link
Owner

lundberg commented Nov 12, 2021

But if I'm interpreting you right here, the problematic part is that bases are merged with each route and we have no way of "rewinding" that merge? So that we can only edit the bases patterns.

Correct @flaeppe.

At the moment, I'm trying to actually implement a rewind feature, or more correctly adjust the merge_patterns function to smartly override any existing bases with new ones in a pattern tree.

Also to get that working, I had to implement .clone() on RouteList, Route and Pattern (nested) to not mutate parent router state 😅 .

@lundberg
Copy link
Owner

lundberg commented Dec 2, 2021

Side note...this would be of interest even for #190 in the case a route is added to the default global respx router, then all subsequent routers would inherit that/those added routes and a test case would only need to be decorated with the sub-router.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants