Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Workaround false "unreachable" positives
Browse files Browse the repository at this point in the history
See Shoobx/mypy-zope#91

```
tests/http/test_proxyagent.py:626: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:762: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:826: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:838: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:845: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:151: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:452: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:60: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:93: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:127: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:152: error: Statement is unreachable  [unreachable]
```
  • Loading branch information
David Robertson committed Feb 10, 2023
1 parent b63789a commit 212897e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 36 deletions.
11 changes: 6 additions & 5 deletions tests/http/federation/test_matrix_federation_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
get_test_ca_cert_file,
)
from tests.server import FakeTransport, ThreadedMemoryReactorClock
from tests.utils import default_config
from tests.utils import default_config, checked_cast

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -146,8 +146,10 @@ def _make_connection(
#
# Normally this would be done by the TCP socket code in Twisted, but we are
# stubbing that out here.
client_protocol = client_factory.buildProtocol(dummy_address)
assert isinstance(client_protocol, _WrappingProtocol)
# NB: we use a checked_cast here to workaround https://github.com/Shoobx/mypy-zope/issues/91)
client_protocol = checked_cast(
_WrappingProtocol, client_factory.buildProtocol(dummy_address)
)
client_protocol.makeConnection(
FakeTransport(server_protocol, self.reactor, client_protocol)
)
Expand Down Expand Up @@ -446,7 +448,6 @@ def _do_get_via_proxy(
server_ssl_protocol = _wrap_server_factory_for_tls(
_get_test_protocol_factory()
).buildProtocol(dummy_address)
assert isinstance(server_ssl_protocol, TLSMemoryBIOProtocol)

# Tell the HTTP server to send outgoing traffic back via the proxy's transport.
proxy_server_transport = proxy_server.transport
Expand Down Expand Up @@ -1529,7 +1530,7 @@ def _check_logcontext(context: LoggingContextOrSentinel) -> None:

def _wrap_server_factory_for_tls(
factory: IProtocolFactory, sanlist: Optional[List[bytes]] = None
) -> IProtocolFactory:
) -> TLSMemoryBIOFactory:
"""Wrap an existing Protocol Factory with a test TLSMemoryBIOFactory
The resultant factory will create a TLS server which presents a certificate
signed by our test CA, valid for the domains in `sanlist`
Expand Down
42 changes: 20 additions & 22 deletions tests/http/test_proxyagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import base64
import logging
import os
from typing import List, Optional
from typing import List, Optional, cast
from unittest.mock import patch

import treq
Expand Down Expand Up @@ -43,6 +43,7 @@
)
from tests.server import FakeTransport, ThreadedMemoryReactorClock
from tests.unittest import TestCase
from tests.utils import checked_cast

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -620,7 +621,6 @@ def _do_https_request_via_proxy(
server_ssl_protocol = _wrap_server_factory_for_tls(
_get_test_protocol_factory()
).buildProtocol(dummy_address)
assert isinstance(server_ssl_protocol, TLSMemoryBIOProtocol)

# Tell the HTTP server to send outgoing traffic back via the proxy's transport.
proxy_server_transport = proxy_server.transport
Expand Down Expand Up @@ -757,12 +757,14 @@ def test_https_request_via_uppercase_proxy_with_blacklist(self) -> None:
assert isinstance(proxy_server, HTTPChannel)

# fish the transports back out so that we can do the old switcheroo
s2c_transport = proxy_server.transport
assert isinstance(s2c_transport, FakeTransport)
client_protocol = s2c_transport.other
assert isinstance(client_protocol, _WrappingProtocol)
c2s_transport = client_protocol.transport
assert isinstance(c2s_transport, FakeTransport)
# To help mypy out with the various Protocols and wrappers and mocks, we do
# some explicit casting. Without the casts, we hit the bug I reported at
# https://github.com/Shoobx/mypy-zope/issues/91 .
# We also double-checked these casts at runtime (test-time) because I found it
# quite confusing to deduce these types in the first place!
s2c_transport = checked_cast(FakeTransport, proxy_server.transport)
client_protocol = checked_cast(_WrappingProtocol, s2c_transport.other)
c2s_transport = checked_cast(FakeTransport, client_protocol.transport)

# the FakeTransport is async, so we need to pump the reactor
self.reactor.advance(0)
Expand Down Expand Up @@ -822,9 +824,9 @@ def test_https_request_via_uppercase_proxy_with_blacklist(self) -> None:
@patch.dict(os.environ, {"http_proxy": "proxy.com:8888"})
def test_proxy_with_no_scheme(self) -> None:
http_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
assert isinstance(http_proxy_agent.http_proxy_endpoint, HostnameEndpoint)
self.assertEqual(http_proxy_agent.http_proxy_endpoint._hostStr, "proxy.com")
self.assertEqual(http_proxy_agent.http_proxy_endpoint._port, 8888)
proxy_ep = checked_cast(HostnameEndpoint, http_proxy_agent.http_proxy_endpoint)
self.assertEqual(proxy_ep._hostStr, "proxy.com")
self.assertEqual(proxy_ep._port, 8888)

@patch.dict(os.environ, {"http_proxy": "socks://proxy.com:8888"})
def test_proxy_with_unsupported_scheme(self) -> None:
Expand All @@ -834,25 +836,21 @@ def test_proxy_with_unsupported_scheme(self) -> None:
@patch.dict(os.environ, {"http_proxy": "http://proxy.com:8888"})
def test_proxy_with_http_scheme(self) -> None:
http_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
assert isinstance(http_proxy_agent.http_proxy_endpoint, HostnameEndpoint)
self.assertEqual(http_proxy_agent.http_proxy_endpoint._hostStr, "proxy.com")
self.assertEqual(http_proxy_agent.http_proxy_endpoint._port, 8888)
proxy_ep = checked_cast(HostnameEndpoint, http_proxy_agent.http_proxy_endpoint)
self.assertEqual(proxy_ep._hostStr, "proxy.com")
self.assertEqual(proxy_ep._port, 8888)

@patch.dict(os.environ, {"http_proxy": "https://proxy.com:8888"})
def test_proxy_with_https_scheme(self) -> None:
https_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
assert isinstance(https_proxy_agent.http_proxy_endpoint, _WrapperEndpoint)
self.assertEqual(
https_proxy_agent.http_proxy_endpoint._wrappedEndpoint._hostStr, "proxy.com"
)
self.assertEqual(
https_proxy_agent.http_proxy_endpoint._wrappedEndpoint._port, 8888
)
proxy_ep = checked_cast(_WrapperEndpoint, https_proxy_agent.http_proxy_endpoint)
self.assertEqual(proxy_ep._wrappedEndpoint._hostStr, "proxy.com")
self.assertEqual(proxy_ep._wrappedEndpoint._port, 8888)


def _wrap_server_factory_for_tls(
factory: IProtocolFactory, sanlist: Optional[List[bytes]] = None
) -> IProtocolFactory:
) -> TLSMemoryBIOFactory:
"""Wrap an existing Protocol Factory with a test TLSMemoryBIOFactory
The resultant factory will create a TLS server which presents a certificate
Expand Down
17 changes: 9 additions & 8 deletions tests/logging/test_remote_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from tests.logging import LoggerCleanupMixin
from tests.server import FakeTransport, get_clock
from tests.unittest import TestCase
from tests.utils import checked_cast


def connect_logging_client(
Expand Down Expand Up @@ -56,8 +57,8 @@ def test_log_output(self) -> None:
client, server = connect_logging_client(self.reactor, 0)

# Trigger data being sent
assert isinstance(client.transport, FakeTransport)
client.transport.flush()
client_transport = checked_cast(FakeTransport, client.transport)
client_transport.flush()

# One log message, with a single trailing newline
logs = server.data.decode("utf8").splitlines()
Expand Down Expand Up @@ -89,8 +90,8 @@ def test_log_backpressure_debug(self) -> None:

# Allow the reconnection
client, server = connect_logging_client(self.reactor, 0)
assert isinstance(client.transport, FakeTransport)
client.transport.flush()
client_transport = checked_cast(FakeTransport, client.transport)
client_transport.flush()

# Only the 7 infos made it through, the debugs were elided
logs = server.data.splitlines()
Expand Down Expand Up @@ -123,8 +124,8 @@ def test_log_backpressure_info(self) -> None:

# Allow the reconnection
client, server = connect_logging_client(self.reactor, 0)
assert isinstance(client.transport, FakeTransport)
client.transport.flush()
client_transport = checked_cast(FakeTransport, client.transport)
client_transport.flush()

# The 10 warnings made it through, the debugs and infos were elided
logs = server.data.splitlines()
Expand All @@ -148,8 +149,8 @@ def test_log_backpressure_cut_middle(self) -> None:

# Allow the reconnection
client, server = connect_logging_client(self.reactor, 0)
assert isinstance(client.transport, FakeTransport)
client.transport.flush()
client_transport = checked_cast(FakeTransport, client.transport)
client_transport.flush()

# The first five and last five warnings made it through, the debugs and
# infos were elided
Expand Down
26 changes: 25 additions & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import atexit
import os
from typing import Any, Callable, Dict, List, Tuple, Union, overload
from typing import Any, Callable, Dict, List, Tuple, Union, overload, TypeVar, Type

import attr
from typing_extensions import Literal, ParamSpec
Expand Down Expand Up @@ -338,3 +338,27 @@ async def create_room(hs: HomeServer, room_id: str, creator_id: str) -> None:
event, context = await event_creation_handler.create_new_client_event(builder)

await persistence_store.persist_event(event, context)


T = TypeVar("T")


def checked_cast(type: Type[T], x: object) -> T:
"""A version of typing.cast that is checked at runtime.
We have our own function for this for two reasons:
1. typing.cast itself is deliberately a no-op at runtime, see
https://docs.python.org/3/library/typing.html#typing.cast
2. To help workaround a mypy-zope bug https://github.com/Shoobx/mypy-zope/issues/91
where mypy would erroneously consider `isinstance(x, type)` to be false in all
circumstances.
For this to make sense, `T` needs to be something that `isinstance` can check; see
https://docs.python.org/3/library/functions.html?highlight=isinstance#isinstance
https://docs.python.org/3/glossary.html#term-abstract-base-class
https://docs.python.org/3/library/typing.html#typing.runtime_checkable
for more details.
"""
assert isinstance(x, type)
return x

0 comments on commit 212897e

Please sign in to comment.