Navigation Menu

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

Add resource management/guarding to unittest #62726

Closed
zware opened this issue Jul 22, 2013 · 6 comments
Closed

Add resource management/guarding to unittest #62726

zware opened this issue Jul 22, 2013 · 6 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@zware
Copy link
Member

zware commented Jul 22, 2013

BPO 18526
Nosy @terryjreedy, @pitrou, @rbtcollins, @ezio-melotti, @bitdancer, @voidspace, @zware, @serhiy-storchaka
Files
  • unittest_resources.diff: Add resource management to unittest, version 1
  • 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 2013-09-16.14:49:22.673>
    created_at = <Date 2013-07-22.05:21:30.426>
    labels = ['type-feature', 'library']
    title = 'Add resource management/guarding to unittest'
    updated_at = <Date 2013-09-16.14:49:22.671>
    user = 'https://github.com/zware'

    bugs.python.org fields:

    activity = <Date 2013-09-16.14:49:22.671>
    actor = 'zach.ware'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-09-16.14:49:22.673>
    closer = 'zach.ware'
    components = ['Library (Lib)']
    creation = <Date 2013-07-22.05:21:30.426>
    creator = 'zach.ware'
    dependencies = []
    files = ['31001']
    hgrepos = []
    issue_num = 18526
    keywords = ['patch']
    message_count = 6.0
    messages = ['193499', '193588', '193589', '193593', '194638', '197905']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'pitrou', 'rbcollins', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'zach.ware', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18526'
    versions = ['Python 3.4']

    @zware
    Copy link
    Member Author

    zware commented Jul 22, 2013

    Here is an initial attempt at adding resource management/guarding to unittest, a la test.support.requires.

    The patch adds 6 new names to unittest.__all__; one function called 'registerResource' which explains itself, one function and one decorator (require and requires, respectively--I'm open to better, more differentiable names) for requiring a resource, and three exceptions (ResourceError for an error and ResourceUnavailable and ResourceDenied for skipping for different reasons). All of these are implemented in a new unittest.resources module.

    Existing modules are changed as follows:

    • util.py defines class _BaseSkip(Exception), which ResourceUnavailable inherits from. ResourceDenied subclasses ResourceUnavailable.
    • case.py: SkipTest inherits from _BaseSkip, and _BaseSkip is checked for to mark a test as skipped in _Outcome.testPartExecutor. Also, TestCase.run is patched to do resource checking for test classes and methods that use the requires decorator.
    • loader.py checks for util._BaseSkip instead of case.SkipTest
    • main.py adds -u and --use command line options, and use= TestProgram constructor arg.

    Tests are added to test_program and a new test_resources. The doc page is updated appropriately.

    The idea behind the departures from the way test.support.requires works is that since this will be used by far more than just the stdlib tests, there will be more than just the resources we use that unittest will need to know about, so there has to be some way to tell it about them--hence 'registerResource'. Also, to alleviate issues like bpo-18441, I have implemented an easy way to test whether the resource is usable, via the 'checker' argument to registerResource. Lastly, 'requires' is provided as a decorator for ease of use, though it does add the most complication to the implementation.

    I look forward to reviews :)

    Thanks,

    Zach

    @zware zware added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jul 22, 2013
    @rbtcollins
    Copy link
    Member

    I'll do a full review shortly, but at the conceptual level, I don't see any benefit in making a new SkipTest base class, other than instantly breaking other runners when an unknown exception is raised. SkipTest is part of the API. Making a new exception part of the API requires a strong reason - not just 'we want to show it differently' but 'we want the runner to behave differently.

    @voidspace
    Copy link
    Contributor

    My question is, is this generally useful enough for test suites beyond the Python standard library. Essentially it provides a command line interface to specialized test skipping.

    I *still* feel that a generalized mechanism for adding command line options to the standard test runner would allow for this and a myriad of other slightly specialized use cases. It's hard to see how to do that cleanly without full blown extension machinery (which I'm in favour of and have prototyped but it is a *much* bigger change set).

    @pitrou
    Copy link
    Member

    pitrou commented Jul 23, 2013

    Does registerResource() mutate some kind of global per-process state? This doesn't sound like a good idea.
    (and even less so the "default resources", IMO: these are stdlib test suite-specific)

    @zware
    Copy link
    Member Author

    zware commented Aug 8, 2013

    (Apologies for the delay in replies, my available time evaporated without notice...)

    Antoine Pitrou wrote:

    Does registerResource() mutate some kind of global per-process state? This
    doesn't sound like a good idea.

    It's not the greatest of ideas, no; but I hadn't come up with a better one
    yet :). Robert's review has given me a couple ideas to try to avoid this,
    though.

    (and even less so the "default resources", IMO: these are stdlib test
    suite-specific)

    That is true, though I attempted to pick out some of the ones used by our test
    suite that are most likely to be applicable elsewhere. I had a couple misses,
    though, and I'm not against removing the defaults entirely.

    ---

    Michael Foord wrote:

    My question is, is this generally useful enough for test suites beyond the
    Python standard library. Essentially it provides a command line interface to
    specialized test skipping.

    To be honest, I'm not sure. Theoretically, I can think of several situations
    where it would be useful, but I have no concrete examples aside from the Pytohn
    test suite.

    I *still* feel that a generalized mechanism for adding command line options to
    the standard test runner would allow for this and a myriad of other slightly
    specialized use cases. It's hard to see how to do that cleanly without full
    blown extension machinery (which I'm in favour of and have prototyped but it
    is a *much* bigger change set)

    Is your prototype available anywhere for me to get a better idea what you mean?
    That could indeed be a much better approach if it is what I think it is.

    ---

    Robert Collins wrote:

    I'll do a full review shortly, but at the conceptual level, I don't see any
    benefit in making a new SkipTest base class, other than instantly breaking
    other runners when an unknown exception is raised. SkipTest is part of the
    API. Making a new exception part of the API requires a strong reason - not
    just 'we want to show it differently' but 'we want the runner to behave
    differently.

    Actually, the real reason I created the new _BaseSkip exception was to avoid an
    import loop between resources.py and case.py. I think I was trying to keep
    SkipTest defined in case.py, but in retrospect it is much saner to just move
    it to util.py and import from there in both other modules, and directly subclass
    SkipTest for the new Resource exceptions. I'll respond to your review on
    Rietveld and with a new patch, as time allows (hopefully before the next alpha).

    ---

    Thanks to all three of you for your comments :)

    @zware
    Copy link
    Member Author

    zware commented Sep 16, 2013

    Having thought about this more, I think I agree that this is the wrong approach to the issue and that a more general ability to add command line options to unittest would be better. Closing this issue.

    @zware zware closed this as completed Sep 16, 2013
    @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-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants