classification
Title: unittest.py patch: add skipped test functionality
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.1, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: benjamin.peterson Nosy List: benjamin.peterson, calvin, georg.brandl, giampaolo.rodola, ncoghlan, pitrou, pupeno, purcell, rblank, rhettinger
Priority: normal Keywords: patch

Created on 2004-09-24 14:08 by rblank, last changed 2009-03-23 21:51 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
unittest_skip.patch rblank, 2004-09-24 14:08 Patch against unittest.py from Python 2.3.3
testSkippedTest.py rblank, 2004-09-25 10:53 Test suite for test-skipping functionality
SkippedTestDemo.py rblank, 2004-09-25 10:54 Sample code for test-skipping functionality
skip.diff pupeno, 2008-08-11 13:14
unittest_galore.patch benjamin.peterson, 2009-03-23 00:57
Messages (27)
msg46926 - (view) Author: Remy Blank (rblank) Date: 2004-09-24 14:08
I added the possibility for tests using the unittest.py 
framework to be skipped. Basically, I added two methods 
to TestCase:

  TestCase.skip(msg): skips test unconditionally
  TestCase.skipIf(expr, msg): skips test if expr is true

These can be called either in setUp() or in the test 
methods. I also added reporting of skipped tests to 
TestResult, _TextTestResult and TextTestRunner. If no 
tests are skipped, everything should be the same as 
before.

I am using Python 2.3.3, so the changes are against the 
file in that version. I can generate a patch for a more 
recent version if desired. I attached the patch against 
the original (unittest_skip.patch). I can provide a 
complete test suite for the new functionality and a usage 
example program.


Quick usage example:

class ReadShadowTest(unittest.TestCase):
        """Read access to /etc/shadow"""
        def testReadingAsRoot(self):
                """Reading /etc/shadow as root"""
                self.skipIf(os.geteuid() != 0, "Must be root")
                open("/etc/shadow").close()


The example program produces the following output:

$ ./SkippedTestDemo.py -v
Access to autoexec.bat ... SKIPPED (Only available on 
Windows)
Access to config.sys ... SKIPPED (Only available on 
Windows)
Reading /etc/shadow as root ... SKIPPED (Must be root)
Reading /etc/shadow as non-root ... ok

-------------------------------------------------------
---------------
Ran 4 tests in 0.004s

OK (skipped=3)
msg46927 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-09-25 08:18
Logged In: YES 
user_id=80475

Will this muck up some the existing test runners that people
have written?
msg46928 - (view) Author: Remy Blank (rblank) Date: 2004-09-25 10:51
Logged In: YES 
user_id=568100

I don't think so. Basically, the patch changes the following:
 - Adds a class SkippedException to the unittest module
 - Adds a skipped attribute to TestResult, containing the list of skipped 
tests, and an addSkipped() method to add to the list.
 - Catches the SkippedException in TestCase.__call__()
 - Adds skip() and skipIf() to TestCase
 - Modifies _TextTestResult and TextTestRunner to report skipped tests 
*only if there are any*

I see two potential problems:
 - Test runners based on (or using the output of) TextTestRunner. I've 
taken care that the output is unchanged if there are no skipped tests.
 - Code that uses repr() of a TestResult, as I extended it to always report 
skipped tests. I think this would be bad practice anyway.

However, to use the test-skipping functionality, custom test runners will 
obviously need to be extended to report skipped tests.

OTOH, I don't have a big test codebase to check. I read that e.g. Zope is 
using unittest. Maybe I can try to run their test suite with the patched 
unittest.py. I'll check.
msg46929 - (view) Author: Remy Blank (rblank) Date: 2004-09-25 10:53
Logged In: YES 
user_id=568100

The test suite for the added functionality.
msg46930 - (view) Author: Remy Blank (rblank) Date: 2004-09-25 10:54
Logged In: YES 
user_id=568100

Added sample code.
msg46931 - (view) Author: Bastian Kleineidam (calvin) Date: 2004-09-26 09:10
Logged In: YES 
user_id=9205

This is a nice patch. I am wondering if it can be extended
to support the resource idea used in the python regression
tests. That is the user has a --resource option to give a
list of available resources and a test only runs if its
requested resources are available. Otherwise it will be skipped.
Example:
TestDemo.py --resource=network --resource=audio
... would supply the network and audio resource.
Or does it make more sense to put this in a separate patch?
msg46932 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-09-26 10:13
Logged In: YES 
user_id=80475

The skipIf() method is sufficient.  From there, it is
trivial to roll your own resource check.
msg46933 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2004-09-29 06:36
Logged In: YES 
user_id=1038590

I'd certainly find such a feature handy - when testing
different variations of an embedded firmware image, it would
make it much easier to enable/disable different tests based
on the capabilities of the firmware.

Ditto for the original example of cross-platform testing.
msg46934 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-09-30 08:04
Logged In: YES 
user_id=80475

After more thought, I think decorators offer a cleaner, more
complete solution without further complicating the unittest
module.

def rootonly(f):
    "Decorator to skip tests that require root access"
    if os.geteuid() == 0:
        return f
    return lambda self: 0


@rootonly
def testReadingAsRoot(self):
   . . .


Note the rootonly() decorator need only be defined once
instead of writing a full self.skipIf(condition) inside
every test.  Also, it appears prior to the definition rather
than inside.  The approach is more flexible than the
original proposal though it does lack a reporting mechanism.
msg46935 - (view) Author: Remy Blank (rblank) Date: 2004-09-30 09:37
Logged In: YES 
user_id=568100

I strongly disagree. Skipped tests should not just be
transformed into passed tests, but must be recorded as
skipped and reported to the user. Knowing that a test
skipped is important information.

The Python regression tests (although I'm not familiar with
them) provide the same "skip" functionality, and I don't
think people would be happy to replace it with just "pass".

The decorator approach is an interesting idea, though, and
could be combined with skipIf() so as to provide the other
advantages you mention, namely single definition and
appearance prior to definition. Something along the following:

def rootOnly(f):
        """Decorator to skip tests that require root access"""
        def wrapper(self):
                self.skipIf(os.getuid() != 0, "Must be root")
                self.f()
        wrapper.__doc__ = f.__doc__
        return wrapper


class ReadShadowTest(unittest.TestCase):
        """Read access to /etc/shadow"""
        @rootOnly
        def testReadingAsRoot(self):
                """Reading /etc/shadow as root"""
                open("/etc/shadow").close()

Note that I'm not yet familiar with decorators, so the
wrapper() function might not be the correct way to do this.
msg46936 - (view) Author: Steve Purcell (purcell) Date: 2004-09-30 11:31
Logged In: YES 
user_id=21477

I've been really tied up; sorry for the delayed response, but I've been 
reading all the comments on this patch. 
 
Overall, I'm leaning in favour of accepting this patch, probably with 
some minor changes to the way skipped tests are reported. 
 
The concept of skipping is one that has been kept out of JUnit, but is 
found in NUnit and is well regarded there. In my XP coaching work 
ThoughtWorks I see an obscenely large number of JUnit tests, and a 
common mistake is to comment out test method bodies, leading to 
"false passes". Explicit support for skipping in unittest would mitigate 
this. 
 
I agree with Remy that the decorator example, though ingenious, has 
the wrong result; skipped tests will be reported as successes. In order 
for a test method to decide if it should be skipped, it will often need 
information from 'self' that was gathered during setUp() -- this makes 
decorators cumbersome for this. Also, a decorator solution would not 
allow test methods to skip if the setUp() method itself decides to skip(). 
 
Please give me a few more days on this, and I'll work on integrating 
and tweaking the patch. 
msg46937 - (view) Author: Remy Blank (rblank) Date: 2004-09-30 13:46
Logged In: YES 
user_id=568100

Speaking of decorators, the NUnit example is quite
instructive, re. their use of attributes to mark classes as
test cases, methods as test methods, grouping tests by
category, and for that matter ignoring tests temporarily. I
expect all of this can be done with decorators: @testMethod
to mark individual tests, @category("LongRunning"), @ignore,
@explicit, ...

And if I'm not mistaken all of this can be added without
breaking backward compatibility.

Interesting times lay ahead!
msg46938 - (view) Author: Steve Purcell (purcell) Date: 2004-09-30 14:20
Logged In: YES 
user_id=21477

Yes, that's right, and I would consider providing a number of such 
decorators at a later date. I've just spent a little time chatting to my 
colleage Joe Walnes (of nMock fame) about all this; he's more of an 
nUnit authority than I am. 
 
Categories are particularly interesting. In theory, it would be possible 
to get the same effect using TestSuites, but in practice tool support 
(including unittest.main()) discourages the use of TestSuites in favour 
of magic discovery of test cases; categories would be a better way of 
allowing tools to dynamically construct suites. @ignore could be 
considered equivalent to @category("ignored") in a certain sense. 
 
Skipping is not quite the same as ignoring, since it's determined at 
run-time, and so I think it is appropriate to add methods to explicitly 
support it. 
 
Interesting times indeed. 
msg46939 - (view) Author: Remy Blank (rblank) Date: 2004-10-04 20:04
Logged In: YES 
user_id=568100

I have just run the unit tests from Zope 2.7.2 (2358 tests) on 
Python 2.3.3, first with the original unittest.py, then with the 
patch. There was no difference in output, except for the total 
duration.

That may give a hint about backward compatibility.
msg46940 - (view) Author: Remy Blank (rblank) Date: 2004-11-24 11:27
Logged In: YES 
user_id=568100

Anything new about skipping tests? I'm still very interested.
msg61284 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-01-20 12:54
So what's the status of this? Skipping test is still an important
ability to have.
msg61492 - (view) Author: Steve Purcell (purcell) Date: 2008-01-22 10:12
The status of this ticket is unchanged.  I'm somewhat removed from the 
Python scene in recent times, and I'm not in a position to apply this 
patch or a variation of it.

I still believe this would be a beneficial change to the unittest 
module, though, and perhaps someone else would be willing to apply 
Remy's patch (updated if necessary to apply cleanly).  There's a minor 
typo in there ("tuble" instead of "tuple"), but otherwise it looks 
acceptable to me.
msg61517 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-01-22 18:09
I'll take it.
msg71014 - (view) Author: J. Pablo Fernández (pupeno) Date: 2008-08-11 13:14
Hello,

The attached patch adds the skip functionality and tests to a very
recent (yesterday) Python 3. It took me a few hours to apply the patch,
change to Python 3 style, merge the tests into the current set of tests
not doing a mess.

I think the whole thing is missing documentation, which I would gladly
write if the code is good to go. Just let me know.

Thanks.
msg71015 - (view) Author: J. Pablo Fernández (pupeno) Date: 2008-08-11 13:17
Oh, I forgot to upgrade versions to include Python 3.0 and to mention
that I have to fix some other tests to work with the new TestCase. It's
nothing serious, just doctests that where too dependent on the output of
the TextTestRunner. As previously discussed, doing that is not a good
idea anyhow, but it is needed for these doctests and I wouldn't expect
anybody else out there in the while to encounter this problem.
msg78409 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-12-28 15:30
Pupeno's patch looks good to me. Additional candy would be a decorator
to flag skipped tests (e.g. @skipped_test or @skipped_test("A
message")), but we can do that later.
msg78424 - (view) Author: Remy Blank (rblank) Date: 2008-12-28 18:43
There's still a typo in the docstring of TestResult.addSkipped() (tuble
-> tuple).
msg78467 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-12-29 17:39
I think this is a good improvement, and I hope it can make it into 2.7/3.1.

Several comments on patch:

- I don't like the name "SkipException" SkipTest is better IMO.

- TestResult.addSkipped should be changed to TestResult.addSkip.

- I'm not sure why TestResult.addSkipped gets sys.exc_info() pass in. I
think you should just catch the exception and pass the reason ("str(e)")
to addSkipped.

- The patch needs docs before it can be applied.

- As Antoine said, it would be nice to have decorators for skipping.
When I implemented this, I added the skip() (unconditional skip)
skip_if(condition, reason) and skip_unless(condition, reason)
decorators. It should also be easy to extend the mechanism, so that
custom decorators can be written.

- It would nice to be able to skip whole classes, too. This could easily
be done with class decorators.

(Georg, I hope you don't mind if I "steal" this from you.)
msg83975 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-03-22 17:20
Ok, here's my unittest skipping patch. It supports skipping classes and
expected failures and includes skipping decorators.

I had to employ a little evil to make test skipping work for classes.
The made a new TestSuite class called ClassTestSuite, to contain the
tests for one class. It pretends to be a TestCase enough that in can be
completely skipped.
msg83977 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-03-22 17:25
Here's the patch on Rietveld: http://codereview.appspot.com/27095
msg83992 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-03-23 00:58
I've attached a new patch which takes into account Antoine's review.
msg84042 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-03-23 21:51
Committed my patch in r70555.
History
Date User Action Args
2009-03-23 21:51:07benjamin.petersonsetstatus: open -> closed
resolution: accepted
messages: + msg84042
2009-03-23 00:58:56benjamin.petersonsetmessages: + msg83992
2009-03-23 00:58:01benjamin.petersonsetfiles: + unittest_galore.patch
2009-03-23 00:57:30benjamin.petersonsetfiles: - unittest_galore.patch
2009-03-22 17:25:23benjamin.petersonsetmessages: + msg83977
2009-03-22 17:20:52benjamin.petersonsetfiles: + unittest_galore.patch

messages: + msg83975
2008-12-29 17:39:33benjamin.petersonsetassignee: georg.brandl -> benjamin.peterson
messages: + msg78467
nosy: + benjamin.peterson
2008-12-28 18:43:04rblanksetmessages: + msg78424
2008-12-28 15:30:56pitrousettype: enhancement
stage: patch review
messages: + msg78409
versions: + Python 3.1, Python 2.7, - Python 2.4, Python 3.0
2008-08-11 13:17:10pupenosetmessages: + msg71015
versions: + Python 3.0
2008-08-11 13:15:00pupenosetfiles: + skip.diff
nosy: + pupeno
messages: + msg71014
2008-01-22 23:30:27giampaolo.rodolasetnosy: + giampaolo.rodola
2008-01-22 18:09:22georg.brandlsetassignee: purcell -> georg.brandl
messages: + msg61517
nosy: + georg.brandl
2008-01-22 10:12:34purcellsetmessages: + msg61492
2008-01-20 12:54:59pitrousetnosy: + pitrou
messages: + msg61284
2004-09-24 14:08:18rblankcreate