Opened 2 years ago

Closed 2 years ago

#5853 enhancement closed fixed (fixed)

Add a TestCase which does not support asynchronous tests but which does have the other nice features of trial

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone: Python 3.3 Minimal
Component: trial Keywords: py3k
Cc: jml Branch: branches/synchronous-testcase-5853-2
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

twisted.trial.unittest.TestCase is the public interface for a great deal of functionality. It provides a number of convenient testing helpers which the standard library unittest.TestCase is missing (although fortunately the standard library is slowly catching up). It also provides support for reactor-based asynchronous tests, which is sometimes a convenient way to write tests.

These are unrelated concerns, though. For the sake of factoring, code simplicity, and (most recently) porting to Python 3, it would be convenient if the reactor-related functionality were provided separately from the other (simpler) functionality.

The idea for doing this is to have a couple different TestCase classes provided by trial. One of which is just the simple testing conveniences (such as mktemp and our various extra assertion methods), the other of which adds to these the reactor-based testing features.

Another explicit goal of this work is for parts of trial's test suite which don't depend on the reactor (and which therefore don't test parts of trial that depend on the reactor) to be tested with the non-reactor version of this TestCase, allowing for a more incremental bootstrapping of trial onto Python 3.

Change History (16)

comment:1 Changed 2 years ago by DefaultCC Plugin

  • Cc jml added

comment:2 Changed 2 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/synchronous-testcase-5853

(In [35097]) Branching to 'synchronous-testcase-5853'

comment:4 Changed 2 years ago by exarkun

(In [35099]) Refactor a few basic TestCase unit tests into three new categories

  1. Tests for fail which depend on stdlib unittest working
  2. Tests for failIf/assertFalse which only depend on the tests from point 1 passing
  3. More tests not yet addressed

After dealing with point 3 some more, the result should be some tests that can safely be
implemented using SynchronousTestCase and also applied to that class. Potentially much
of point 3 can be delayed to a later time, since it may just be nice cleanup rather than
necessary refactoring.

refs #5853

comment:5 Changed 2 years ago by exarkun

(In [35120]) Introduce SynchronousTestCase with no additional behavior beyond _Assertions; adjust the test suite to require that it have the assertion methods and behavior we expect of it.

refs #5853

comment:6 Changed 2 years ago by exarkun

  • Branch changed from branches/synchronous-testcase-5853 to branches/synchronous-testcase-5853-2

(In [35178]) Branching to 'synchronous-testcase-5853-2'

comment:7 Changed 2 years ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

Somewhat large. It's probably possible to split up, if desired. An outline of the changes:

  • test_assertions.py is largely just switched to use SynchronousTestCase instead of TestCase. The assertion methods are simple and self-contained, so no attempt is made to apply these tests to TestCase directly. Rather, it is assumed that they will still work when inherited by that class.
  • suppression.py has twice as many test cases now, one version for each of SynchronousTestCase and TestCase. This allows all of the suppression tests to be applied to both classes, necessary because of custom suppression logic in each of them.
  • Similar changes to erroneous.py, and a new skipping.py which is sort of a dumping group for a bunch of other tests pulled out of test_tests.py.
  • test_util.py has one instance of the same sort of change, plus some other minor improvements and adjustment to reflect the new names in suppression.py
  • test_reporter.py is also adjusted to reflect a changed name in erroneous.py
  • test_log.py also has tests doubled to apply to both classes, as well as one new test to explicitly cover the removal of the twisted.python.log observer.
  • test_tests.py has most of its tests doubled to apply to both SynchronousTestCase and TestCase. Additionally, many of the nested test cases it previously defined have been moved to skipping.py where they were also doubled. A few tests are also modified for content, mostly to add new assertions for missing coverage.
  • reporter.py is taught about the very intimate details of SynchronousTestCase so it can "trim" frames from a call stack for reporting purposes.
  • unittest.py has the new SynchronousTestCase introduced, with as much functionality from TestCase moved up onto it as is practical. Some changes to how TestCase implements test running are made to facilitate code sharing.
  • In a few places elsewhere, tests for trial functionality are switched from TestCase to SynchronousTestCase where doing so was trivial. Due to the large size of trial's test suite, I decided not to make the rest of these changes in this branch. I'll file a new ticket and submit the changes there.

Build results

comment:8 Changed 2 years ago by itamar

  • Milestone set to Python 3.3 Minimal
  • skipping.SkippedClassMixin.test_skip1 has a NameError. Not that it matters if it's actually skipped (the same NameError happens elsewhere as well)
  • I would suggest using RuntimeError(...) in StrictTodoMixin, just to get a head start on Python 3 support (and to get in the habit of looking for it).
  • Need to add SynchronousTestCase to unittest.__all__.

More tomorrow, when I am less tired.

comment:9 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun
  • SynchronousTestCase never defines self._cleanups, even though it implements addCleanup. The fact this wasn't caught suggests there's a missing test.
  • SynchronousTestCase uses self._passed and self._warnings (and resets them in run()), so not sure why they're only set in TestCase.__init__.
  • Perhaps self._timedOut=False shouldn't be in SynchronousTestCase at all, only in the subclass.
  • Two line breaks between TestCase methods.
  • I assumed the code in TestCase is pretty much pre-existing code; if any of it is not, please point it out.
  • SynchronousTestCase never sets _testMethodName (TestCase does), yet uses it. Again, not sure how this passes tests (pyunit does this maybe?).
  • In test_tests, the following are probably inheritng from the wrong test case class (the cleanup one is definitely wrong, since it has an addCleanup that returns a Deferred):
    class AsynchronousAddCleanupTests(AddCleanupMixin, unittest.SynchronousTestCase):
    class AsynchronousSuiteClearingTests(SuiteClearingMixin, unittest.SynchronousTestCase):
    class AsynchronousTestDecoratorTests(TestDecoratorMixin, unittest.SynchronousTestCase):
    class AsynchronousMonkeyPatchTests(MonkeyPatchMixin, unittest.SynchronousTestCase):
    class AsynchronousIterateTestsTests(IterateTestsMixin, unittest.SynchronousTestCase):
    
    • To prevent silent failures from previous item, I suggest having SynchronousTestCase complain if any of the methods it calls (setUp, tearDown, test method, addCleanup function) return a Deferred.

comment:10 Changed 2 years ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

skipping.SkippedClassMixin.test_skip1 has a NameError. Not that it matters if it's actually skipped (the same NameError happens elsewhere as well)

Fixed in r35237.

I would suggest using RuntimeError(...) in StrictTodoMixin, just to get a head start on Python 3 support (and to get in the habit of looking for it).

Fixed in r35238.

Need to add SynchronousTestCase to unittest.all.

Fixed in r35239.

SynchronousTestCase never defines self._cleanups, even though it implements addCleanup. The fact this wasn't caught suggests there's a missing test.

Ha ha. stdlib unittest.TestCase.__init__ defines _cleanups, so it worked by accident, at least on Python 2.7. I'll move the initializer to SynchronousTest.__init__ so the code makes more sense, but it may not even be necessary. I moved it up, though, to make the code make more sense. in r35241.

SynchronousTestCase uses self._passed and self._warnings (and resets them in run()), so not sure why they're only set in TestCase.init.

_passed is the same as _cleanups above. I don't see the same case applying to _warnings? It looks like it is only set in run to me.

Perhaps self._timedOut=False shouldn't be in SynchronousTestCase at all, only in the subclass.

Yea. r35242.

Two line breaks between TestCase methods.

Fixed in r35243.

I assumed the code in TestCase is pretty much pre-existing code; if any of it is not, please point it out.

It is almost exclusively pre-existing, but a few areas had minor changes made:

  1. __init__ is now empty, with all initialization moved up. Method still exists for documentation purposes.
  2. _run used to call self.getSuppress(), now it calls the private self._getSuppress() to bypass the compatibility layer I added to TestCase (more below)
  3. _ebDeferSetUp used to call self._getReason which has been renamed self._getSkipReason, moved up to the base class, changed to take an exception instead of a failure, and improved to emit the warning about the correct file/line.
  4. Similar change in _ebDeferTestMethod
  5. _runFixturesAndTest is the guts of the old TestCase.run, pulled out into a separate method so that most of TestCase.run could be factored up to SynchronousTestCase.run
  6. getSuppress now just calls the private base method _getSuppress. SynchronousTestCase needs suppression logic too, but I didn't feel the need to add getSuppress as a public API on the new class.

SynchronousTestCase never sets _testMethodName (TestCase does), yet uses it. Again, not sure how this passes tests (pyunit does this maybe?).

Another case where pyunit does it for us, yea. Dealt with this at the same time as _cleanups.

In test_tests, the following are probably inheritng from the wrong test case class (the cleanup one is definitely wrong, since it has an addCleanup that returns a Deferred):

Fixed in r35240.

To prevent silent failures from previous item, I suggest having SynchronousTestCase complain if any of the methods it calls (setUp, tearDown, test method, addCleanup function) return a Deferred.

I like that idea, but it will make SynchronousTestCase depend on twisted.internet.defer, which would be counter-productive. Do you have a suggestion for a good way to do this? As I understand it, test methods are allowed to return anything they want in xUnit, otherwise we could just assert None return values.

Thanks. Sorry about the suffering this branch has caused.

comment:11 Changed 2 years ago by exarkun

Oops, latest build results.

comment:12 Changed 2 years ago by exarkun

Okay, another observation, Python 2.6 does not set _cleanups, so at least there the tests were failing before, hopefully they'll be passing now.

comment:13 Changed 2 years ago by exarkun

  • Owner itamar deleted

comment:14 Changed 2 years ago by exarkun

(In [35244]) Fix a few pydoctor errors caused by unresolvable link targets

refs #5853

comment:15 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner set to exarkun

OK, looks good, please merge.

comment:16 Changed 2 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [35249]) Merge synchronous-testcase-5853-2

Author: exarkun
Reviewer: itamar
Fixes: #5853

Factor trial's reactor-related testing features into a separate class from its other helpful
functionality. A new test case base class for the reactor-free version of the functionality is
now provided as twisted.trial.unittest.SynchronousTestCase.

Note: See TracTickets for help on using tickets.