Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unittest should understand SkipTest at import time during test discovery #61139

Closed
brettcannon opened this issue Jan 11, 2013 · 20 comments
Closed
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

brettcannon commented Jan 11, 2013

BPO 16935
Nosy @brettcannon, @vstinner, @ezio-melotti, @merwok, @voidspace, @asvetlov, @cjerdonek, @zware
PRs
  • bpo-30813: Fix unittest when hunting refleaks #2502
  • [3.6] bpo-30813: Fix unittest when hunting refleaks (#2502) #2505
  • [3.5] bpo-30813: Fix unittest when hunting refleaks (#2502) #2506
  • Files
  • issue16935.diff: Allow SkipTest to be raised at module level during discovery
  • issue16935.v2.diff: Version 2, with test and doc change
  • issue16935.v3.diff: Version 3
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ezio-melotti'
    closed_at = <Date 2013-03-01.12:49:23.465>
    created_at = <Date 2013-01-11.14:43:08.605>
    labels = ['type-bug', 'library']
    title = 'unittest should understand SkipTest at import time during test discovery'
    updated_at = <Date 2017-06-30.11:39:51.110>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2017-06-30.11:39:51.110>
    actor = 'pitrou'
    assignee = 'ezio.melotti'
    closed = True
    closed_date = <Date 2013-03-01.12:49:23.465>
    closer = 'ezio.melotti'
    components = ['Library (Lib)']
    creation = <Date 2013-01-11.14:43:08.605>
    creator = 'brett.cannon'
    dependencies = []
    files = ['28884', '29039', '29237']
    hgrepos = []
    issue_num = 16935
    keywords = ['patch']
    message_count = 20.0
    messages = ['179681', '179682', '179700', '179770', '179903', '179904', '179946', '179952', '180877', '181864', '181902', '181909', '182969', '183122', '183256', '183257', '183258', '297375', '297385', '297389']
    nosy_count = 10.0
    nosy_names = ['brett.cannon', 'vstinner', 'ezio.melotti', 'eric.araujo', 'Arfrever', 'michael.foord', 'asvetlov', 'chris.jerdonek', 'python-dev', 'zach.ware']
    pr_nums = ['2502', '2505', '2506']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue16935'
    versions = ['Python 3.4']

    @brettcannon
    Copy link
    Member Author

    brettcannon commented Jan 11, 2013

    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

    @brettcannon brettcannon added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 11, 2013
    @voidspace
    Copy link
    Contributor

    voidspace commented Jan 11, 2013

    Agreed and it should be easy to implement.

    @merwok
    Copy link
    Member

    merwok commented Jan 11, 2013

    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).

    @ezio-melotti
    Copy link
    Member

    ezio-melotti commented Jan 12, 2013

    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.

    @cjerdonek
    Copy link
    Member

    cjerdonek commented Jan 14, 2013

    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()

    @cjerdonek
    Copy link
    Member

    cjerdonek commented Jan 14, 2013

    Should be: "module = support.import_module('module')"

    @pitrou
    Copy link
    Member

    pitrou commented Jan 14, 2013

    Still, raising SkipTest from the toplevel is useful when some toplevel setup code otherwise depends on the missing module.

    @zware
    Copy link
    Member

    zware commented Jan 14, 2013

    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 bpo-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.

    @zware
    Copy link
    Member

    zware commented Jan 28, 2013

    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]
    """

    @voidspace
    Copy link
    Contributor

    voidspace commented Feb 11, 2013

    The patch looks good - can you add a test and documentation for this?

    @zware
    Copy link
    Member

    zware commented Feb 11, 2013

    Sure can. With a little luck, I'll have the patch ready later today; with less luck it'll be sometime later this week.

    @zware
    Copy link
    Member

    zware commented Feb 11, 2013

    I think this patch should cover the test and Doc changes necessary. Of course, let me know if it doesn't :)

    @zware
    Copy link
    Member

    zware commented Feb 25, 2013

    Thanks for the review, Ezio. I've made some changes and here's the new patch.

    @ezio-melotti
    Copy link
    Member

    ezio-melotti commented Feb 27, 2013

    LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 1, 2013

    New changeset 61ce6deb4577 by Ezio Melotti in branch 'default':
    bpo-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

    @ezio-melotti
    Copy link
    Member

    ezio-melotti commented Mar 1, 2013

    Applied, thanks for the patch!
    SkipTest should probably be documented, but that's a separate issue :)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 1, 2013

    New changeset 22b6b59c70e6 by Ezio Melotti in branch 'default':
    bpo-16935: update test_crypt now that unittest discover understands SkipTest.
    http://hg.python.org/cpython/rev/22b6b59c70e6

    @vstinner
    Copy link
    Member

    vstinner commented Jun 30, 2017

    New changeset e4f9a2d by Victor Stinner in branch 'master':
    bpo-30813: Fix unittest when hunting refleaks (bpo-2502)
    e4f9a2d

    @vstinner
    Copy link
    Member

    vstinner commented Jun 30, 2017

    New changeset 714afcc by Victor Stinner in branch '3.5':
    bpo-30813: Fix unittest when hunting refleaks (bpo-2502) (bpo-2506)
    714afcc

    @vstinner
    Copy link
    Member

    vstinner commented Jun 30, 2017

    New changeset 22d4e8f by Victor Stinner in branch '3.6':
    bpo-30813: Fix unittest when hunting refleaks (bpo-2502) (bpo-2505)
    22d4e8f

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants