From d3e1fb3c7018e40dbf79a394b6f848da52a8273e Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sun, 29 Jul 2018 12:44:30 -0400 Subject: [PATCH] test: Partially typecheck tests for annotated modules The only type annotations added were those required by mypy. This found some errors in the original annotations. --- setup.cfg | 11 ++++++++ tornado/httputil.py | 10 +++---- tornado/test/escape_test.py | 8 ++++-- tornado/test/httputil_test.py | 49 ++++++++++++++++---------------- tornado/test/util_test.py | 53 ++++++++++++++++++++--------------- tornado/util.py | 28 ++++++++++++------ 6 files changed, 96 insertions(+), 63 deletions(-) diff --git a/setup.cfg b/setup.cfg index 5ea4c875..4097ecf4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,3 +9,14 @@ disallow_untyped_defs = True [mypy-tornado.escape] disallow_untyped_defs = True + +# It's generally too tedious to require type annotations in tests, but +# we do want to type check them as much as type inference allows. +[mypy-tornado.test.util_test] +check_untyped_defs = True + +[mypy-tornado.test.httputil_test] +check_untyped_defs = True + +[mypy-tornado.test.escape_test] +check_untyped_defs = True diff --git a/tornado/httputil.py b/tornado/httputil.py index 72f1eb6b..d5dc675b 100644 --- a/tornado/httputil.py +++ b/tornado/httputil.py @@ -347,7 +347,7 @@ class HTTPServerRequest(object): def __init__(self, method: str=None, uri: str=None, version: str="HTTP/1.0", headers: HTTPHeaders=None, body: bytes=None, host: str=None, - files: Dict[str, 'HTTPFile']=None, connection: 'HTTPConnection'=None, + files: Dict[str, List['HTTPFile']]=None, connection: 'HTTPConnection'=None, start_line: 'RequestStartLine'=None, server_connection: object=None) -> None: if start_line is not None: method, uri, version = start_line @@ -578,7 +578,7 @@ class HTTPConnection(object): raise NotImplementedError() -def url_concat(url: str, args: Union[Dict[str, str], List[Tuple[str, str]], +def url_concat(url: str, args: Union[None, Dict[str, str], List[Tuple[str, str]], Tuple[Tuple[str, str], ...]]) -> str: """Concatenate url and arguments regardless of whether url has existing query parameters. @@ -702,7 +702,7 @@ def _int_or_none(val: str) -> Optional[int]: def parse_body_arguments(content_type: str, body: bytes, arguments: Dict[str, List[bytes]], - files: Dict[str, HTTPFile], headers: HTTPHeaders=None) -> None: + files: Dict[str, List[HTTPFile]], headers: HTTPHeaders=None) -> None: """Parses a form request body. Supports ``application/x-www-form-urlencoded`` and @@ -739,7 +739,7 @@ def parse_body_arguments(content_type: str, body: bytes, arguments: Dict[str, Li def parse_multipart_form_data(boundary: bytes, data: bytes, arguments: Dict[str, List[bytes]], - files: Dict[str, HTTPFile]) -> None: + files: Dict[str, List[HTTPFile]]) -> None: """Parses a ``multipart/form-data`` body. The ``boundary`` and ``data`` parameters are both byte strings. @@ -783,7 +783,7 @@ def parse_multipart_form_data(boundary: bytes, data: bytes, arguments: Dict[str, name = disp_params["name"] if disp_params.get("filename"): ctype = headers.get("Content-Type", "application/unknown") - files.setdefault(name, []).append(HTTPFile( # type: ignore + files.setdefault(name, []).append(HTTPFile( filename=disp_params["filename"], body=value, content_type=ctype)) else: diff --git a/tornado/test/escape_test.py b/tornado/test/escape_test.py index 6881850d..5c458f08 100644 --- a/tornado/test/escape_test.py +++ b/tornado/test/escape_test.py @@ -7,6 +7,8 @@ from tornado.escape import ( ) from tornado.util import unicode_type +from typing import List, Tuple, Union, Dict, Any # noqa + linkify_tests = [ # (input, linkify_kwargs, expected_output) @@ -132,7 +134,7 @@ linkify_tests = [ ("www.external-link.com", {"extra_params": lambda href: ' rel="nofollow" class="external" '}, u'www.external-link.com'), # noqa: E501 -] +] # type: List[Tuple[Union[str, bytes], Dict[str, Any], str]] class EscapeTestCase(unittest.TestCase): @@ -152,7 +154,7 @@ class EscapeTestCase(unittest.TestCase): (u"<\u00e9>", u"<\u00e9>"), (b"<\xc3\xa9>", b"<\xc3\xa9>"), - ] + ] # type: List[Tuple[Union[str, bytes], Union[str, bytes]]] for unescaped, escaped in tests: self.assertEqual(utf8(xhtml_escape(unescaped)), utf8(escaped)) self.assertEqual(utf8(unescaped), utf8(xhtml_unescape(escaped))) @@ -178,7 +180,7 @@ class EscapeTestCase(unittest.TestCase): # unicode strings become utf8 (u'\u00e9', '%C3%A9'), - ] + ] # type: List[Tuple[Union[str, bytes], str]] for unescaped, escaped in tests: self.assertEqual(url_escape(unescaped), escaped) diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py index cec6017f..d6d3e5c0 100644 --- a/tornado/test/httputil_test.py +++ b/tornado/test/httputil_test.py @@ -2,7 +2,7 @@ from tornado.httputil import ( url_concat, parse_multipart_form_data, HTTPHeaders, format_timestamp, HTTPServerRequest, parse_request_start_line, parse_cookie, qs_to_qsl, - HTTPInputError, + HTTPInputError, HTTPFile ) from tornado.escape import utf8, native_str from tornado.log import gen_log @@ -16,6 +16,17 @@ import time import urllib.parse import unittest +from typing import Tuple, Dict, List + + +def form_data_args() -> Tuple[Dict[str, List[bytes]], Dict[str, List[HTTPFile]]]: + """Return two empty dicts suitable for use with parse_multipart_form_data. + + mypy insists on type annotations for dict literals, so this lets us avoid + the verbose types throughout this test. + """ + return {}, {} + class TestUrlConcat(unittest.TestCase): def test_url_concat_no_query_params(self): @@ -122,8 +133,7 @@ Content-Disposition: form-data; name="files"; filename="ab.txt" Foo --1234--""".replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() parse_multipart_form_data(b"1234", data, args, files) file = files["files"][0] self.assertEqual(file["filename"], "ab.txt") @@ -137,8 +147,7 @@ Content-Disposition: form-data; name=files; filename=ab.txt Foo --1234--""".replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() parse_multipart_form_data(b"1234", data, args, files) file = files["files"][0] self.assertEqual(file["filename"], "ab.txt") @@ -155,15 +164,14 @@ Foo ] for filename in filenames: logging.debug("trying filename %r", filename) - data = """\ + str_data = """\ --1234 Content-Disposition: form-data; name="files"; filename="%s" Foo --1234--""" % filename.replace('\\', '\\\\').replace('"', '\\"') - data = utf8(data.replace("\n", "\r\n")) - args = {} - files = {} + data = utf8(str_data.replace("\n", "\r\n")) + args, files = form_data_args() parse_multipart_form_data(b"1234", data, args, files) file = files["files"][0] self.assertEqual(file["filename"], filename) @@ -176,8 +184,7 @@ Content-Disposition: form-data; name="files"; filename="ab.txt"; filename*=UTF-8 Foo --1234--""".replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() parse_multipart_form_data(b"1234", data, args, files) file = files["files"][0] self.assertEqual(file["filename"], u"áb.txt") @@ -190,8 +197,7 @@ Content-Disposition: form-data; name="files"; filename="ab.txt" Foo --1234--'''.replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() parse_multipart_form_data(b'"1234"', data, args, files) file = files["files"][0] self.assertEqual(file["filename"], "ab.txt") @@ -203,8 +209,7 @@ Foo Foo --1234--'''.replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() with ExpectLog(gen_log, "multipart/form-data missing headers"): parse_multipart_form_data(b"1234", data, args, files) self.assertEqual(files, {}) @@ -216,8 +221,7 @@ Content-Disposition: invalid; name="files"; filename="ab.txt" Foo --1234--'''.replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() with ExpectLog(gen_log, "Invalid multipart/form-data"): parse_multipart_form_data(b"1234", data, args, files) self.assertEqual(files, {}) @@ -228,8 +232,7 @@ Foo Content-Disposition: form-data; name="files"; filename="ab.txt" Foo--1234--'''.replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() with ExpectLog(gen_log, "Invalid multipart/form-data"): parse_multipart_form_data(b"1234", data, args, files) self.assertEqual(files, {}) @@ -241,8 +244,7 @@ Content-Disposition: form-data; filename="ab.txt" Foo --1234--""".replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() with ExpectLog(gen_log, "multipart/form-data value missing name"): parse_multipart_form_data(b"1234", data, args, files) self.assertEqual(files, {}) @@ -258,8 +260,7 @@ Content-Disposition: form-data; name="files"; filename="ab.txt" Foo --1234-- """.replace(b"\n", b"\r\n") - args = {} - files = {} + args, files = form_data_args() parse_multipart_form_data(b"1234", data, args, files) file = files["files"][0] self.assertEqual(file["filename"], "ab.txt") @@ -436,7 +437,7 @@ class HTTPServerRequestTest(unittest.TestCase): self.assertIsInstance(requets.body, bytes) def test_repr_does_not_contain_headers(self): - request = HTTPServerRequest(uri='/', headers={'Canary': 'Coal Mine'}) + request = HTTPServerRequest(uri='/', headers=HTTPHeaders({'Canary': ['Coal Mine']})) self.assertTrue('Canary' not in repr(request)) diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index 59b13ad0..57682088 100644 --- a/tornado/test/util_test.py +++ b/tornado/test/util_test.py @@ -12,6 +12,12 @@ from tornado.util import ( timedelta_to_seconds, import_object, re_unescape, is_finalizing ) +import typing +from typing import cast + +if typing.TYPE_CHECKING: + from typing import Dict, Any # noqa: F401 + class RaiseExcInfoTest(unittest.TestCase): def test_two_arg_exception(self): @@ -94,15 +100,18 @@ class ConfigurableTest(unittest.TestCase): obj = TestConfig1(a=1) self.assertEqual(obj.a, 1) - obj = TestConfig2(b=2) - self.assertEqual(obj.b, 2) + obj2 = TestConfig2(b=2) + self.assertEqual(obj2.b, 2) def test_default(self): - obj = TestConfigurable() + # In these tests we combine a typing.cast to satisfy mypy with + # a runtime type-assertion. Without the cast, mypy would only + # let us access attributes of the base class. + obj = cast(TestConfig1, TestConfigurable()) self.assertIsInstance(obj, TestConfig1) self.assertIs(obj.a, None) - obj = TestConfigurable(a=1) + obj = cast(TestConfig1, TestConfigurable(a=1)) self.assertIsInstance(obj, TestConfig1) self.assertEqual(obj.a, 1) @@ -110,11 +119,11 @@ class ConfigurableTest(unittest.TestCase): def test_config_class(self): TestConfigurable.configure(TestConfig2) - obj = TestConfigurable() + obj = cast(TestConfig2, TestConfigurable()) self.assertIsInstance(obj, TestConfig2) self.assertIs(obj.b, None) - obj = TestConfigurable(b=2) + obj = cast(TestConfig2, TestConfigurable(b=2)) self.assertIsInstance(obj, TestConfig2) self.assertEqual(obj.b, 2) @@ -122,11 +131,11 @@ class ConfigurableTest(unittest.TestCase): def test_config_str(self): TestConfigurable.configure('tornado.test.util_test.TestConfig2') - obj = TestConfigurable() + obj = cast(TestConfig2, TestConfigurable()) self.assertIsInstance(obj, TestConfig2) self.assertIs(obj.b, None) - obj = TestConfigurable(b=2) + obj = cast(TestConfig2, TestConfigurable(b=2)) self.assertIsInstance(obj, TestConfig2) self.assertEqual(obj.b, 2) @@ -134,11 +143,11 @@ class ConfigurableTest(unittest.TestCase): def test_config_args(self): TestConfigurable.configure(None, a=3) - obj = TestConfigurable() + obj = cast(TestConfig1, TestConfigurable()) self.assertIsInstance(obj, TestConfig1) self.assertEqual(obj.a, 3) - obj = TestConfigurable(42, a=4) + obj = cast(TestConfig1, TestConfigurable(42, a=4)) self.assertIsInstance(obj, TestConfig1) self.assertEqual(obj.a, 4) self.assertEqual(obj.pos_arg, 42) @@ -150,11 +159,11 @@ class ConfigurableTest(unittest.TestCase): def test_config_class_args(self): TestConfigurable.configure(TestConfig2, b=5) - obj = TestConfigurable() + obj = cast(TestConfig2, TestConfigurable()) self.assertIsInstance(obj, TestConfig2) self.assertEqual(obj.b, 5) - obj = TestConfigurable(42, b=6) + obj = cast(TestConfig2, TestConfigurable(42, b=6)) self.assertIsInstance(obj, TestConfig2) self.assertEqual(obj.b, 6) self.assertEqual(obj.pos_arg, 42) @@ -166,15 +175,15 @@ class ConfigurableTest(unittest.TestCase): def test_config_multi_level(self): TestConfigurable.configure(TestConfig3, a=1) - obj = TestConfigurable() + obj = cast(TestConfig3A, TestConfigurable()) self.assertIsInstance(obj, TestConfig3A) self.assertEqual(obj.a, 1) TestConfigurable.configure(TestConfig3) TestConfig3.configure(TestConfig3B, b=2) - obj = TestConfigurable() - self.assertIsInstance(obj, TestConfig3B) - self.assertEqual(obj.b, 2) + obj2 = cast(TestConfig3B, TestConfigurable()) + self.assertIsInstance(obj2, TestConfig3B) + self.assertEqual(obj2.b, 2) def test_config_inner_level(self): # The inner level can be used even when the outer level @@ -187,12 +196,12 @@ class ConfigurableTest(unittest.TestCase): self.assertIsInstance(obj, TestConfig3B) # Configuring the base doesn't configure the inner. - obj = TestConfigurable() - self.assertIsInstance(obj, TestConfig1) + obj2 = TestConfigurable() + self.assertIsInstance(obj2, TestConfig1) TestConfigurable.configure(TestConfig2) - obj = TestConfigurable() - self.assertIsInstance(obj, TestConfig2) + obj3 = TestConfigurable() + self.assertIsInstance(obj3, TestConfig2) obj = TestConfig3() self.assertIsInstance(obj, TestConfig3B) @@ -224,14 +233,14 @@ class ArgReplacerTest(unittest.TestCase): def test_omitted(self): args = (1, 2) - kwargs = dict() + kwargs = dict() # type: Dict[str, Any] self.assertIs(self.replacer.get_old_value(args, kwargs), None) self.assertEqual(self.replacer.replace('new', args, kwargs), (None, (1, 2), dict(callback='new'))) def test_position(self): args = (1, 2, 'old', 3) - kwargs = dict() + kwargs = dict() # type: Dict[str, Any] self.assertEqual(self.replacer.get_old_value(args, kwargs), 'old') self.assertEqual(self.replacer.replace('new', args, kwargs), ('old', [1, 2, 'new', 3], dict())) diff --git a/tornado/util.py b/tornado/util.py index 9991b3a5..c1013311 100644 --- a/tornado/util.py +++ b/tornado/util.py @@ -19,14 +19,14 @@ import typing import zlib from typing import ( - Any, Optional, Dict, Mapping, List, Tuple, Match, Callable, Type, + Any, Optional, Dict, Mapping, List, Tuple, Match, Callable, Type, Sequence ) if typing.TYPE_CHECKING: # Additional imports only used in type comments. # This lets us make these imports lazy. import datetime # noqa - import types # noqa + from types import TracebackType # noqa from typing import Union # noqa import unittest # noqa @@ -154,14 +154,24 @@ def exec_in(code: Any, glob: Dict[str, Any], loc: Mapping[str, Any]=None) -> Non exec(code, glob, loc) -def raise_exc_info(exc_info): - # type: (Tuple[type, BaseException, types.TracebackType]) -> typing.NoReturn +def raise_exc_info( + exc_info, # type: Tuple[Optional[type], Optional[BaseException], Optional[TracebackType]] +): + # type: (...) -> typing.NoReturn + # + # This function's type annotation must use comments instead of + # real annotations because typing.NoReturn does not exist in + # python 3.5's typing module. The formatting is funky because this + # is apparently what flake8 wants. try: - raise exc_info[1].with_traceback(exc_info[2]) + if exc_info[1] is not None: + raise exc_info[1].with_traceback(exc_info[2]) + else: + raise TypeError("raise_exc_info called with no exception") finally: # Clear the traceback reference from our stack frame to # minimize circular references that slow down GC. - exc_info = None # type: ignore + exc_info = (None, None, None) def errno_from_exception(e: BaseException) -> Optional[int]: @@ -367,7 +377,7 @@ class ArgReplacer(object): return code.co_varnames[:code.co_argcount] raise - def get_old_value(self, args: List[Any], kwargs: Dict[str, Any], default: Any=None) -> Any: + def get_old_value(self, args: Sequence[Any], kwargs: Dict[str, Any], default: Any=None) -> Any: """Returns the old value of the named argument without replacing it. Returns ``default`` if the argument is not present. @@ -377,8 +387,8 @@ class ArgReplacer(object): else: return kwargs.get(self.name, default) - def replace(self, new_value: Any, args: List[Any], - kwargs: Dict[str, Any]) -> Tuple[Any, List[Any], Dict[str, Any]]: + def replace(self, new_value: Any, args: Sequence[Any], + kwargs: Dict[str, Any]) -> Tuple[Any, Sequence[Any], Dict[str, Any]]: """Replace the named argument in ``args, kwargs`` with ``new_value``. Returns ``(old_value, args, kwargs)``. The returned ``args`` and