[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: I forgot to merge branches? Thanks Ned. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: Related issues: issue19492 and issue19493. -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Roundup Robot added the comment: New changeset 1feeeb8992f8 by Serhiy Storchaka in branch '3.3': Issue #18702: All skipped tests now reported as skipped. http://hg.python.org/cpython/rev/1feeeb8992f8 New changeset 09105051b9f4 by Serhiy Storchaka in branch 'default': Issue #18702: All skipped tests now reported as skipped. http://hg.python.org/cpython/rev/09105051b9f4 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Roundup Robot added the comment: New changeset 2d330f7908e7 by Serhiy Storchaka in branch '2.7': Issue #18702: All skipped tests now reported as skipped. http://hg.python.org/cpython/rev/2d330f7908e7 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Roundup Robot added the comment: New changeset 0d8f0526813f by Serhiy Storchaka in branch '2.7': Fix test_os (issue #18702). http://hg.python.org/cpython/rev/0d8f0526813f -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Roundup Robot added the comment: New changeset a699550bc73b by Ned Deily in branch 'default': Issue #18702 null merge http://hg.python.org/cpython/rev/a699550bc73b -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: Patch is rebased to tip. -- Added file: http://bugs.python.org/file32245/skip_tests_3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Ned Deily added the comment: I took a quick look at the revised patch. Clearly, it's too big to review at a detailed level. I tried the approach of running two complete verbose regrtest runs (-m test -v -uall), before and after the patch, and diffing the results in a smart diff app. Again, this is extremely tedious: while some simple filtering could be done to mask some differences (like elapsed times and time stamps), the non-deterministic output of many tests remains (and demonstrates how difficult it currently is to automatically compare regrtest run results). I did a spot check of the results and didn't notice any glaring problems and *did* notice that some tests were now being reported as skipped, as expected. Overall, I think this is a useful change to the test suite so I would say apply it to default (for 3.4.0) but I'm ambivalent about applying it to 3.3 at this point. -- nosy: +ned.deily ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Changes by Zachary Ware zachary.w...@gmail.com: -- nosy: +zach.ware ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Changes by STINNER Victor victor.stin...@gmail.com: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: I'm not sure I understand you Rietveld comment right Terry. We can get rid of _have_ssl (this is implementation detail and shouldn't be required) and just try import ssl. try: import ssl except ImportError: ssl = None If ssl is not None but nntplib.NNTP_SSL doesn't exist the NetworkedNNTP_SSLTests tests will failed: == ERROR: setUpClass (test.test_nntplib.NetworkedNNTP_SSLTests) -- Traceback (most recent call last): File /home/serhiy/py/cpython/Lib/test/test_nntplib.py, line 298, in setUpClass cls.server = cls.NNTP_CLASS(cls.NNTP_HOST, timeout=TIMEOUT, usenetrc=False) TypeError: 'NoneType' object is not callable -- This doesn't different from a case when some exception is raised in NNTP_SSL constructor. We can add a separate test in MiscTests: @unittest.skipUnless(ssl, 'requires SSL support') def test_ssl_support(self): self.assertTrue(hasattr(nntplib, 'NNTP_SSL')) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Terry J. Reedy added the comment: You changed NNTP_CLASS = nntplib.NNTP_SSL, which could potentially fail, to NNTP_CLASS = getattr(nntplib, 'NNTP_SSL', None), which cannot fail. Since that was the only thing that previously could fail, the change leaves nothing that can fail, so the test is not a test. I suggested that you either not add the third param, a default, or that you remove the null test completely. If particular, if the only chunk of code in nntplib that is currently being tested is being executed by some other test, then do the latter. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: You changed NNTP_CLASS = nntplib.NNTP_SSL, which could potentially fail, to NNTP_CLASS = getattr(nntplib, 'NNTP_SSL', None), which cannot fail. Since that was the only thing that previously could fail, the change leaves nothing that can fail, so the test is not a test. This statement is not a part of a test. It runs at module import time. As you said in msg194907 the failing here will prohibit the running of all tests (even not SSL related). I suggested that you either not add the third param, a default, getattr(nntplib, 'NNTP_SSL') is same as nntplib.NNTP_SSL. And it will break all tests. or that you remove the null test completely. The purpose of this issue is stopping silence skipping tests. If some tests skipped for some reasons it should be reported. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Terry J. Reedy added the comment: My original suggestion was to put the possibly failing assignment inside @classmethod def setUpClass: NNTP_CLASS = nntplib.NNTP_SSL so that failure of the assignment would be be reported, but not affect import. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: Actually you should call parent's setUpClass at the and of declared setUpClass. Therefore you need 4 nontrivial lines instead of 1. The test will fail in setUpClass() in any case because parent's setUpClass() uses NNTP_CLASS. There are no behavior difference between this variants, the variant with getattr just is shorter. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: Which one is the preferred way to go? requires or test needs or needs? Or it does not matter? I used the wording which used in other skips in the same file or in similar skips in other files. If it matters I will correct messages. Which one is the preferred way to go? With or without parentheses? Or it does not matter? Thank you for note. I will add missed parentheses to function names. There are existing skip messages which uses function names without parentheses. I left them untouched (the patch already is a large enough). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Terry J. Reedy added the comment: The patch applies cleanly on my 3.4 Win 7, fresh debug build. Somewhat fortuitously, as it turns out, I have not downloaded the files needed for ssl support. For each modified file, I ran python_d -m test -v test_xxx test test_nntplib crashed -- Traceback (most recent call last): File F:\Python\dev\py34\lib\test\regrtest.py, line 1298, in runtest_inner the_module = importlib.import_module(abstest) File F:\Python\dev\py34\lib\importlib\__init__.py, line 93, in import_module return _bootstrap._gcd_import(name[level:], package, level) File frozen importlib._bootstrap, line 1613, in _gcd_import File frozen importlib._bootstrap, line 1594, in _find_and_load File frozen importlib._bootstrap, line 1561, in _find_and_load_unlocked File frozen importlib._bootstrap, line 607, in _check_name_wrapper File frozen importlib._bootstrap, line 1056, in load_module File frozen importlib._bootstrap, line 926, in load_module File frozen importlib._bootstrap, line 274, in _call_with_frames_removed File F:\Python\dev\py34\lib\test\test_nntplib.py, line 305, in module class NetworkedNNTP_SSLTests(NetworkedNNTPTests): File F:\Python\dev\py34\lib\test\test_nntplib.py, line 315, in NetworkedNNTP_SSLTests NNTP_CLASS = nntplib.NNTP_SSL AttributeError: 'module' object has no attribute 'NNTP_SSL' I do not understand the frozen importlib stuff, but nntplib._have_ssl is False, so the line asking for the non-existent attribute should not be executed. The problem is that changing a guard from 'if hasattr' to the decorator is a *semantic change* in that it allows execution of the guarded statement. Skips only skip function calling. For a function statement, creating a function object from the compiled code object and binding a name is trivial. Not calling the function is the important part. For a class with code other than function definitions, the difference is crucial, as the above shows. The general solution is to put class code inside a function so it is only executed later, as part of the test process, rather than during inport. @classmethod def setUpClass(cls): cls.NNTP_CLASS = nntplib.NNTP_SSL In this case, you could instead guard it with 'if _have_ssl', but since this assignment is the only thing being tested by this test case, it should be inside a method anyway. I suggest that you review your patch for other changed classes that might have class-level code that could fail with the change to the skip decorator. Here is similar problem: trying to subclass a class that does not exist. test test_socketserver crashed File F:\Python\dev\py34\lib\test\test_socketserver.py, line 50, in module socketserver.UnixStreamServer): AttributeError: 'module' object has no attribute 'UnixStreamServer' class ForkingUnixStreamServer(socketserver.ForkingMixIn, socketserver.UnixStreamServer): pass I think you have to go back to 'if HAVE_UNIX_SOCKETS:' for this. Or re-factor somehow. --- I cannot help wondering whether test_math.xxx.test_exceptions should still be (normally) disabled. I sent Mark Dickinson a separate note. -- nosy: +terry.reedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Serhiy Storchaka added the comment: Thank you Ezio and Terry for review. Here is updated patch. Problems with class initialization solved. I have made a lot of other changes especially in test_os.py and test_posix.py. -- Added file: http://bugs.python.org/file31233/skip_tests_2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Terry J. Reedy added the comment: Applies and runs (with text_posix entirely skipped). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
Vajrasky Kok added the comment: I read the patch. It looks good. Anyway, I have two questions: 1. I saw 3 variants of writing description for skipping test descriptor: requires os.mkfifo test needs socket.inet_pton() needs os.read Which one is the preferred way to go? requires or test needs or needs? Or it does not matter? 2. I saw 2 variants of writing the function name required for running the test. test needs socket.inet_pton() test needs socket.inet_pton Which one is the preferred way to go? With or without parentheses? Or it does not matter? -- nosy: +vajrasky ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue18702] Report skipped tests as skipped
New submission from Serhiy Storchaka: Some tests in Python testsuite are silently skipped if requirements is not satisfied. The proposed patch adds explicit skipUnless() and raise SkipTest() so that these tests now reported as skipped. I.e. the code like if not condition: def test_foo(self): ... is replaced by @unittest.skipUnless(condition, requires foo) def test_foo(self): ... and the code like def test_foo(self): ... if not condition: return ... is replaced by def test_foo(self): ... if not condition: raise unittest.SkipTest(requires foo) ... -- components: Tests files: skip_tests.patch keywords: patch messages: 194790 nosy: ezio.melotti, michael.foord, pitrou, serhiy.storchaka priority: normal severity: normal stage: patch review status: open title: Report skipped tests as skipped type: enhancement versions: Python 3.3, Python 3.4 Added file: http://bugs.python.org/file31213/skip_tests.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue18702 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
