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 runner needs an option to call gc.collect() after each test #62108

Open
gvanrossum opened this issue May 5, 2013 · 20 comments
Open
Labels
3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@gvanrossum
Copy link
Member

BPO 17908
Nosy @gvanrossum, @pitrou, @vstinner, @giampaolo, @rbtcollins, @ezio-melotti, @voidspace, @asvetlov, @serhiy-storchaka, @phmc
Files
  • unittest-add-gc.patch: Patch that adds command line option for garbage collection
  • 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 = None
    created_at = <Date 2013-05-05.02:20:44.755>
    labels = ['type-feature', 'library', '3.9']
    title = 'Unittest runner needs an option to call gc.collect() after each test'
    updated_at = <Date 2019-07-29.11:47:10.140>
    user = 'https://github.com/gvanrossum'

    bugs.python.org fields:

    activity = <Date 2019-07-29.11:47:10.140>
    actor = 'vstinner'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2013-05-05.02:20:44.755>
    creator = 'gvanrossum'
    dependencies = []
    files = ['40711']
    hgrepos = []
    issue_num = 17908
    keywords = ['patch']
    message_count = 20.0
    messages = ['188420', '188422', '188436', '188451', '188744', '188745', '188793', '188818', '242165', '242166', '242168', '242190', '242191', '242202', '242220', '249033', '252957', '252958', '252962', '348635']
    nosy_count = 11.0
    nosy_names = ['gvanrossum', 'pitrou', 'vstinner', 'giampaolo.rodola', 'rbcollins', 'ezio.melotti', 'michael.foord', 'asvetlov', 'serhiy.storchaka', 'pconnell', 'azsorkin']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17908'
    versions = ['Python 3.9']

    @gvanrossum
    Copy link
    Member Author

    It would be nice if there was a command-line option to tell the test runner to call gc.collect() after each test.

    See https://groups.google.com/d/msg/python-tulip/tstjvOQowIU/IRihc8NCHZUJ -- Glyph points out that always doing this is problematic because it slows down tests too much; OTOH it's a useful option to have in case you're tracking down something that happens (or doesn't happen) when an object is collected (e.g. in __del__).

    @gvanrossum gvanrossum added easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 5, 2013
    @ezio-melotti
    Copy link
    Member

    See also bpo-17534.

    @pitrou
    Copy link
    Member

    pitrou commented May 5, 2013

    OTOH it's a useful option to have in case you're tracking down
    something that happens (or doesn't happen) when an object is collected

    IMO this is a good reason to implement your specific tearDown method (or call addCleanup if you prefer), possibly in a shared base class.

    I don't think this is a good candidate for a command-line option, it's too specialized and it's also not something which should be enabled blindly. In other words, each test case should know whether it needs a collection or not.

    @gvanrossum
    Copy link
    Member Author

    On Sun, May 5, 2013 at 4:29 AM, Antoine Pitrou <report@bugs.python.org> wrote:

    Antoine Pitrou added the comment:

    > OTOH it's a useful option to have in case you're tracking down
    > something that happens (or doesn't happen) when an object is collected

    IMO this is a good reason to implement your specific tearDown method (or call addCleanup if you prefer), possibly in a shared base class.

    I don't think this is a good candidate for a command-line option, it's too specialized and it's also not something which should be enabled blindly. In other words, each test case should know whether it needs a collection or not.

    This is not for tests that know or expect they need a call to
    gc.collect(). This is for the case where you have 500 tests that
    weren't written with gc.collect() in mind, and suddenly you have a
    nondeterministic failure because something goes wrong during
    collection. The cause is probably many tests earlier -- and if you
    could just call gc.collect() in each tearDown() it would be a cinch to
    pinpoint the test that causes this. But (unless you had all that
    foresight) that's a massive undertaking. However turning on the
    command line option makes it trivial.

    It's rare that extra collections cause tests to fail (and if it does
    that's a flakey test anyway) so just turning this on for all tests
    shouldn't affect correct tests -- however it can slow down your test
    suite 3x or more so you don't want this to be unittest's default
    behavior. Hence the suggestion of a command line flag.

    @voidspace
    Copy link
    Contributor

    I'm afraid I'm inclined to agree with Antoine. This seems a relatively specialised problem and I'd expect to see it solved in the test system rather than in unittest itself.

    One solution, and something I'd like to see, is a way for test systems to extend the unittest command line handling so that they can add extra options. At the moment it's very painful to do.

    @gvanrossum
    Copy link
    Member Author

    TBH I would *love* for there to be a standardized way to extend the command line options! If it existed I would help myself. As it is, that's way to complicated. (Then again, most customizations to the default test runner are too complicated IMO. :-)

    @voidspace
    Copy link
    Contributor

    The default test runner is quite horrible.

    @pitrou
    Copy link
    Member

    pitrou commented May 10, 2013

    TBH I would *love* for there to be a standardized way to extend the
    command line options! If it existed I would help myself. As it is,
    that's way to complicated. (Then again, most customizations to the
    default test runner are too complicated IMO. :-)

    +1. I found myself wanting to add a "-F" option à la regrtest, and I
    ended up processing sys.argv manually :-/

    @pitrou pitrou changed the title Unittest runner needs an option to call gc.collect() after each test Unittest runner needs an option to call gc.collect() after each test May 10, 2013
    @azsorkin
    Copy link
    Mannequin

    azsorkin mannequin commented Apr 28, 2015

    Is this enhancement still open? I've run into this problem previously, and would be more than happy to implement this feature.

    @gvanrossum
    Copy link
    Member Author

    Does the cpython test runner have this? Might be nice there. Not sure if
    the default test runner is something to extend at this point. Eveybody is
    using py.test anyway...

    On Monday, April 27, 2015, Ned Deily <report@bugs.python.org> wrote:

    Changes by Ned Deily <nad@acm.org <javascript:;>>:

    ----------
    nosy: +rbcollins


    Python tracker <report@bugs.python.org <javascript:;>>
    <http://bugs.python.org/issue17908\>


    @gvanrossum gvanrossum changed the title Unittest runner needs an option to call gc.collect() after each test Unittest runner needs an option to call gc.collect() after each test Apr 28, 2015
    @pitrou
    Copy link
    Member

    pitrou commented Apr 28, 2015

    I still think that it's something that people can trivially implement and that has no place in the standard unittest package.

    @gvanrossum
    Copy link
    Member Author

    It's trivial to add to a single test or even a single TestCase subclass, yes. The use case is more that you have a ton of tests and you suspect there's a problem due to GC. Being able to call gc.collect() after each test through the flip of a flag would be useful.

    But I agree it's iffy whether this belongs in the unittest package; we could add it to the CPython run_tests.py script, or you could request this as a feature from py.test.

    If someone wants to add this to run_tests.py I think this issue would be the place to review the patch.

    @serhiy-storchaka
    Copy link
    Member

    See also bpo-23839 where proposed not only call gc.collect(), but clear all caches in test.regrtest.

    @rbtcollins
    Copy link
    Member

    I'm going to disagree with michael and antoine here.

    The *internals* should be clean and pluggable for sure, but this is actually a pretty common thing to try, so there's no reason to force it to only be done by external plugins.

    Right now the way to plug this in has been complicated by the addition of module / class suites, which already perform extra work around individual tests, but in a non-introspectable / extensible fashion.

    So you could add this as a hook to the loader (decorate each test with some new thing) and a CLI option to use that hook for a gc collect call.

    Alternatively, we could face down the class/module stuff and rearrange it to be extensible (e.g. something along the lines of testresources internals - generic groups pre-post stuff) or via some interaction with TestResult.... but I really dislike using TestResult to control the run - I have a better layout mostly sketched in mind but haven't had time to formalise it.

    So I recommend the TestLoader hook point - its well within the current responsibilities for it to do this, and I don't see any long term maintenance issues.

    @pitrou
    Copy link
    Member

    pitrou commented Apr 29, 2015

    Le 28/04/2015 21:00, Robert Collins a écrit :

    So you could add this as a hook to the loader (decorate each test
    with some new thing) and a CLI option to use that hook for a gc
    collect call.

    Can I take this as meaning you're not opposed to adding other options to
    the unittest core? (say, something to run a test until failure, or to
    watch for reference leaks, or to run tests in multiple processes :-))

    @rbtcollins
    Copy link
    Member

    "say, something to run a test until failure, or to
    watch for reference leaks, or to run tests in multiple processes :-))"

    I think a few complimentary things.

    unittest extensability currently requires a new CLI entry point for each thing. I'd like to fix that.

    The actual plumbing is fairly extensible, though its not always obvious. I'd like to fix that too - without any global state getting involved.

    Of the things you mention, running a given command line until failure and checking for reference leaks are both straight forward, very common requests (as is the gc check) and I'd like to see those implemented as extensions shipped in the stdlib.

    Running in parallel becomes important when one is doing slow (e.g. functional) tests with unittest, and I think thats important to support. It is however much harder to do well: some of the current idioms that have snuck in (like the handling of stdout/stderr capturing without the buffer flag) are not well matched to the needs of reporting on concurrent tests to users. I'm not in any way opposed to a good implementation, but it would need to be good I think - there's not much point having a poor implementation, given the rich set of parallel test runners that are out there that already build on the unittest core (green, nose, testrepository just for starters). The only unique audience for stdlib test facilities is the stdlib itself and I think a better way to solve that is to enable the use of alternative runners for our own tests (moving them to be a little cleaner should enable that) - and then the point where it matters is really 'when buildbots would be enough faster to make a difference'.

    @azsorkin
    Copy link
    Mannequin

    azsorkin mannequin commented Oct 13, 2015

    Any comments about this proposed patch?

    @vstinner
    Copy link
    Member

    While I like the idea of adding an option to force a garbage collection after running each time, I don't like how unittest is growing :-( It looks more and more like regrtest and I hate regrtest (I reworked its code recently to create a less ugly "libregrtes").

    Once Antoine Pitrou proposed to add "plugins" to unittest. I agree that we need a gate between unittest and all other test frameworks: nose, py.test, testtools/testrepository/testr, etc.

    Seriously, don't you think that something is wrong with such API?

    def __init__(self, module='__main__', defaultTest=None, argv=None,
                 testRunner=None, testLoader=loader.defaultTestLoader,
                 exit=True, verbosity=1, failfast=None, catchbreak=None,
                 buffer=None, warnings=None, *, tb_locals=False,
                 gc_enabled=False):

    @vstinner
    Copy link
    Member

    Well, maybe we can add yet another parameter to all unittest classes
    (push this change), but IMHO someone must rework the unittest API to
    make it more extensible instead of adding more and more stuff in the
    base API.

    @vstinner
    Copy link
    Member

    This issue is no newcomer friendly, I remove the "easy" keyword.

    @vstinner vstinner added 3.9 only security fixes and removed easy labels Jul 29, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added 3.13 new features, bugs and security fixes and removed 3.9 only security fixes labels May 20, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    Status: No status
    Development

    No branches or pull requests

    8 participants