diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index 0429feee83..4432bb1758 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -5,7 +5,6 @@ from tornado.web import Application import asyncio import contextlib -import inspect import gc import os import platform @@ -118,7 +117,11 @@ def tearDown(self): super().tearDown() -class AsyncTestCaseWrapperTest(unittest.TestCase): +class AsyncTestCaseReturnAssertionsTest(unittest.TestCase): + # These tests verify that tests that return non-None values (without being decorated with + # @gen_test) raise errors instead of incorrectly succeeding. These tests should be removed or + # updated when the _callTestMethod method is removed from AsyncTestCase (the same checks will + # still happen, but they'll be performed in the stdlib as DeprecationWarnings) def test_undecorated_generator(self): class Test(AsyncTestCase): def test_gen(self): @@ -135,7 +138,10 @@ def test_gen(self): "pypy destructor warnings cannot be silenced", ) @unittest.skipIf( - sys.version_info >= (3, 12), "py312 has its own check for test case returns" + # This check actually exists in 3.11 but it changed in 3.12 in a way that breaks + # this test. + sys.version_info >= (3, 12), + "py312 has its own check for test case returns", ) def test_undecorated_coroutine(self): class Test(AsyncTestCase): @@ -176,17 +182,6 @@ def test_other_return(self): self.assertEqual(len(result.errors), 1) self.assertIn("Return value from test method ignored", result.errors[0][1]) - def test_unwrap(self): - class Test(AsyncTestCase): - def test_foo(self): - pass - - test = Test("test_foo") - self.assertIs( - inspect.unwrap(test.test_foo), - test.test_foo.orig_method, # type: ignore[attr-defined] - ) - class SetUpTearDownTest(unittest.TestCase): def test_set_up_tear_down(self): diff --git a/tornado/testing.py b/tornado/testing.py index bdbff87bc3..4c33b3e22f 100644 --- a/tornado/testing.py +++ b/tornado/testing.py @@ -84,39 +84,6 @@ def get_async_test_timeout() -> float: return 5 -class _TestMethodWrapper(object): - """Wraps a test method to raise an error if it returns a value. - - This is mainly used to detect undecorated generators (if a test - method yields it must use a decorator to consume the generator), - but will also detect other kinds of return values (these are not - necessarily errors, but we alert anyway since there is no good - reason to return a value from a test). - """ - - def __init__(self, orig_method: Callable) -> None: - self.orig_method = orig_method - self.__wrapped__ = orig_method - - def __call__(self, *args: Any, **kwargs: Any) -> None: - result = self.orig_method(*args, **kwargs) - if isinstance(result, Generator) or inspect.iscoroutine(result): - raise TypeError( - "Generator and coroutine test methods should be" - " decorated with tornado.testing.gen_test" - ) - elif result is not None: - raise ValueError("Return value from test method ignored: %r" % result) - - def __getattr__(self, name: str) -> Any: - """Proxy all unknown attributes to the original method. - - This is important for some of the decorators in the `unittest` - module, such as `unittest.skipIf`. - """ - return getattr(self.orig_method, name) - - class AsyncTestCase(unittest.TestCase): """`~unittest.TestCase` subclass for testing `.IOLoop`-based asynchronous code. @@ -173,12 +140,6 @@ def __init__(self, methodName: str = "runTest") -> None: self.__stop_args = None # type: Any self.__timeout = None # type: Optional[object] - # It's easy to forget the @gen_test decorator, but if you do - # the test will silently be ignored because nothing will consume - # the generator. Replace the test method with a wrapper that will - # make sure it's not an undecorated generator. - setattr(self, methodName, _TestMethodWrapper(getattr(self, methodName))) - # Not used in this class itself, but used by @gen_test self._test_generator = None # type: Optional[Union[Generator, Coroutine]] @@ -289,6 +250,30 @@ def run( self.__rethrow() return ret + def _callTestMethod(self, method: Callable) -> None: + """Run the given test method, raising an error if it returns non-None. + + Failure to decorate asynchronous test methods with ``@gen_test`` can lead to tests + incorrectly passing. + + Remove this override when Python 3.10 support is dropped. This check (in the form of a + DeprecationWarning) became a part of the standard library in 3.11. + + Note that ``_callTestMethod`` is not documented as a public interface. However, it is + present in all supported versions of Python (3.8+), and if it goes away in the future that's + OK because we can just remove this override as noted above. + """ + # Calling super()._callTestMethod would hide the return value, even in python 3.8-3.10 + # where the check isn't being done for us. + result = method() + if isinstance(result, Generator) or inspect.iscoroutine(result): + raise TypeError( + "Generator and coroutine test methods should be" + " decorated with tornado.testing.gen_test" + ) + elif result is not None: + raise ValueError("Return value from test method ignored: %r" % result) + def stop(self, _arg: Any = None, **kwargs: Any) -> None: """Stops the `.IOLoop`, causing one pending (or future) call to `wait()` to return.