Skip to content

Commit

Permalink
lint: Use newer mypy
Browse files Browse the repository at this point in the history
This required some minor code changes, mainly some adjustments in tests
(which are now analyzed more thoroughly in spite of being mostly
unannotated), and some changes to placement of type:ignore comments.
  • Loading branch information
bdarnell committed Nov 23, 2019
1 parent 8837c30 commit 44ae52c
Show file tree
Hide file tree
Showing 28 changed files with 221 additions and 157 deletions.
2 changes: 2 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.

# type: ignore

import os
import platform
import sys
Expand Down
4 changes: 2 additions & 2 deletions tornado/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def _on_authentication_verified(
) -> Dict[str, Any]:
handler = cast(RequestHandler, self)
if b"is_valid:true" not in response.body:
raise AuthError("Invalid OpenID response: %s" % response.body)
raise AuthError("Invalid OpenID response: %r" % response.body)

# Make sure we got back at least an email from attribute exchange
ax_ns = None
Expand Down Expand Up @@ -556,7 +556,7 @@ def authorize_redirect(
client_id: Optional[str] = None,
client_secret: Optional[str] = None,
extra_params: Optional[Dict[str, Any]] = None,
scope: Optional[str] = None,
scope: Optional[List[str]] = None,
response_type: str = "code",
) -> None:
"""Redirects the user to obtain OAuth authorization for this service.
Expand Down
4 changes: 2 additions & 2 deletions tornado/autoreload.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@ def _reload() -> None:
# Unfortunately the errno returned in this case does not
# appear to be consistent, so we can't easily check for
# this error specifically.
os.spawnv( # type: ignore
os.P_NOWAIT, sys.executable, [sys.executable] + argv
os.spawnv(
os.P_NOWAIT, sys.executable, [sys.executable] + argv # type: ignore
)
# At this point the IOLoop has been closed and finally
# blocks will experience errors if we allow the stack to
Expand Down
8 changes: 4 additions & 4 deletions tornado/httpclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
from tornado.ioloop import IOLoop
from tornado.util import Configurable

from typing import Type, Any, Union, Dict, Callable, Optional, cast, Awaitable
from typing import Type, Any, Union, Dict, Callable, Optional, cast


class HTTPClient(object):
Expand Down Expand Up @@ -251,7 +251,7 @@ def fetch(
request: Union[str, "HTTPRequest"],
raise_error: bool = True,
**kwargs: Any
) -> Awaitable["HTTPResponse"]:
) -> "Future[HTTPResponse]":
"""Executes a request, asynchronously returning an `HTTPResponse`.
The request may be either a string URL or an `HTTPRequest` object.
Expand Down Expand Up @@ -506,7 +506,7 @@ def __init__(
"""
# Note that some of these attributes go through property setters
# defined below.
self.headers = headers
self.headers = headers # type: ignore
if if_modified_since:
self.headers["If-Modified-Since"] = httputil.format_timestamp(
if_modified_since
Expand All @@ -518,7 +518,7 @@ def __init__(
self.proxy_auth_mode = proxy_auth_mode
self.url = url
self.method = method
self.body = body
self.body = body # type: ignore
self.body_producer = body_producer
self.auth_username = auth_username
self.auth_password = auth_password
Expand Down
4 changes: 3 additions & 1 deletion tornado/httputil.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,9 @@ def __init__(
def cookies(self) -> Dict[str, http.cookies.Morsel]:
"""A dictionary of ``http.cookies.Morsel`` objects."""
if not hasattr(self, "_cookies"):
self._cookies = http.cookies.SimpleCookie()
self._cookies = (
http.cookies.SimpleCookie()
) # type: http.cookies.SimpleCookie
if "Cookie" in self.headers:
try:
parsed = parse_cookie(self.headers["Cookie"])
Expand Down
2 changes: 1 addition & 1 deletion tornado/iostream.py
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,7 @@ def write_to_fd(self, data: memoryview) -> int:
del data

def connect(
self: _IOStreamType, address: tuple, server_hostname: Optional[str] = None
self: _IOStreamType, address: Any, server_hostname: Optional[str] = None
) -> "Future[_IOStreamType]":
"""Connects the socket to a remote address without blocking.
Expand Down
2 changes: 1 addition & 1 deletion tornado/netutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ def _resolve_addr(
results = []
for fam, socktype, proto, canonname, address in addrinfo:
results.append((fam, address))
return results
return results # type: ignore


class DefaultExecutorResolver(Resolver):
Expand Down
19 changes: 13 additions & 6 deletions tornado/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def cpu_count() -> int:
except NotImplementedError:
pass
try:
return os.sysconf("SC_NPROCESSORS_CONF")
return os.sysconf("SC_NPROCESSORS_CONF") # type: ignore
except (AttributeError, ValueError):
pass
gen_log.error("Could not detect number of processors; assuming 1")
Expand Down Expand Up @@ -117,6 +117,10 @@ def fork_processes(
Availability: Unix
"""
if sys.platform == "win32":
# The exact form of this condition matters to mypy; it understands
# if but not assert in this context.
raise Exception("fork not available on windows")
if max_restarts is None:
max_restarts = 100

Expand Down Expand Up @@ -349,7 +353,7 @@ def _cleanup(cls) -> None:
@classmethod
def _try_cleanup_process(cls, pid: int) -> None:
try:
ret_pid, status = os.waitpid(pid, os.WNOHANG)
ret_pid, status = os.waitpid(pid, os.WNOHANG) # type: ignore
except ChildProcessError:
return
if ret_pid == 0:
Expand All @@ -359,11 +363,14 @@ def _try_cleanup_process(cls, pid: int) -> None:
subproc.io_loop.add_callback_from_signal(subproc._set_returncode, status)

def _set_returncode(self, status: int) -> None:
if os.WIFSIGNALED(status):
self.returncode = -os.WTERMSIG(status)
if sys.platform == "win32":
self.returncode = -1
else:
assert os.WIFEXITED(status)
self.returncode = os.WEXITSTATUS(status)
if os.WIFSIGNALED(status):
self.returncode = -os.WTERMSIG(status)
else:
assert os.WIFEXITED(status)
self.returncode = os.WEXITSTATUS(status)
# We've taken over wait() duty from the subprocess.Popen
# object. If we don't inform it of the process's return code,
# it will log a warning at destruction in python 3.6+.
Expand Down
2 changes: 1 addition & 1 deletion tornado/test/asyncio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def run_policy_test(self, accessor, expected_type):
self.assertIsInstance(self.executor.submit(accessor).result(), expected_type)
# Clean up to silence leak warnings. Always use asyncio since
# IOLoop doesn't (currently) close the underlying loop.
self.executor.submit(lambda: asyncio.get_event_loop().close()).result()
self.executor.submit(lambda: asyncio.get_event_loop().close()).result() # type: ignore

def test_asyncio_accessor(self):
self.run_policy_test(asyncio.get_event_loop, asyncio.AbstractEventLoop)
Expand Down
8 changes: 4 additions & 4 deletions tornado/test/auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def get(self):
raise Exception("user is None")
self.finish(user)
return
res = self.authenticate_redirect()
res = self.authenticate_redirect() # type: ignore
assert res is None


Expand Down Expand Up @@ -131,7 +131,7 @@ def initialize(self, test):
self._OAUTH_AUTHORIZE_URL = test.get_url("/oauth2/server/authorize")

def get(self):
res = self.authorize_redirect()
res = self.authorize_redirect() # type: ignore
assert res is None


Expand All @@ -152,7 +152,7 @@ def get(self):
)
self.write(user)
else:
yield self.authorize_redirect(
self.authorize_redirect(
redirect_uri=self.request.full_url(),
client_id=self.settings["facebook_api_key"],
extra_params={"scope": "read_stream,offline_access"},
Expand Down Expand Up @@ -547,7 +547,7 @@ def get(self):
user["access_token"] = access["access_token"]
self.write(user)
else:
yield self.authorize_redirect(
self.authorize_redirect(
redirect_uri=self._OAUTH_REDIRECT_URI,
client_id=self.settings["google_oauth"]["key"],
client_secret=self.settings["google_oauth"]["secret"],
Expand Down
13 changes: 8 additions & 5 deletions tornado/test/concurrent_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import logging
import re
import socket
import typing
import unittest

from tornado.concurrent import (
Expand Down Expand Up @@ -98,6 +99,8 @@ def capitalize(self, request_data):


class ClientTestMixin(object):
client_class = None # type: typing.Callable

def setUp(self):
super(ClientTestMixin, self).setUp() # type: ignore
self.server = CapServer()
Expand All @@ -109,27 +112,27 @@ def tearDown(self):
self.server.stop()
super(ClientTestMixin, self).tearDown() # type: ignore

def test_future(self):
def test_future(self: typing.Any):
future = self.client.capitalize("hello")
self.io_loop.add_future(future, self.stop)
self.wait()
self.assertEqual(future.result(), "HELLO")

def test_future_error(self):
def test_future_error(self: typing.Any):
future = self.client.capitalize("HELLO")
self.io_loop.add_future(future, self.stop)
self.wait()
self.assertRaisesRegexp(CapError, "already capitalized", future.result)
self.assertRaisesRegexp(CapError, "already capitalized", future.result) # type: ignore

def test_generator(self):
def test_generator(self: typing.Any):
@gen.coroutine
def f():
result = yield self.client.capitalize("hello")
self.assertEqual(result, "HELLO")

self.io_loop.run_sync(f)

def test_generator_error(self):
def test_generator_error(self: typing.Any):
@gen.coroutine
def f():
with self.assertRaisesRegexp(CapError, "already capitalized"):
Expand Down
3 changes: 3 additions & 0 deletions tornado/test/http1connection_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import socket
import typing

from tornado.http1connection import HTTP1Connection
from tornado.httputil import HTTPMessageDelegate
Expand All @@ -9,6 +10,8 @@


class HTTP1ConnectionTest(AsyncTestCase):
code = None # type: typing.Optional[int]

def setUp(self):
super(HTTP1ConnectionTest, self).setUp()
self.asyncSetUp()
Expand Down
11 changes: 8 additions & 3 deletions tornado/test/httpclient_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ class AllMethodsHandler(RequestHandler):
SUPPORTED_METHODS = RequestHandler.SUPPORTED_METHODS + ("OTHER",) # type: ignore

def method(self):
assert self.request.method is not None
self.write(self.request.method)

get = head = post = put = delete = options = patch = other = method # type: ignore
Expand Down Expand Up @@ -177,6 +178,7 @@ def test_hello_world(self):
self.assertEqual(response.code, 200)
self.assertEqual(response.headers["Content-Type"], "text/plain")
self.assertEqual(response.body, b"Hello world!")
assert response.request_time is not None
self.assertEqual(int(response.request_time), 0)

response = self.fetch("/hello?name=Ben")
Expand Down Expand Up @@ -281,7 +283,7 @@ def test_unsupported_auth_mode(self):
# important thing is that they don't fall back to basic auth
# on an unknown mode.
with ExpectLog(gen_log, "uncaught exception", required=False):
with self.assertRaises((ValueError, HTTPError)):
with self.assertRaises((ValueError, HTTPError)): # type: ignore
self.fetch(
"/auth",
auth_username="Aladdin",
Expand Down Expand Up @@ -521,6 +523,8 @@ def test_future_interface(self):
def test_future_http_error(self):
with self.assertRaises(HTTPError) as context:
yield self.http_client.fetch(self.get_url("/notfound"))
assert context.exception is not None
assert context.exception.response is not None
self.assertEqual(context.exception.code, 404)
self.assertEqual(context.exception.response.code, 404)

Expand Down Expand Up @@ -551,7 +555,7 @@ def test_bind_source_ip(self):
response = yield self.http_client.fetch(request)
self.assertEqual(response.code, 200)

with self.assertRaises((ValueError, HTTPError)) as context:
with self.assertRaises((ValueError, HTTPError)) as context: # type: ignore
request = HTTPRequest(url, network_interface="not-interface-or-ip")
yield self.http_client.fetch(request)
self.assertIn("not-interface-or-ip", str(context.exception))
Expand Down Expand Up @@ -642,6 +646,7 @@ def test_response_times(self):
self.assertLess(response.request_time, 1.0)
# A very crude check to make sure that start_time is based on
# wall time and not the monotonic clock.
assert response.start_time is not None
self.assertLess(abs(response.start_time - start_time), 1.0)

for k, v in response.time_info.items():
Expand Down Expand Up @@ -698,7 +703,7 @@ def test_defaults_none(self):
class HTTPResponseTestCase(unittest.TestCase):
def test_str(self):
response = HTTPResponse( # type: ignore
HTTPRequest("http://example.com"), 200, headers={}, buffer=BytesIO()
HTTPRequest("http://example.com"), 200, buffer=BytesIO()
)
s = str(response)
self.assertTrue(s.startswith("HTTPResponse("))
Expand Down
25 changes: 12 additions & 13 deletions tornado/test/httpserver_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ def finish(self):


class HandlerBaseTestCase(AsyncHTTPTestCase):
Handler = None

def get_app(self):
return Application([("/", self.__class__.Handler)])

Expand Down Expand Up @@ -121,32 +123,32 @@ def get_ssl_options(self):
def get_ssl_version(self):
raise NotImplementedError()

def test_ssl(self):
def test_ssl(self: typing.Any):
response = self.fetch("/")
self.assertEqual(response.body, b"Hello world")

def test_large_post(self):
def test_large_post(self: typing.Any):
response = self.fetch("/", method="POST", body="A" * 5000)
self.assertEqual(response.body, b"Got 5000 bytes in POST")

def test_non_ssl_request(self):
def test_non_ssl_request(self: typing.Any):
# Make sure the server closes the connection when it gets a non-ssl
# connection, rather than waiting for a timeout or otherwise
# misbehaving.
with ExpectLog(gen_log, "(SSL Error|uncaught exception)"):
with ExpectLog(gen_log, "Uncaught exception", required=False):
with self.assertRaises((IOError, HTTPError)):
with self.assertRaises((IOError, HTTPError)): # type: ignore
self.fetch(
self.get_url("/").replace("https:", "http:"),
request_timeout=3600,
connect_timeout=3600,
raise_error=True,
)

def test_error_logging(self):
def test_error_logging(self: typing.Any):
# No stack traces are logged for SSL errors.
with ExpectLog(gen_log, "SSL Error") as expect_log:
with self.assertRaises((IOError, HTTPError)):
with self.assertRaises((IOError, HTTPError)): # type: ignore
self.fetch(
self.get_url("/").replace("https:", "http:"), raise_error=True
)
Expand Down Expand Up @@ -961,7 +963,7 @@ def test_keepalive_chunked_head_no_body(self):
self.close()


class GzipBaseTest(object):
class GzipBaseTest(AsyncHTTPTestCase):
def get_app(self):
return Application([("/", EchoHandler)])

Expand Down Expand Up @@ -1182,14 +1184,11 @@ def initialize(self):
self.bytes_read = 0

def prepare(self):
conn = typing.cast(HTTP1Connection, self.request.connection)
if "expected_size" in self.request.arguments:
self.request.connection.set_max_body_size(
int(self.get_argument("expected_size"))
)
conn.set_max_body_size(int(self.get_argument("expected_size")))
if "body_timeout" in self.request.arguments:
self.request.connection.set_body_timeout(
float(self.get_argument("body_timeout"))
)
conn.set_body_timeout(float(self.get_argument("body_timeout")))

def data_received(self, data):
self.bytes_read += len(data)
Expand Down
Loading

0 comments on commit 44ae52c

Please sign in to comment.