Troscoe 35978 exceptions in class setup teardown http reporter

[mrtyler]

Changes:

• Add EVENT_ON_COMPLETE_TEST_CASE.
• test_runner_clients use this phase to tell the test_runner_server that they are done. This replaces the old method, where the server disconnected the client as soon as the last method from the checked-out test case was returned. The immediate disconnect left no way to communicate class_teardown failures.
• class_teardown failures are correctly handled in client/server mode.
• Exceptions in class_setup_teardown methods are handled correctly and covered by test instead of causing Testify to crash.

One of this branch's many virtues is that it addresses issue #84.

[mrtyler]

This is now complete (...hopefully). Additional changes:

• Tests for the different failure_limits (there are two! one in TestCase and one in the client/server logic. Both are tested.)
• SqlReporter filters out the special 'run' test result. This fixes the "-5184 incomplete tests" phenomenon in bbmgr.

[baris]

Ship it! :)

[blampe]

Cool! Big fan of the test coverage. I'm not super familiar with this part of Testify so I don't have much to comment on. My one question would be what our behavior is like when class_setup_teardown fails (before and after the yield).

[mrtyler]

Re: better solutions than the misappropriation of TestResult: please add your ideas to issue #121.

[mrtyler]

Re: class_setup_teardown

Consider this test case:

class FakeClassSetupTeardownSetupExceptionTestCase(TestCase):
    @class_setup_teardown
    def class_setup_teardown_raises_exception_during_setup(self):
        raise FakeClassFixtureException
        yield # Never reached obv

    def test1(self):
        print "i am test1"
        pass

    def test2(self):
        print "i am test2"
        pass

Here is what current stable Testify 0.3.4 does:

$ testify -v yelp.tests.tmp_blowup.FakeClassSetupTeardownSetupExceptionTestCase
Traceback (most recent call last):
  File "/usr/bin/testify", line 25, in <module>
    test_program.TestProgram()
  File "/usr/lib/python2.6/dist-packages/testify/test_program.py", line 276, in __init__
    result = runner.run()
  File "/usr/lib/python2.6/dist-packages/testify/test_runner.py", line 157, in run
    runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/profile.py", line 24, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/code_coverage.py", line 25, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/seed.py", line 24, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/test_case.py", line 295, in run
    self.__enter_context_managers(self.class_setup_teardown_fixtures, self.__run_test_methods)
  File "/usr/lib/python2.6/dist-packages/testify/test_case.py", line 362, in __enter_context_managers
    with contextmanager(fixture_methods[0])():
  File "/usr/lib/python2.6/contextlib.py", line 16, in __enter__
    return self.gen.next()
  File "./yelp/tests/tmp_blowup.py", line 64, in class_setup_teardown_raises_exception_during_setup
    raise FakeClassFixtureException
yelp.tests.tmp_blowup.FakeClassFixtureException

Note that this isn't a report of an exception in a test; Testify fails to catch FakeClassFixtureException and dies!

Here is what my branch does:

$ PYTHONPATH=~/Testify/ ~/Testify/bin/testify -v yelp.tests.tmp_blowup.FakeClassSetupTeardownSetupExceptionTest
Case
yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test1 ... i am test1
ok in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test2 ... i am test2
ok in 0.00s

PASSED.  2 tests / 1 case: 2 passed, 0 failed.  (Total test time 0.00s)

So... that's a bug :(. I'll take a crack at that shortly.

Now consider this test case:

class FakeClassSetupTeardownTeardownExceptionTestCase(TestCase):
    @class_setup_teardown
    def class_setup_teardown_raises_exception_during_teardown(self):
        yield
        raise FakeClassFixtureException

    def test1(self):
        print "i am test1"
        pass

    def test2(self):
        print "i am test2"
        pass

With Testify 0.3.4:

$ testify -v yelp.tests.tmp_blowup.FakeClassSetupTeardownTeardownExceptionTestCase
yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.test1 ... i am test1
ok in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.test2 ... i am test2
ok in 0.00s
Traceback (most recent call last):
  File "/usr/bin/testify", line 25, in <module>
    test_program.TestProgram()
  File "/usr/lib/python2.6/dist-packages/testify/test_program.py", line 276, in __init__
    result = runner.run()
  File "/usr/lib/python2.6/dist-packages/testify/test_runner.py", line 157, in run
    runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/profile.py", line 24, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/code_coverage.py", line 25, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/plugins/seed.py", line 24, in run_test_case
    return runnable()
  File "/usr/lib/python2.6/dist-packages/testify/test_case.py", line 295, in run
    self.__enter_context_managers(self.class_setup_teardown_fixtures, self.__run_test_methods)
  File "/usr/lib/python2.6/dist-packages/testify/test_case.py", line 363, in __enter_context_managers
    self.__enter_context_managers(fixture_methods[1:], callback)
  File "/usr/lib/python2.6/contextlib.py", line 23, in __exit__
    self.gen.next()
  File "./yelp/tests/tmp_blowup.py", line 79, in class_setup_teardown_raises_exception_during_teardown
    raise FakeClassFixtureException
yelp.tests.tmp_blowup.FakeClassFixtureException

Again, Testify crashes.

With my branch:

troscoe@dev7:~/pg/loc (troscoe_class_teardown) $ PYTHONPATH=~/Testify/ ~/Testify/bin/testify -v yelp.tests.tmp_blowup.FakeClassSetupTeardownTeardownExceptionTestCase
yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.test1 ... i am test1
ok in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.test2 ... i am test2
ok in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.class_setup_teardown_raises_exception_during_teardown ... error: yelp.tests.tmp_blowup FakeClassSetupTeardownTeardownExceptionTestCase.class_setup_teardown_raises_exception_during_teardown
Traceback (most recent call last):
  File "/usr/lib/python2.6/contextlib.py", line 23, in __exit__
    self.gen.next()
  File "./yelp/tests/tmp_blowup.py", line 79, in class_setup_teardown_raises_exception_during_teardown
    raise FakeClassFixtureException
FakeClassFixtureException

ERROR in 0.00s

FAILED.  3 tests / 1 case: 2 passed, 1 failed.  (Total test time 0.00s)

This result is correct!

[bukzor]

I was discussing possible solutions for this issue with Ken, before we knew of this pull request. At first we considered implementing somethign like what you've done here, inserting a fake test to which we can attach the teardown failure, but making the total number of tests vary based on how things fail seems wrong. What Ken suggested, which seems better, is to say that the associated tests failed, since they failed to tear down. This means the number of tests doesn't vary, just the number of failures.

Is that harder to do than what you've done here? It seems like it would factor out the need for the special test method named "run".

In any case, @class_setup / @setup / @teardown / @class_teardown should all have the same behavior with regards to error reporting. I see a special-case for "class_teardown" which makes me concerned. What was your rationale there?

[mrtyler]

@bukzor (and Ken),

Testify reports results as they happen. By the time we discover that a class_teardown method raised an exception, the results of the test methods in that test case have already been reported. Going back and amending these results would require significant refactoring and would be impossible for something like the basic TextLogger (we already printed; there's no going back).

We could have Testify wait until an entire test case has run before reporting, but that would require significant re-design and I'm not sure it's desirable anyway.

Since the test methods from the test case have already been reported, the only thing left to do is report a new test result to communicate the class_teardown failure.

setup, teardown, and class_setup have the luxury of running before any test methods have run (or finished) and thus before they have been reported. These fixtures use the strategy you and Ken specify -- report the fixture error as the result of running each test method.

[mrtyler]

@blampe et al,

I added tests for the class_setup_teardown behavior and fixed some stuff. Compare and contrast:

class_setup:

$ PYTHONPATH=~/Testify/ ~/Testify/bin/testify -v yelp.tests.tmp_blowup FakeClassSetupTestCase
yelp.tests.tmp_blowup FakeClassSetupTestCase.test1 ... error: yelp.tests.tmp_blowup FakeClassSetupTestCase.test1
Traceback (most recent call last):
  File "./yelp/tests/tmp_blowup.py", line 51, in class_setup_raises_exception
    raise FakeClassFixtureException
FakeClassFixtureException

ERROR in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTestCase.test2 ... error: yelp.tests.tmp_blowup FakeClassSetupTestCase.test2
Traceback (most recent call last):
  File "./yelp/tests/tmp_blowup.py", line 51, in class_setup_raises_exception
    raise FakeClassFixtureException
FakeClassFixtureException

ERROR in 0.00s

FAILED.  2 tests / 1 case: 0 passed, 2 failed.  (Total test time 0.00s)

Setup phase of class_setup_teardown:

$ PYTHONPATH=~/Testify/ ~/Testify/bin/testify -v yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase
yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test1 ... error: yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test1
Traceback (most recent call last):
  File "/usr/lib/python2.6/contextlib.py", line 16, in __enter__
    return self.gen.next()
  File "./yelp/tests/tmp_blowup.py", line 64, in class_setup_teardown_raises_exception_during_setup
    raise FakeClassFixtureException
FakeClassFixtureException

ERROR in 0.00s
yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test2 ... error: yelp.tests.tmp_blowup FakeClassSetupTeardownSetupExceptionTestCase.test2
Traceback (most recent call last):
  File "/usr/lib/python2.6/contextlib.py", line 16, in __enter__
    return self.gen.next()
  File "./yelp/tests/tmp_blowup.py", line 64, in class_setup_teardown_raises_exception_during_setup
    raise FakeClassFixtureException
FakeClassFixtureException

ERROR in 0.00s

FAILED.  2 tests / 1 case: 0 passed, 2 failed.  (Total test time 0.00s)

[bukzor]

@mrtyler: That makes lots of sense, thanks for the reply. The behavior is certainly improved, and that's the main thing that matters.

[blampe]

LGTM!