classification
Title: unittest should understand SkipTest at import time during test discovery
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: Arfrever, asvetlov, brett.cannon, chris.jerdonek, eric.araujo, ezio.melotti, michael.foord, pitrou, python-dev, zach.ware
Priority: normal Keywords: patch

Created on 2013-01-11 14:43 by brett.cannon, last changed 2013-03-01 12:54 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue16935.diff zach.ware, 2013-01-28 19:48 Allow SkipTest to be raised at module level during discovery review
issue16935.v2.diff zach.ware, 2013-02-11 16:54 Version 2, with test and doc change review
issue16935.v3.diff zach.ware, 2013-02-25 18:29 Version 3 review
Messages (17)
msg179681 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-01-11 14:43
For test discovery to work where a dependent module is optional, you end up needing to do something like what is done in http://hg.python.org/cpython/rev/15ddd683c321:

-crypt = support.import_module('crypt')
+def setUpModule():
+    # this import will raise unittest.SkipTest if _crypt doesn't exist,
+    # so it has to be done in setUpModule for test discovery to work
+    global crypt
+    crypt = support.import_module('crypt')

That's kind of ugly. It would be better if unittest recognized SkipTest at import time during test discovery
msg179682 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-01-11 14:44
Agreed and it should be easy to implement.
msg179700 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-01-11 16:29
FTR there is already an alternative to setupmodule:

try:
    import module
except ImportError:
    module = None

@unittest.skipUnless(module, 'requires module')
class ModuleTests(unittest.TestCase):
    pass


This idiom is more lines than support.import_module, but works for non-stdlib tests too, and is useful when you don’t want the whole file to be skipped if the module is missing (like in distutils’ test_sdist where zlib can be missing).
msg179770 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-12 05:34
FWIW the difference between support.import_module and the try/except/skip is that usually the former is used when *all* the tests require the imported module, whereas the latter is used when only some of the tests requires it.
msg179903 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-14 00:06
Without the proposed enhancement, you could also combine Éric's approach with the original patch by doing something like:

try:
    support.import_module(module)
except SkipTest:
    module = None

def setUpModule():
    if module is None:
        raise SkipTest()
msg179904 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-14 00:08
Should be: "module = support.import_module('module')"
msg179946 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-14 16:01
Still, raising SkipTest from the toplevel is useful when some toplevel setup code otherwise depends on the missing module.
msg179952 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-01-14 16:34
I agree that raising SkipTest (or subclasses thereof, such as ResourceDenied) at module level should be supported.  That would mean no changes would be needed in most of the should-be-skipped-but-fail-instead tests listed in issue 16748 to make test discovery play nicely, and in fact the changes to test_crypt could be mostly reverted.

Personally, I don't find either of the suggestions given as alternates to what I did in test_crypt to be particularly prettier, not that what I did is pretty either.
msg180877 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-01-28 19:48
Here's a patch that I believe nicely handles the raising of unittest.SkipTest at module level while doing test discovery.  It adds a _make_skipped_test function to unittest.loader, and an ``except case.SkipTest`` clause to TestLoader._find_tests.  For our own test package, this covers non-enabled resources as well.

Here's some example output:
"""
P:\cpython>python -m unittest discover -v Lib/test/ "test_curs*"
test_curses (unittest.loader.ModuleSkipped) ... skipped "Use of the 'curses' res
ource not enabled"

----------------------------------------------------------------------
Ran 1 test in 0.001s

OK (skipped=1)
[102289 refs, 38970 blocks]
"""
msg181864 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-02-11 00:32
The patch looks good - can you add a test and documentation for this?
msg181902 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-02-11 15:53
Sure can.  With a little luck, I'll have the patch ready later today; with less luck it'll be sometime later this week.
msg181909 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-02-11 16:54
I think this patch should cover the test and Doc changes necessary.  Of course, let me know if it doesn't :)
msg182969 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-02-25 18:29
Thanks for the review, Ezio.  I've made some changes and here's the new patch.
msg183122 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-02-27 08:16
LGTM.
msg183256 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-01 12:48
New changeset 61ce6deb4577 by Ezio Melotti in branch 'default':
#16935: unittest now counts the module as skipped if it raises SkipTest, instead of counting it as an error.  Patch by Zachary Ware.
http://hg.python.org/cpython/rev/61ce6deb4577
msg183257 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-01 12:49
Applied, thanks for the patch!
SkipTest should probably be documented, but that's a separate issue :)
msg183258 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-01 12:54
New changeset 22b6b59c70e6 by Ezio Melotti in branch 'default':
#16935: update test_crypt now that unittest discover understands SkipTest.
http://hg.python.org/cpython/rev/22b6b59c70e6
History
Date User Action Args
2013-03-01 12:54:47python-devsetmessages: + msg183258
2013-03-01 12:49:23ezio.melottisetstatus: open -> closed
messages: + msg183257

assignee: michael.foord -> ezio.melotti
keywords: - gsoc
resolution: fixed
stage: test needed -> resolved
2013-03-01 12:48:08python-devsetnosy: + python-dev
messages: + msg183256
2013-02-27 08:16:55ezio.melottisetmessages: + msg183122
2013-02-25 18:29:12zach.waresetfiles: + issue16935.v3.diff

messages: + msg182969
2013-02-11 16:54:08zach.waresetfiles: + issue16935.v2.diff

messages: + msg181909
2013-02-11 15:53:45zach.waresetmessages: + msg181902
2013-02-11 00:32:48michael.foordsetmessages: + msg181864
2013-01-28 19:48:07zach.waresetfiles: + issue16935.diff
keywords: + patch
messages: + msg180877
2013-01-14 22:10:17asvetlovsetnosy: + asvetlov
2013-01-14 16:34:47zach.waresetmessages: + msg179952
2013-01-14 16:01:23pitrousetnosy: + pitrou
messages: + msg179946
2013-01-14 00:08:42chris.jerdoneksetmessages: + msg179904
2013-01-14 00:06:27chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg179903
2013-01-13 21:43:08Arfreversetnosy: + Arfrever
2013-01-12 05:34:59ezio.melottisetnosy: + ezio.melotti
messages: + msg179770
2013-01-11 16:29:12eric.araujosetnosy: + eric.araujo
messages: + msg179700
2013-01-11 14:57:59zach.waresetnosy: + zach.ware
2013-01-11 14:44:05michael.foordsetkeywords: + gsoc

messages: + msg179682
2013-01-11 14:43:08brett.cannoncreate