From ccbe5a2787c2721f2aa0b7f5f08f89183aef2415 Mon Sep 17 00:00:00 2001 From: Akihiro Yamazaki Date: Thu, 27 Mar 2014 09:37:29 +0900 Subject: [PATCH 1/3] put assertion outside of the exception handler --- tornado/test/concurrent_test.py | 2 +- tornado/test/httpclient_test.py | 5 +++-- tornado/test/ioloop_test.py | 3 ++- tornado/test/template_test.py | 22 ++++++++++++---------- tornado/test/testing_test.py | 11 ++++++----- tornado/test/util_test.py | 3 ++- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py index 5e93ad6a..2506ef45 100644 --- a/tornado/test/concurrent_test.py +++ b/tornado/test/concurrent_test.py @@ -168,7 +168,7 @@ class ReturnFutureTest(AsyncTestCase): self.fail("didn't get expected exception") except ZeroDivisionError: tb = traceback.extract_tb(sys.exc_info()[2]) - self.assertIn(self.expected_frame, tb) + self.assertIn(self.expected_frame, tb) # The following series of classes demonstrate and test various styles # of use, with and without generators and futures. diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 7ffddbc3..984f5e0c 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -359,8 +359,9 @@ Transfer-Encoding: chunked try: yield self.http_client.fetch(self.get_url('/notfound')) except HTTPError as e: - self.assertEqual(e.code, 404) - self.assertEqual(e.response.code, 404) + got_exception = e + self.assertEqual(got_exception.code, 404) + self.assertEqual(got_exception.response.code, 404) @gen_test def test_reuse_request_from_response(self): diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py index ff26bde1..bd069c24 100644 --- a/tornado/test/ioloop_test.py +++ b/tornado/test/ioloop_test.py @@ -116,8 +116,9 @@ class TestIOLoop(AsyncTestCase): try: other_ioloop.add_callback(lambda: None) except RuntimeError as e: - self.assertEqual("IOLoop is closing", str(e)) + got_exception = e break + self.assertEqual("IOLoop is closing", str(got_exception)) def test_handle_callback_exception(self): # IOLoop.handle_callback_exception can be overridden to catch diff --git a/tornado/test/template_test.py b/tornado/test/template_test.py index f3a9e059..c1b1e48e 100644 --- a/tornado/test/template_test.py +++ b/tornado/test/template_test.py @@ -183,7 +183,8 @@ three try: loader.load("test.html").generate() except ZeroDivisionError: - self.assertTrue("# test.html:2" in traceback.format_exc()) + exc_stack = traceback.format_exc() + self.assertTrue("# test.html:2" in exc_stack) def test_error_line_number_directive(self): loader = DictLoader({"test.html": """one @@ -193,7 +194,8 @@ three{%end%} try: loader.load("test.html").generate() except ZeroDivisionError: - self.assertTrue("# test.html:2" in traceback.format_exc()) + exc_stack = traceback.format_exc() + self.assertTrue("# test.html:2" in exc_stack) def test_error_line_number_module(self): loader = DictLoader({ @@ -204,8 +206,8 @@ three{%end%} loader.load("base.html").generate() except ZeroDivisionError: exc_stack = traceback.format_exc() - self.assertTrue('# base.html:1' in exc_stack) - self.assertTrue('# sub.html:1' in exc_stack) + self.assertTrue('# base.html:1' in exc_stack) + self.assertTrue('# sub.html:1' in exc_stack) def test_error_line_number_include(self): loader = DictLoader({ @@ -215,8 +217,8 @@ three{%end%} try: loader.load("base.html").generate() except ZeroDivisionError: - self.assertTrue("# sub.html:1 (via base.html:1)" in - traceback.format_exc()) + exc_stack = traceback.format_exc() + self.assertTrue("# sub.html:1 (via base.html:1)" in exc_stack) def test_error_line_number_extends_base_error(self): loader = DictLoader({ @@ -241,8 +243,8 @@ three{%end%} try: loader.load("sub.html").generate() except ZeroDivisionError: - self.assertTrue("# sub.html:4 (via base.html:1)" in - traceback.format_exc()) + exc_stack = traceback.format_exc() + self.assertTrue("# sub.html:4 (via base.html:1)" in exc_stack) def test_multi_includes(self): loader = DictLoader({ @@ -253,8 +255,8 @@ three{%end%} try: loader.load("a.html").generate() except ZeroDivisionError: - self.assertTrue("# c.html:1 (via b.html:1, a.html:1)" in - traceback.format_exc()) + exc_stack = traceback.format_exc() + self.assertTrue("# c.html:1 (via b.html:1, a.html:1)" in exc_stack) class AutoEscapeTest(unittest.TestCase): diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index aabdaced..ef545e76 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -155,11 +155,12 @@ class GenTest(AsyncTestCase): test(self) self.fail("did not get expected exception") except ioloop.TimeoutError: - # The stack trace should blame the add_timeout line, not just - # unrelated IOLoop/testing internals. - self.assertIn( - "gen.Task(self.io_loop.add_timeout, self.io_loop.time() + 1)", - traceback.format_exc()) + exc_stack = traceback.format_exc() + # The stack trace should blame the add_timeout line, not just + # unrelated IOLoop/testing internals. + self.assertIn( + "gen.Task(self.io_loop.add_timeout, self.io_loop.time() + 1)", + exc_stack) self.finished = True diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index 41ccbb9a..fa965fbe 100644 --- a/tornado/test/util_test.py +++ b/tornado/test/util_test.py @@ -30,7 +30,8 @@ class RaiseExcInfoTest(unittest.TestCase): raise_exc_info(exc_info) self.fail("didn't get expected exception") except TwoArgException as e: - self.assertIs(e, exc_info[1]) + got_exception = e + self.assertIs(got_exception, exc_info[1]) class TestConfigurable(Configurable): From e914a5143e8fd99c4904eedc7547d0d637d752de Mon Sep 17 00:00:00 2001 From: Akihiro Yamazaki Date: Fri, 28 Mar 2014 00:27:39 +0900 Subject: [PATCH 2/3] use "with self.assertRaises" pattern --- tornado/test/concurrent_test.py | 6 ++--- tornado/test/httpclient_test.py | 8 +++--- tornado/test/ioloop_test.py | 8 ++---- tornado/test/template_test.py | 45 ++++++++++++--------------------- tornado/test/testing_test.py | 11 +++----- tornado/test/util_test.py | 7 ++--- 6 files changed, 28 insertions(+), 57 deletions(-) diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py index 2506ef45..edd70c9a 100644 --- a/tornado/test/concurrent_test.py +++ b/tornado/test/concurrent_test.py @@ -163,11 +163,9 @@ class ReturnFutureTest(AsyncTestCase): self.expected_frame = traceback.extract_tb( sys.exc_info()[2], limit=1)[0] raise - try: + with self.assertRaises(ZeroDivisionError): yield f() - self.fail("didn't get expected exception") - except ZeroDivisionError: - tb = traceback.extract_tb(sys.exc_info()[2]) + tb = traceback.extract_tb(sys.exc_info()[2]) self.assertIn(self.expected_frame, tb) # The following series of classes demonstrate and test various styles diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 984f5e0c..78daa74d 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -356,12 +356,10 @@ Transfer-Encoding: chunked @gen_test def test_future_http_error(self): - try: + with self.assertRaises(HTTPError) as context: yield self.http_client.fetch(self.get_url('/notfound')) - except HTTPError as e: - got_exception = e - self.assertEqual(got_exception.code, 404) - self.assertEqual(got_exception.response.code, 404) + self.assertEqual(context.exception.code, 404) + self.assertEqual(context.exception.response.code, 404) @gen_test def test_reuse_request_from_response(self): diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py index bd069c24..b7d1f3a6 100644 --- a/tornado/test/ioloop_test.py +++ b/tornado/test/ioloop_test.py @@ -112,13 +112,9 @@ class TestIOLoop(AsyncTestCase): thread = threading.Thread(target=target) thread.start() closing.wait() - for i in range(1000): - try: + with self.assertRaisesRegexp(RuntimeError, "\AIOLoop is closing\Z"): + for i in range(1000): other_ioloop.add_callback(lambda: None) - except RuntimeError as e: - got_exception = e - break - self.assertEqual("IOLoop is closing", str(got_exception)) def test_handle_callback_exception(self): # IOLoop.handle_callback_exception can be overridden to catch diff --git a/tornado/test/template_test.py b/tornado/test/template_test.py index c1b1e48e..181008dc 100644 --- a/tornado/test/template_test.py +++ b/tornado/test/template_test.py @@ -149,20 +149,14 @@ try{% set y = 1/x %} self.assertEqual(result, b"013456") def test_break_outside_loop(self): - try: + with self.assertRaises(ParseError): Template(utf8("{% break %}")) - raise Exception("Did not get expected exception") - except ParseError: - pass def test_break_in_apply(self): # This test verifies current behavior, although of course it would # be nice if apply didn't cause seemingly unrelated breakage - try: + with self.assertRaises(ParseError): Template(utf8("{% for i in [] %}{% apply foo %}{% break %}{% end %}{% end %}")) - raise Exception("Did not get expected exception") - except ParseError: - pass @unittest.skipIf(sys.version_info >= division.getMandatoryRelease(), 'no testable future imports') @@ -180,10 +174,9 @@ class StackTraceTest(unittest.TestCase): two{{1/0}} three """}) - try: + with self.assertRaises(ZeroDivisionError): loader.load("test.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# test.html:2" in exc_stack) def test_error_line_number_directive(self): @@ -191,10 +184,9 @@ three two{%if 1/0%} three{%end%} """}) - try: + with self.assertRaises(ZeroDivisionError): loader.load("test.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# test.html:2" in exc_stack) def test_error_line_number_module(self): @@ -202,10 +194,9 @@ three{%end%} "base.html": "{% module Template('sub.html') %}", "sub.html": "{{1/0}}", }, namespace={"_tt_modules": ObjectDict({"Template": lambda path, **kwargs: loader.load(path).generate(**kwargs)})}) - try: + with self.assertRaises(ZeroDivisionError): loader.load("base.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue('# base.html:1' in exc_stack) self.assertTrue('# sub.html:1' in exc_stack) @@ -214,10 +205,9 @@ three{%end%} "base.html": "{% include 'sub.html' %}", "sub.html": "{{1/0}}", }) - try: + with self.assertRaises(ZeroDivisionError): loader.load("base.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# sub.html:1 (via base.html:1)" in exc_stack) def test_error_line_number_extends_base_error(self): @@ -225,10 +215,9 @@ three{%end%} "base.html": "{{1/0}}", "sub.html": "{% extends 'base.html' %}", }) - try: + with self.assertRaises(ZeroDivisionError): loader.load("sub.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# base.html:1" in exc_stack) def test_error_line_number_extends_sub_error(self): @@ -240,10 +229,9 @@ three{%end%} {{1/0}} {% end %} """}) - try: + with self.assertRaises(ZeroDivisionError): loader.load("sub.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# sub.html:4 (via base.html:1)" in exc_stack) def test_multi_includes(self): @@ -252,10 +240,9 @@ three{%end%} "b.html": "{% include 'c.html' %}", "c.html": "{{1/0}}", }) - try: + with self.assertRaises(ZeroDivisionError): loader.load("a.html").generate() - except ZeroDivisionError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() self.assertTrue("# c.html:1 (via b.html:1, a.html:1)" in exc_stack) diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index ef545e76..92eb9a48 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -28,11 +28,8 @@ def set_environ(name, value): class AsyncTestCaseTest(AsyncTestCase): def test_exception_in_callback(self): self.io_loop.add_callback(lambda: 1 / 0) - try: + with self.assertRaises(ZeroDivisionError): self.wait() - self.fail("did not get expected exception") - except ZeroDivisionError: - pass def test_wait_timeout(self): time = self.io_loop.time @@ -151,11 +148,9 @@ class GenTest(AsyncTestCase): # This can't use assertRaises because we need to inspect the # exc_info triple (and not just the exception object) - try: + with self.assertRaises(ioloop.TimeoutError): test(self) - self.fail("did not get expected exception") - except ioloop.TimeoutError: - exc_stack = traceback.format_exc() + exc_stack = traceback.format_exc() # The stack trace should blame the add_timeout line, not just # unrelated IOLoop/testing internals. self.assertIn( diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index fa965fbe..fab94e7d 100644 --- a/tornado/test/util_test.py +++ b/tornado/test/util_test.py @@ -26,12 +26,9 @@ class RaiseExcInfoTest(unittest.TestCase): raise TwoArgException(1, 2) except TwoArgException: exc_info = sys.exc_info() - try: + with self.assertRaises(TwoArgException) as context: raise_exc_info(exc_info) - self.fail("didn't get expected exception") - except TwoArgException as e: - got_exception = e - self.assertIs(got_exception, exc_info[1]) + self.assertIs(context.exception, exc_info[1]) class TestConfigurable(Configurable): From 32c7a328ed2c00d0ec9adc62b036e1bee65ad146 Mon Sep 17 00:00:00 2001 From: Akihiro Yamazaki Date: Sun, 30 Mar 2014 23:15:46 +0900 Subject: [PATCH 3/3] correct testcases --- tornado/test/concurrent_test.py | 8 +++-- tornado/test/ioloop_test.py | 7 ++-- tornado/test/template_test.py | 64 +++++++++++++++++++++------------ tornado/test/testing_test.py | 20 ++++++----- tornado/test/util_test.py | 6 ++-- 5 files changed, 67 insertions(+), 38 deletions(-) diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py index edd70c9a..5e93ad6a 100644 --- a/tornado/test/concurrent_test.py +++ b/tornado/test/concurrent_test.py @@ -163,10 +163,12 @@ class ReturnFutureTest(AsyncTestCase): self.expected_frame = traceback.extract_tb( sys.exc_info()[2], limit=1)[0] raise - with self.assertRaises(ZeroDivisionError): + try: yield f() - tb = traceback.extract_tb(sys.exc_info()[2]) - self.assertIn(self.expected_frame, tb) + self.fail("didn't get expected exception") + except ZeroDivisionError: + tb = traceback.extract_tb(sys.exc_info()[2]) + self.assertIn(self.expected_frame, tb) # The following series of classes demonstrate and test various styles # of use, with and without generators and futures. diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py index b7d1f3a6..ff26bde1 100644 --- a/tornado/test/ioloop_test.py +++ b/tornado/test/ioloop_test.py @@ -112,9 +112,12 @@ class TestIOLoop(AsyncTestCase): thread = threading.Thread(target=target) thread.start() closing.wait() - with self.assertRaisesRegexp(RuntimeError, "\AIOLoop is closing\Z"): - for i in range(1000): + for i in range(1000): + try: other_ioloop.add_callback(lambda: None) + except RuntimeError as e: + self.assertEqual("IOLoop is closing", str(e)) + break def test_handle_callback_exception(self): # IOLoop.handle_callback_exception can be overridden to catch diff --git a/tornado/test/template_test.py b/tornado/test/template_test.py index 181008dc..32bbe421 100644 --- a/tornado/test/template_test.py +++ b/tornado/test/template_test.py @@ -149,14 +149,20 @@ try{% set y = 1/x %} self.assertEqual(result, b"013456") def test_break_outside_loop(self): - with self.assertRaises(ParseError): + try: Template(utf8("{% break %}")) + raise Exception("Did not get expected exception") + except ParseError: + pass def test_break_in_apply(self): # This test verifies current behavior, although of course it would # be nice if apply didn't cause seemingly unrelated breakage - with self.assertRaises(ParseError): + try: Template(utf8("{% for i in [] %}{% apply foo %}{% break %}{% end %}{% end %}")) + raise Exception("Did not get expected exception") + except ParseError: + pass @unittest.skipIf(sys.version_info >= division.getMandatoryRelease(), 'no testable future imports') @@ -174,50 +180,58 @@ class StackTraceTest(unittest.TestCase): two{{1/0}} three """}) - with self.assertRaises(ZeroDivisionError): + try: loader.load("test.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue("# test.html:2" in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + self.assertTrue("# test.html:2" in traceback.format_exc()) def test_error_line_number_directive(self): loader = DictLoader({"test.html": """one two{%if 1/0%} three{%end%} """}) - with self.assertRaises(ZeroDivisionError): + try: loader.load("test.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue("# test.html:2" in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + self.assertTrue("# test.html:2" in traceback.format_exc()) def test_error_line_number_module(self): loader = DictLoader({ "base.html": "{% module Template('sub.html') %}", "sub.html": "{{1/0}}", }, namespace={"_tt_modules": ObjectDict({"Template": lambda path, **kwargs: loader.load(path).generate(**kwargs)})}) - with self.assertRaises(ZeroDivisionError): + try: loader.load("base.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue('# base.html:1' in exc_stack) - self.assertTrue('# sub.html:1' in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + exc_stack = traceback.format_exc() + self.assertTrue('# base.html:1' in exc_stack) + self.assertTrue('# sub.html:1' in exc_stack) def test_error_line_number_include(self): loader = DictLoader({ "base.html": "{% include 'sub.html' %}", "sub.html": "{{1/0}}", }) - with self.assertRaises(ZeroDivisionError): + try: loader.load("base.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue("# sub.html:1 (via base.html:1)" in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + self.assertTrue("# sub.html:1 (via base.html:1)" in + traceback.format_exc()) def test_error_line_number_extends_base_error(self): loader = DictLoader({ "base.html": "{{1/0}}", "sub.html": "{% extends 'base.html' %}", }) - with self.assertRaises(ZeroDivisionError): + try: loader.load("sub.html").generate() - exc_stack = traceback.format_exc() + self.fail("did not get expected exception") + except ZeroDivisionError: + exc_stack = traceback.format_exc() self.assertTrue("# base.html:1" in exc_stack) def test_error_line_number_extends_sub_error(self): @@ -229,10 +243,12 @@ three{%end%} {{1/0}} {% end %} """}) - with self.assertRaises(ZeroDivisionError): + try: loader.load("sub.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue("# sub.html:4 (via base.html:1)" in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + self.assertTrue("# sub.html:4 (via base.html:1)" in + traceback.format_exc()) def test_multi_includes(self): loader = DictLoader({ @@ -240,10 +256,12 @@ three{%end%} "b.html": "{% include 'c.html' %}", "c.html": "{{1/0}}", }) - with self.assertRaises(ZeroDivisionError): + try: loader.load("a.html").generate() - exc_stack = traceback.format_exc() - self.assertTrue("# c.html:1 (via b.html:1, a.html:1)" in exc_stack) + self.fail("did not get expected exception") + except ZeroDivisionError: + self.assertTrue("# c.html:1 (via b.html:1, a.html:1)" in + traceback.format_exc()) class AutoEscapeTest(unittest.TestCase): diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index 92eb9a48..aabdaced 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -28,8 +28,11 @@ def set_environ(name, value): class AsyncTestCaseTest(AsyncTestCase): def test_exception_in_callback(self): self.io_loop.add_callback(lambda: 1 / 0) - with self.assertRaises(ZeroDivisionError): + try: self.wait() + self.fail("did not get expected exception") + except ZeroDivisionError: + pass def test_wait_timeout(self): time = self.io_loop.time @@ -148,14 +151,15 @@ class GenTest(AsyncTestCase): # This can't use assertRaises because we need to inspect the # exc_info triple (and not just the exception object) - with self.assertRaises(ioloop.TimeoutError): + try: test(self) - exc_stack = traceback.format_exc() - # The stack trace should blame the add_timeout line, not just - # unrelated IOLoop/testing internals. - self.assertIn( - "gen.Task(self.io_loop.add_timeout, self.io_loop.time() + 1)", - exc_stack) + self.fail("did not get expected exception") + except ioloop.TimeoutError: + # The stack trace should blame the add_timeout line, not just + # unrelated IOLoop/testing internals. + self.assertIn( + "gen.Task(self.io_loop.add_timeout, self.io_loop.time() + 1)", + traceback.format_exc()) self.finished = True diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index fab94e7d..41ccbb9a 100644 --- a/tornado/test/util_test.py +++ b/tornado/test/util_test.py @@ -26,9 +26,11 @@ class RaiseExcInfoTest(unittest.TestCase): raise TwoArgException(1, 2) except TwoArgException: exc_info = sys.exc_info() - with self.assertRaises(TwoArgException) as context: + try: raise_exc_info(exc_info) - self.assertIs(context.exception, exc_info[1]) + self.fail("didn't get expected exception") + except TwoArgException as e: + self.assertIs(e, exc_info[1]) class TestConfigurable(Configurable):