Tests are not showing as "Skipped" when a unittest.SkipTest error is raised inside setUpClass()

dougthor42 commented 7 years ago

Summary

It appears that green is not handling raised unittest.SkipTest exceptions in setUpClass() as expected. It's silently skipping the entire class instead of verbosely skipping them.

Steps to Reproduce

Make this file:

# green_skiptest_error.py
import unittest

class TestMe(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        raise unittest.SkipTest("Intentionally throwing SkipTest exception")
        pass

    def test_me(self):
        self.assertTrue(True)

    def test_me_too(self):
        self.assertTrue(True)

if __name__ == "__main__":
    unittest.main(verbosity=2)

and then run:

python -m green green_skiptest_error.py -vvv

Actual

(.venv-tph-orm) C:\gitlab>python -m green green_skiptest_error.py -vvv
Green 2.8.1, Coverage 4.3.4, Python 3.5.2

Ran 0 tests in 0.427s

No Tests Found

Expected

(.venv-tph-orm) C:\gitlab>python -m green green_skiptest_error.py -vvv
Green 2.8.1, Coverage 4.3.4, Python 3.5.2

green_skiptest_error
  TestMe
s   test_me -- Intentionally throwing SkipTest exception
s   test_me_too -- Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me - Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me_too - Intentionally throwing SkipTest exception

Ran 2 tests in 0.489s

OK (skips=2)

For comparison, here's the output of `nose2 green_skiptest_error -vvv` and `python green_skiptest_error.py`:

skipped Intentionally throwing SkipTest exception

----------------------------------------------------------------------
Ran 0 tests in 0.001s

OK (skipped=1)

Note: nose2 and unittest have the same output because nose2 is simply a test runner and it doesn't modify any output like green.

Versions

Listed in all the code snippets, but here they are anyway:

Python 3.5.2 (64-bit) running on WinPython 3.5.2.2
Windows 10 Pro AU, 64-bit (v1607, build 14393.1066)
green 2.8.1
nose2 0.6.5

Investigation

If `SkipTest` is raised inside a test method rather than the `setUpClass` method, then things work as expected:

import unittest

class TestMe(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        pass

    def test_me(self):
        raise unittest.SkipTest("Intentionally throwing SkipTest exception")
        self.assertTrue(True)

    def test_me_too(self):
        self.assertTrue(True)

(.venv-tph-orm) C:\gitlab>python -m green green_skiptest_error.py -vvv
Green 2.8.1, Coverage 4.3.4, Python 3.5.2

green_skiptest_error
  TestMe
s   test_me -- Intentionally throwing SkipTest exception
.   test_me_too

Skipped green_skiptest_error.TestMe.test_me - Intentionally throwing SkipTest exception

Ran 2 tests in 0.484s

OK (passes=1, skips=1)

Throwing `SkipTest` inside `setUp` works correctly

import unittest

class TestMe(unittest.TestCase):
    def setUp(self):
        raise unittest.SkipTest("Intentionally throwing SkipTest exception")
        pass

    def test_me(self):
        self.assertTrue(True)

    def test_me_too(self):
        self.assertTrue(True)

(.venv-tph-orm) C:\gitlab>python -m green green_skiptest_error.py -vvv
Green 2.8.1, Coverage 4.3.4, Python 3.5.2

green_skiptest_error
  TestMe
s   test_me -- Intentionally throwing SkipTest exception
s   test_me_too -- Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me - Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me_too - Intentionally throwing SkipTest exception

Ran 2 tests in 0.491s

OK (skips=2)

Comments

When an entire class is skipped, the skip message should be listed on the class output and the test output. Example (colors are simply listed in square brackets at the end of the line):

(.venv-tph-orm) C:\gitlab>python -m green green_skiptest_error.py -vvv
Green 2.8.1, Coverage 4.3.4, Python 3.5.2

green_skiptest_error
  TestMe -- [Skipped] Intentionally throwing SkipTest exception     [cyan/blue]
s   test_me -- Intentionally throwing SkipTest exception            [cyan/blue]
s   test_me_too -- Intentionally throwing SkipTest exception        [cyan/blue]

Skipped green_skiptest_error.TestMe.test_me - Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me_too - Intentionally throwing SkipTest exception

Ran 2 tests in 0.491s

OK (skips=2)

(However, this probably should be a separate issue/feature request)

CleanCut commented 7 years ago

Wow! Thanks for the thorough write-up.

This was a tricky issue. It all boiled down to Python treating a SkipTest exception raised during setUpClass as a class setup failure. Which it is not. In some versions of Python that meant this got reported as a mysterious setup error without indicating which module/class/test caused it. Other versions of Python caught it later and marked it as a single skip of the class.

To fix this I chose to override the exception-handling of the code that runs setUpClass and actually handle the special case of SkipTest being raised, as the unittest module itself ought to have done in the first place. This results in each test independently being reported as being skipped, which I think is the most sane behavior. Edit: More to the point, this makes it so that raising SkipTest is handled the same way as if the class were decorated. The fact that the unittest module wasn't handling it in the same way was the cause of the bug in the first place.

$ ./g -vvv green_skiptest_error
Green 2.8.2, Coverage 4.3.4, Python 2.7.13

green_skiptest_error
  TestMe
s   test_me -- Intentionally throwing SkipTest exception
s   test_me_too -- Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me - Intentionally throwing SkipTest exception

Skipped green_skiptest_error.TestMe.test_me_too - Intentionally throwing SkipTest exception

Ran 2 tests in 0.112s

OK (skips=2)

CleanCut commented 7 years ago

This fix is included in version 2.8.2 (just released).

dougthor42 commented 7 years ago

Yup, that looks to have cleared things up!

Thanks a bunch.

threexc commented 4 years ago

Hello, I've encountered a similar issue on the Yocto Project with a particular QA test. It doesn't look like unittest has been tweaked to incorporate fixes similar to yours. Do you have any plans to submit a fix for this upstream for unittest?

CleanCut commented 4 years ago

@threexc The thought hadn't occurred to me!

I created https://github.com/CleanCut/green/issues/215 to track contributing the fix to upstream, but I have no idea when I'll be able to get to it. I included a link to the actual few lines that are the fix, so if you or anyone else would like to have the fun of contributing it upstream, please go ahead! 😄

CleanCut / green