[Patches] [ python-Patches-1034053 ] unittest.py patch: add skipped test functionality

Thu Sep 30 16:20:30 CEST 2004

Patches item #1034053, was opened at 2004-09-24 16:08
Message generated for change (Comment added) made by purcell
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1034053&group_id=5470

Category: Library (Lib)
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Remy Blank (remyblank)
Assigned to: Steve Purcell (purcell)
Summary: unittest.py patch: add skipped test functionality

Initial Comment:
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)

----------------------------------------------------------------------

>Comment By: Steve Purcell (purcell)
Date: 2004-09-30 16:20

Message:
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. 

----------------------------------------------------------------------

Comment By: Remy Blank (remyblank)
Date: 2004-09-30 15:46

Message:
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!

----------------------------------------------------------------------

Comment By: Steve Purcell (purcell)
Date: 2004-09-30 13:31

Message:
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. 

----------------------------------------------------------------------

Comment By: Remy Blank (remyblank)
Date: 2004-09-30 11:37

Message:
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.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-09-30 10:04

Message:
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.

----------------------------------------------------------------------

Comment By: Nick Coghlan (ncoghlan)
Date: 2004-09-29 08:36

Message:
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.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-09-26 12:13

Message:
Logged In: YES 
user_id=80475

The skipIf() method is sufficient.  From there, it is
trivial to roll your own resource check.

----------------------------------------------------------------------

Comment By: Wummel (calvin)
Date: 2004-09-26 11:10

Message:
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?

----------------------------------------------------------------------

Comment By: Remy Blank (remyblank)
Date: 2004-09-25 12:54

Message:
Logged In: YES 
user_id=568100

Added sample code.

----------------------------------------------------------------------

Comment By: Remy Blank (remyblank)
Date: 2004-09-25 12:53

Message:
Logged In: YES 
user_id=568100

The test suite for the added functionality.

----------------------------------------------------------------------

Comment By: Remy Blank (remyblank)
Date: 2004-09-25 12:51

Message:
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.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-09-25 10:18

Message:
Logged In: YES 
user_id=80475

Will this muck up some the existing test runners that people
have written?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1034053&group_id=5470