Skip to content

Commit

Permalink
Reduce number of Optional return values (#217)
Browse files Browse the repository at this point in the history
* Disallow slice assigning routes

* type CallList

* Remove unnececary backend arg

* type side_effect setter

* configure mypy - include tests for mypy

* Test python 3.11

* Make Call.response non optional, raise instead

* assert resolved.response is not None

* type ignore bad usage

* use == instead of is to be reachable

* Introduce Noop to make pattern non optional

* Add Call.has_response helper
  • Loading branch information
lundberg authored Sep 16, 2022
1 parent 6403618 commit 207cd16
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 55 deletions.
4 changes: 2 additions & 2 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
docs_requirements = ("mkdocs", "mkdocs-material", "mkautodoc>=0.1.0")


@nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"])
@nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10", "3.11"])
def test(session):
deps = ["pytest", "pytest-asyncio", "pytest-cov", "trio", "starlette", "flask"]
session.install("--upgrade", *deps)
Expand All @@ -30,7 +30,7 @@ def check(session):
session.run("black", "--check", "--diff", "--target-version=py36", *source_files)
session.run("isort", "--check", "--diff", "--project=respx", *source_files)
session.run("flake8", *source_files)
session.run("mypy", "respx")
session.run("mypy")


@nox.session
Expand Down
38 changes: 27 additions & 11 deletions respx/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,40 @@ def clone_response(response: httpx.Response, request: httpx.Request) -> httpx.Re

class Call(NamedTuple):
request: httpx.Request
response: Optional[httpx.Response]
optional_response: Optional[httpx.Response]

@property
def response(self) -> httpx.Response:
if self.optional_response is None:
raise ValueError(f"{self!r} has no response")
return self.optional_response

@property
def has_response(self) -> bool:
return self.optional_response is not None


class CallList(list, mock.NonCallableMock):
def __init__(self, *args, name="respx", **kwargs):
super().__init__(*args, **kwargs)
def __init__(self, *args: Sequence[Call], name: Any = "respx") -> None:
super().__init__(*args)
mock.NonCallableMock.__init__(self, name=name)

@property
def called(self) -> bool: # type: ignore
def called(self) -> bool: # type: ignore[override]
return bool(self)

@property
def call_count(self) -> int: # type: ignore
def call_count(self) -> int: # type: ignore[override]
return len(self)

@property
def last(self) -> Optional[Call]:
return self[-1] if self else None
def last(self) -> Call:
return self[-1]

def record(
self, request: httpx.Request, response: Optional[httpx.Response]
) -> Call:
call = Call(request=request, response=response)
call = Call(request=request, optional_response=response)
self.append(call)
return call

Expand Down Expand Up @@ -155,7 +165,7 @@ def name(self, name: str) -> None:
raise NotImplementedError("Can't set name on route.")

@property
def pattern(self) -> Optional[Pattern]:
def pattern(self) -> Pattern:
return self._pattern

@pattern.setter
Expand All @@ -174,7 +184,9 @@ def return_value(self, return_value: Optional[httpx.Response]) -> None:
self._return_value = return_value

@property
def side_effect(self) -> Optional[SideEffectTypes]:
def side_effect(
self,
) -> Optional[Union[SideEffectTypes, Sequence[SideEffectListTypes]]]:
return self._side_effect

@side_effect.setter
Expand Down Expand Up @@ -230,7 +242,9 @@ def mock(
self,
return_value: Optional[httpx.Response] = None,
*,
side_effect: Optional[SideEffectTypes] = None,
side_effect: Optional[
Union[SideEffectTypes, Sequence[SideEffectListTypes]]
] = None,
) -> "Route":
self.return_value = return_value
self.side_effect = side_effect
Expand Down Expand Up @@ -430,6 +444,8 @@ def __setitem__(self, i: slice, routes: "RouteList") -> None:
"""
Re-set all routes to given routes.
"""
if (i.start, i.stop, i.step) != (None, None, None):
raise TypeError("Can't slice assign routes")
self._routes = list(routes._routes)
self._names = dict(routes._names)

Expand Down
45 changes: 36 additions & 9 deletions respx/patterns.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,26 @@ def __init__(self, value: Any, lookup: Optional[Lookup] = None) -> None:
def __iter__(self):
yield self

def __bool__(self):
return True

def __and__(self, other: "Pattern") -> "Pattern":
if not bool(other):
return self
elif not bool(self):
return other
return _And((self, other))

def __or__(self, other: "Pattern") -> "Pattern":
if not bool(other):
return self
elif not bool(self):
return other
return _Or((self, other))

def __invert__(self):
if not bool(self):
return self
return _Invert(self)

def __repr__(self): # pragma: nocover
Expand Down Expand Up @@ -159,6 +172,22 @@ def _in(self, value: Any) -> Match:
return Match(value in self.value)


class Noop(Pattern):
def __init__(self) -> None:
super().__init__(None)

def __repr__(self):
return f"<{self.__class__.__name__}>"

def __bool__(self) -> bool:
# Treat this pattern as non-existent, e.g. when filtering or conditioning
return False

def match(self, request: httpx.Request) -> Match:
# If this pattern is part of a combined pattern, always be truthy, i.e. noop
return Match(True)


class PathPattern(Pattern):
path: Optional[str]

Expand Down Expand Up @@ -500,7 +529,7 @@ def clean(self, value: Dict) -> bytes:
return data


def M(*patterns: Pattern, **lookups: Any) -> Optional[Pattern]:
def M(*patterns: Pattern, **lookups: Any) -> Pattern:
extras = None

for pattern__lookup, value in lookups.items():
Expand Down Expand Up @@ -550,12 +579,10 @@ def get_scheme_port(scheme: Optional[str]) -> Optional[int]:
return {"http": 80, "https": 443}.get(scheme or "")


def combine(
patterns: Sequence[Pattern], op: Callable = operator.and_
) -> Optional[Pattern]:
def combine(patterns: Sequence[Pattern], op: Callable = operator.and_) -> Pattern:
patterns = tuple(filter(None, patterns))
if not patterns:
return None
return Noop()
return reduce(op, patterns)


Expand Down Expand Up @@ -598,14 +625,14 @@ def parse_url_patterns(
return bases


def merge_patterns(pattern: Optional[Pattern], **bases: Pattern) -> Optional[Pattern]:
def merge_patterns(pattern: Pattern, **bases: Pattern) -> Pattern:
if not bases:
return pattern

if pattern:
# Flatten pattern
patterns = list(iter(pattern))
# Flatten pattern
patterns: List[Pattern] = list(filter(None, iter(pattern)))

if patterns:
if "host" in (_pattern.key for _pattern in patterns):
# Pattern is "absolute", skip merging
bases = {}
Expand Down
13 changes: 13 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ skip_covered = True
show_missing = True

[mypy]
python_version = 3.6
files = respx,tests
pretty = True

no_implicit_reexport = True
no_implicit_optional = True
strict_equality = True
Expand All @@ -53,3 +57,12 @@ show_error_codes = True

[mypy-pytest.*]
ignore_missing_imports = True

[mypy-trio.*]
ignore_missing_imports = True

[mypy-flask.*]
ignore_missing_imports = True

[mypy-starlette.*]
ignore_missing_imports = True
21 changes: 14 additions & 7 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ async def test_url_match(client, url, pattern):
async def test_invalid_url_pattern():
async with MockRouter() as respx_mock:
with pytest.raises(TypeError):
respx_mock.get(["invalid"])
respx_mock.get(["invalid"]) # type: ignore[arg-type]


@pytest.mark.asyncio
Expand Down Expand Up @@ -277,7 +277,10 @@ async def test_raising_content(client):
async with MockRouter() as respx_mock:
url = "https://foo.bar/"
request = respx_mock.get(url)
request.side_effect = httpx.ConnectTimeout("X-P", request=None)
request.side_effect = httpx.ConnectTimeout(
"X-P",
request=None, # type: ignore[arg-type]
)
with pytest.raises(httpx.ConnectTimeout):
await client.get(url)

Expand All @@ -293,7 +296,9 @@ async def test_raising_content(client):

assert route.call_count == 2
assert route.calls.last.request is not None
assert route.calls.last.response is None
assert route.calls.last.has_response is False
with pytest.raises(ValueError, match="has no response"):
assert route.calls.last.response


@pytest.mark.asyncio
Expand Down Expand Up @@ -356,7 +361,9 @@ def callback(request, name):
assert response.text == "hello lundberg"

with pytest.raises(TypeError):
respx_mock.get("https://ham.spam/").mock(side_effect=lambda req: "invalid")
respx_mock.get("https://ham.spam/").mock(
side_effect=lambda req: "invalid" # type: ignore[arg-type,return-value]
)
await client.get("https://ham.spam/")

with pytest.raises(httpx.NetworkError):
Expand Down Expand Up @@ -526,10 +533,10 @@ def test_add():
assert respx.routes["foobar"].called

with pytest.raises(TypeError):
respx.add(route, status_code=418) # pragma: nocover
respx.add(route, status_code=418) # type: ignore[call-arg]

with pytest.raises(ValueError):
respx.add("GET") # pragma: nocover
respx.add("GET") # type: ignore[arg-type]

with pytest.raises(NotImplementedError):
route.name = "spam"
Expand All @@ -554,7 +561,7 @@ def test_respond():
route.respond(content={})

with pytest.raises(TypeError, match="content can only be"):
route.respond(content=Exception())
route.respond(content=Exception()) # type: ignore[arg-type]


@pytest.mark.asyncio
Expand Down
18 changes: 9 additions & 9 deletions tests/test_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,25 +317,25 @@ async def test_configured_router_reuse(client):
with router:
route.return_value = httpx.Response(202)
response = await client.get("https://foo/bar/")
assert route.called is True
assert route.called == True # noqa: E712
assert response.status_code == 202
assert router.calls.call_count == 1
assert respx.calls.call_count == 0

assert len(router.routes) == 1
assert route.called is False
assert route.called == False # noqa: E712
assert router.calls.call_count == 0

async with router:
assert router.calls.call_count == 0
response = await client.get("https://foo/bar/")
assert route.called is True
assert route.called == True # noqa: E712
assert response.status_code == 404
assert router.calls.call_count == 1
assert respx.calls.call_count == 0

assert len(router.routes) == 1
assert route.called is False
assert route.called == False # noqa: E712
assert router.calls.call_count == 0
assert respx.calls.call_count == 0

Expand All @@ -346,7 +346,7 @@ async def test_router_return_type_misuse():
route = router.get("https://hot.dog/")

with pytest.raises(TypeError):
route.return_value = "not-a-httpx-response"
route.return_value = "not-a-httpx-response" # type: ignore[assignment]


@pytest.mark.asyncio
Expand Down Expand Up @@ -396,20 +396,20 @@ async def test_start_stop(client):
try:
respx.start()
response = await client.get(url)
assert request.called is True
assert request.called == True # noqa: E712
assert response.status_code == 202
assert response.text == ""
assert respx.calls.call_count == 1

respx.stop(clear=False, reset=False)
assert len(respx.routes) == 1
assert respx.calls.call_count == 1
assert request.called is True
assert request.called == True # noqa: E712

respx.reset()
assert len(respx.routes) == 1
assert respx.calls.call_count == 0
assert request.called is False
assert request.called == False # noqa: E712

respx.clear()
assert len(respx.routes) == 0
Expand Down Expand Up @@ -545,7 +545,7 @@ async def test():

def test_router_using__invalid():
with pytest.raises(ValueError, match="using"):
respx.MockRouter(using=123).using
respx.MockRouter(using=123).using # type: ignore[arg-type]


def test_mocker_subclass():
Expand Down
13 changes: 13 additions & 0 deletions tests/test_patterns.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
Lookup,
M,
Method,
Noop,
Params,
Path,
Pattern,
Expand Down Expand Up @@ -66,6 +67,18 @@ def test_match_context():
assert match.context == {"host": "foo.bar", "slug": "baz"}


def test_noop_pattern():
assert bool(Noop()) is False
assert bool(Noop().match(httpx.Request("GET", "https://example.org"))) is True
assert list(filter(None, [Noop()])) == []
assert repr(Noop()) == "<Noop>"
assert isinstance(~Noop(), Noop)
assert Method("GET") & Noop() == Method("GET")
assert Noop() & Method("GET") == Method("GET")
assert Method("GET") | Noop() == Method("GET")
assert Noop() | Method("GET") == Method("GET")


@pytest.mark.parametrize(
"kwargs,url,expected",
[
Expand Down
2 changes: 1 addition & 1 deletion tests/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_remote_pass_through(using, client_lib, call_count): # pragma: nocover
assert response.json()["json"] == {"foo": "bar"}

assert respx_mock.calls.last.request.url == url
assert respx_mock.calls.last.response is None
assert respx_mock.calls.last.has_response is False

assert route.call_count == call_count
assert respx_mock.calls.call_count == call_count
Loading

0 comments on commit 207cd16

Please sign in to comment.