classification
Title: Add resource management/guarding to unittest
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, michael.foord, pitrou, r.david.murray, rbcollins, serhiy.storchaka, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2013-07-22 05:21 by zach.ware, last changed 2013-09-16 14:49 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
unittest_resources.diff zach.ware, 2013-07-22 05:21 Add resource management to unittest, version 1 review
Messages (6)
msg193499 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-07-22 05:21
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 #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
msg193588 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2013-07-23 08:26
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.
msg193589 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-07-23 08:37
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).
msg193593 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-23 09:40
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)
msg194638 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-08-08 02:14
(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 :)
msg197905 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-09-16 14:49
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.
History
Date User Action Args
2013-09-16 14:49:22zach.waresetstatus: open -> closed
resolution: rejected
messages: + msg197905
2013-08-08 02:14:36zach.waresetmessages: + msg194638
2013-07-23 09:40:56pitrousetmessages: + msg193593
2013-07-23 08:37:52michael.foordsetmessages: + msg193589
2013-07-23 08:26:31rbcollinssetmessages: + msg193588
2013-07-23 08:23:16rbcollinssetnosy: + rbcollins
2013-07-22 05:21:30zach.warecreate