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 module cleanup functions not run unless tearDownModule() is defined #88079

Closed
rtarpine mannequin opened this issue Apr 22, 2021 · 12 comments
Closed

unittest module cleanup functions not run unless tearDownModule() is defined #88079

rtarpine mannequin opened this issue Apr 22, 2021 · 12 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@rtarpine
Copy link
Mannequin

rtarpine mannequin commented Apr 22, 2021

BPO 43913
Nosy @terryjreedy, @rbtcollins, @ezio-melotti, @voidspace, @ambv, @serhiy-storchaka, @miss-islington, @miguendes
PRs
  • bpo-43913: Fix unittest module cleanup #25700
  • bpo-43913: Fix bugs in cleaning up classes and modules in unittest. #28006
  • [3.10] bpo-43913: Fix bugs in cleaning up classes and modules in unittest. (GH-28006) #28070
  • [3.9] bpo-43913: Fix bugs in cleaning up classes and modules in unitt… #28071
  • 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 = None
    closed_at = <Date 2021-08-30.18:02:06.690>
    created_at = <Date 2021-04-22.17:30:03.199>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = 'unittest module cleanup functions not run unless tearDownModule() is defined'
    updated_at = <Date 2021-08-30.18:02:06.690>
    user = 'https://bugs.python.org/rtarpine'

    bugs.python.org fields:

    activity = <Date 2021-08-30.18:02:06.690>
    actor = 'lukasz.langa'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-08-30.18:02:06.690>
    closer = 'lukasz.langa'
    components = ['Library (Lib)']
    creation = <Date 2021-04-22.17:30:03.199>
    creator = 'rtarpine'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43913
    keywords = ['patch']
    message_count = 12.0
    messages = ['391619', '391800', '392268', '392580', '392590', '392773', '400090', '400442', '400633', '400637', '400641', '400644']
    nosy_count = 10.0
    nosy_names = ['terry.reedy', 'rbcollins', 'ezio.melotti', 'michael.foord', 'lukasz.langa', 'python-dev', 'serhiy.storchaka', 'miss-islington', 'rtarpine', 'miguendes']
    pr_nums = ['25700', '28006', '28070', '28071']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue43913'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @rtarpine
    Copy link
    Mannequin Author

    rtarpine mannequin commented Apr 22, 2021

    Functions registered with unittest.addModuleCleanup are not called unless the user defines tearDownModule in their test module.

    This behavior is unexpected because functions registered with TestCase.addClassCleanup are called even the user doesn't define tearDownClass, and similarly with addCleanup/tearDown.

    The implementing code is basically the same for all 3 cases, the difference is that unittest.TestCase itself defines tearDown and tearDownClass; so even though doClassCleanups is only called if tearDownClass is defined, in practice it always is.

    doModuleCleanups should be called even if tearDownModule is not defined.

    @rtarpine rtarpine mannequin added 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Apr 22, 2021
    @terryjreedy
    Copy link
    Member

    import unittest

    #def setUpModule(): raise Exception()
    #def tearDownModule(): print('module teardown')

    unittest.addModuleCleanup(print, 'module cleanup')
    
    class Dummy(unittest.TestCase):
        def test_dummy(self):
            self.addCleanup(print, 'test cleanup')
            self.addClassCleanup(print, 'class cleanup')
    
    unittest.main()

    Above verifies the claim in 3.10.0a7. Only 'test cleanup' and 'class cleanup' print. Uncomment either function and 'module cleanup' is also printed.

    https://docs.python.org/3/library/unittest.html#unittest.addModuleCleanu
    """
    unittest.addModuleCleanup(function, /, *args, **kwargs)

    Add a function to be called after tearDownModule() to cleanup resources used during the test class. Functions will be called in reverse order to the order they are added (LIFO). They are called with any arguments and keyword arguments passed into addModuleCleanup() when they are added.
    
    If setUpModule() fails, meaning that tearDownModule() is not called, then any cleanup functions added will still be called.
    

    """
    This seems slightly ambiguous as to behavior when no tearDownModule. However, the doc for addClassCleanup is exactly parallel.

    https://docs.python.org/3/library/unittest.html#unittest.TestCase.addClassCleanup
    """
    classmethod addClassCleanup(function, /, *args, **kwargs)

    Add a function to be called after tearDownClass() to cleanup resources used during the test class. Functions will be called in reverse order to the order they are added (LIFO). They are called with any arguments and keyword arguments passed into addClassCleanup() when they are added.
    
    If setUpClass() fails, meaning that tearDownClass() is not called, then any cleanup functions added will still be called.
    

    """

    The doc for addCleanup is also parallel. The behavior difference therefore seems like a bug and calling in any case seems more correct.

    Ryan, can you prepare a PR?

    @terryjreedy terryjreedy added 3.9 only security fixes 3.10 only security fixes labels Apr 24, 2021
    @miguendes
    Copy link
    Mannequin

    miguendes mannequin commented Apr 28, 2021

    Hello, first time here. I created an PR for that. Managed to reproduce the issue both manually and via unit test.

    I hope there's no edge case but all tests pass on my machine.

    @miguendes
    Copy link
    Mannequin

    miguendes mannequin commented May 1, 2021

    I was reading through the dev guide and past issues and I didn't know it's advisable to give the author of the issue a chance to submit the PR.

    Sorry about that, you can close mine in this case.

    @terryjreedy
    Copy link
    Member

    No apology needed. I don't know what has been added to the devguide, but most OPs never submit a PR unless they either do so or say they will when opening an issue. In any case, a week is more than a chance.

    @miguendes
    Copy link
    Mannequin

    miguendes mannequin commented May 3, 2021

    Thanks terry.reedy, actually I read it in the mailing list, someones comment not a guideline.

    Do you mind having a look at the PR?

    @serhiy-storchaka
    Copy link
    Member

    Functions registered with TestCase.addClassCleanup are called because TestCase has default implementation of tearDownClass. But you can suppress it by setting tearDownClass = None. So there is a similar issue with class cleanup functions.

    @serhiy-storchaka serhiy-storchaka added 3.11 bug and security fixes and removed 3.8 only security fixes labels Aug 22, 2021
    @serhiy-storchaka
    Copy link
    Member

    There are several other bugs in the code for cleaning up classes and modules:

    • Functions registered with addClassCleanup() are not called if tearDownClass is set to None or its look up raises AttributeError.
    • Buffering in TestResult blows up when functions registered with addClassCleanup() and addModuleCleanup() produce raise exceptions.
    • Buffering in TestResult does not buffer output of functions registered with addClassCleanup() and addModuleCleanup().
    • TestSuite.debug() blows up if functions registered with addClassCleanup() and addModuleCleanup() raise exceptions.
    • Errors in setUpModule() and function registered with addModuleCleanup() are reported in wrong order.

    And several lesser bugs.

    @ambv
    Copy link
    Contributor

    ambv commented Aug 30, 2021

    New changeset 08d9e59 by Serhiy Storchaka in branch 'main':
    bpo-43913: Fix bugs in cleaning up classes and modules in unittest. (GH-28006)
    08d9e59

    @ambv
    Copy link
    Contributor

    ambv commented Aug 30, 2021

    New changeset 9827710 by Serhiy Storchaka in branch '3.9':
    [3.9] bpo-43913: Fix bugs in cleaning up classes and modules in unittest. (GH-28006) (GH-28071)
    9827710

    @miss-islington
    Copy link
    Contributor

    New changeset d65fad0 by Miss Islington (bot) in branch '3.10':
    bpo-43913: Fix bugs in cleaning up classes and modules in unittest. (GH-28006)
    d65fad0

    @ambv
    Copy link
    Contributor

    ambv commented Aug 30, 2021

    Thanks for the fix, Serhiy, and Ryan for reporting the problem! ✨ 🍰 ✨

    @ambv ambv closed this as completed Aug 30, 2021
    @ambv ambv closed this as completed Aug 30, 2021
    @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
    3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants