#6033 enhancement closed fixed (fixed)

Port async TestCase to Python 3

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: trial Keywords:
Cc: Branch: branches/testcase-py3-6033-2
(diff, github, buildbot, log)
Author: itamarst Launchpad Bug:

Description

twisted.trial.unittest.TestCase should run on Python 3.

Change History (13)

comment:1 Changed 22 months ago by itamarst

  • Author set to itamarst
  • Branch set to branches/testcase-py3-6033

(In [35867]) Branching to 'testcase-py3-6033'

comment:2 Changed 22 months ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/testcase-py3-6033

In order to get decent coverage I had to port an unfortunately large bunch of modules, and make reporter at least import (it's used by tests, not by TestCase at least). On the plus side there weren't many changes to make in any of them.

comment:3 Changed 22 months ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar

Thanks.

  1. At the end of twisted/trial/test/test_deferred.py, the _PY3 conditional doesn't seem like it needs to be conditional. trial actually loads TestTester too, but doesn't run it because it has no test_ methods. Whereas the stdlib runner seems to believe that runTest is a test (and this manifests as a problem elsewhere too, so potentially we should fix it, but I don't think it's related to the Python 3 port). So anyway, just delete TestTester unconditionally, I think. Also, the tests in this module are wacko. :/ It seems like they should be written differently (new ticket):
    1. runTest is part of the TestCase API, it's a bad idea to re-use it for something else.
    2. Other trial tests use the loader to get something runnable instead of doing a manual loading step like getTest is doing.
    3. Some of these tests are slow, waiting for a 0.1s timeout before proceeding.
    4. No docs anywhere :/
  2. Looks like test_threads got lost.
  3. in twisted/trial/test/test_tests.py, a skip message is duplicated many times. The idiom of defining the skip message once at the beginning of a module and then re-using it is a sort of nice one I prefer (eg how ipv6Skip works in twisted/internet/test/test_tcp.py).
  4. It would be better to port the twisted.python.deprecate dependency first instead of poking at a couple of its edges in this branch without being able to run its tests. Without running its tests, how do we know the changes are correct?
  5. in twisted/trial/reporter.py
    1. under what conditions does the collections.OrderedDict import fail? OrderedDict appears to be in Python 2.7.
    2. why break the link to untilConcludes? That seems bad.
  6. in twisted/trial/test/test_reporter.py, just ugh ugh ugh ugh

comment:4 Changed 22 months ago by exarkun

  • Component changed from core to trial

comment:5 Changed 22 months ago by itamar

See [35887] for fixes mentioned below.

  1. Made the delete unconditional, opened #6038.
  2. Added it back.
  3. Unified them all into a single message constant.
  4. Will do.
  5. OrderedDict isn't present in Python 2.6. The link to untilConcludes was causing pydoctor errors, probably because of that bug mwhudson fixed recently.
  6. It was like that when I got here.

comment:6 Changed 22 months ago by itamar

OK, #6036 is up for review.

comment:7 Changed 22 months ago by itamarst

  • Branch changed from branches/testcase-py3-6033 to branches/testcase-py3-6033-2

(In [35928]) Branching to 'testcase-py3-6033-2'

comment:8 Changed 22 months ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

comment:9 Changed 22 months ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar

Thanks

  1. twisted/trial/_asynctest.py
    1. zope.interface import belongs above the Twisted imports
    2. The twisted.internet.reactor import hidden in TestCase._run can be moved to module-scope now? Perhaps we also need a ticket for turning it into a parameter... somewhere (on the results object? as an argument to a new run-like method that the runner can call instead of run?)
    3. The timeout support in TestCase._run should probably be based on Deferred cancellation, I suppose? Hard to say, since there is still no real cancellation documentation. Still, I suspect it should be. Can you file a ticket for that?
    4. Another nested reactor import in _runFixturesAndTest.
    5. And another in _wait
  2. twisted/trial/reporter.py
    1. The untilConcludes link is still being broken. Maybe you said something about that on IRC but I forget what. It doesn't seem like we need to break this link though.
    2. zope.interface import here should also be moved above Twisted imports
  3. twisted/trial/test/test_asyncassertions.py
    1. The stdlib unittest import belongs above the Twisted imports
    2. Many tests in this module fail to follow the coding standard. Can you file a ticket for fixing that?
  4. twisted/trial/test/test_deferred.py
    1. stdlib unittest import should be above Twisted imports
    2. Many tests in this module fail to follow the coding standard. Can you file a ticket for fixing that?
    3. I guess there should be a ticket for the del TestLoader thing. We shouldn't really need to be doing that.
  5. twisted/trial/test/test_tests.py
    1. stdlib unittest import belongs above the Twisted imports
    2. There are references to #6034 in the code. #6034 was closed as a duplicate of #5964.
    3. Many of the tests in this module do not follow the coding standard. Can you file a ticket for fixing this?

Overall, seems good. Merge when the above are addressed.

comment:10 Changed 22 months ago by itamar

  1. #6047 and #6048.
  2. I'm pretty sure the link is broken due to the bug Michael Hudson recently fixed in pydoctor. I'll try another way, which will definitely work if not now then eventually.
  3. #6049.
  4. #6050, expanded #6038 for the del thing.
  5. #6051.

comment:11 Changed 22 months ago by itamarst

(In [35930]) Address review comments. Refs #6033

comment:12 Changed 22 months ago by itamar

Oh. There's a reason the reactor isn't imported at module level - it prevents trial from choosing a reactor:

./bin/trial: The specified reactor cannot be used, failed with error: reactor already installed.
See the list of available reactors with --help-reactors

So I will revert that particular change and add a comment explaining why there's no import at top level.

comment:13 Changed 22 months ago by itamarst

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

(In [35932]) Merge testcase-py3-6033-2.

Author: itamar
Review: exarkun
Fixes: #6033

twisted.trial.unittest.TestCase now runs on Python 3.

Note: See TracTickets for help on using tickets.