[issue20510] Test cases in test_sys don't match the comments
Zachary Ware added the comment: I considered doing a test like that, but figured it was pretty well covered by the assert_python_ok no-arg test. On the other hand, it looks like we document that the default value of SystemExit().code is None, so it should be tested. I'll add it in. Thanks, Serhiy! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Roundup Robot added the comment: New changeset 63f0a1e95d2b by Zachary Ware in branch '2.7': Issue #20510: Rewrote test_exit in test_sys to match existing comments http://hg.python.org/cpython/rev/63f0a1e95d2b New changeset fa81f6ddd60e by Zachary Ware in branch '3.3': Issue #20510: Rewrote test_exit in test_sys to match existing comments http://hg.python.org/cpython/rev/fa81f6ddd60e New changeset 2438fbf4ad9d by Zachary Ware in branch 'default': Issue #20510: Merge with 3.3. http://hg.python.org/cpython/rev/2438fbf4ad9d -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Zachary Ware added the comment: Fixed, thanks for the report and patch! And btw, you are right to avoid while we're in there changes in general, but modernizing the test suite gets a little bit of leniency in that regard. It wouldn't have been appropriate to venture outside of test_exit in this issue, but test_exit was terribly outdated and needed some fixing. -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Serhiy Storchaka added the comment: What about following test? with self.assertRaises(SystemExit) as cm: sys.exit() self.assertIsNone(cm.exception.code) -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Zachary Ware added the comment: The newer patch looks good to me, I'll get it committed as soon as I can test it. Thanks! -- assignee: - zach.ware ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
New submission from Gareth Rees: Lib/test/test_sys.py contains test cases with incorrect comments -- or comments with incorrect test cases, if you prefer: # call without argument try: sys.exit(0) except SystemExit as exc: self.assertEqual(exc.code, 0) ... # call with tuple argument with one entry # entry will be unpacked try: sys.exit(42) except SystemExit as exc: self.assertEqual(exc.code, 42) ... # call with integer argument try: sys.exit((42,)) except SystemExit as exc: self.assertEqual(exc.code, 42) ... (In the quote above I've edited out some inessential detail; see the file if you really want to know.) You can see that in the first test case sys.exit is called with an argument (although the comment claims otherwise); in the second it is called with an integer (not a tuple), and in the third it is called with a tuple (not an integer). These comments have been unchanged since the original commit by Walter Dörwald http://hg.python.org/cpython/rev/6a1394660270. I've attached a patch that corrects the first test case and swaps the comments for the second and third test cases: # call without argument rc = subprocess.call([sys.executable, -c, import sys; sys.exit()]) self.assertEqual(rc, 0) # call with integer argument try: sys.exit(42) except SystemExit as exc: self.assertEqual(exc.code, 42) ... # call with tuple argument with one entry # entry will be unpacked try: sys.exit((42,)) except SystemExit as exc: self.assertEqual(exc.code, 42) ... Note that in the first test case (without an argument) sys.exit() with no argument actually raises SystemExit(None), so it's not sufficient to catch the SystemExit and check exc.code; I need to check that it actually gets translated to 0 on exit. -- components: Tests files: exittest.patch keywords: patch messages: 210246 nosy: Gareth.Rees priority: normal severity: normal status: open title: Test cases in test_sys don't match the comments type: enhancement versions: Python 3.4 Added file: http://bugs.python.org/file33908/exittest.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Changes by Zachary Ware zachary.w...@gmail.com: -- nosy: +zach.ware ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Zachary Ware added the comment: Good catch! See my review comments. Also, if you haven't already, could you please sign a contributor's agreement? See http://www.python.org/psf/contrib/. -- stage: - patch review versions: +Python 2.7, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20510] Test cases in test_sys don't match the comments
Gareth Rees added the comment: I normally try not to make changes while we're in here for fear of introducing errors! But I guess the test cases are less critical, so I've taken your review comments as a license to submit a revised patch that: * incorporates your suggestion to use assert_python_ok from test.script_helper, instead of subprocess.call; * replaces the other uses of subprocess.call with assert_python_failure and adds a check on stdout; * cleans up the assertion-testing code using the context manager form of unittest.TestCase.assertRaises. I've signed and submitted a contributor agreement as requested. -- Added file: http://bugs.python.org/file33914/exittest-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20510 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
