Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#5885 defect closed fixed (fixed)

Once SynchronousTestCase is ported to Python 3, switch test modules using pyunit back to using it

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

Description (last modified by itamar)

test_compat, test_versions and test_monkey (after #5896 and #5785 are merged) use pyunit, and in some cases have workarounds for lack of SynchronousTestCase: reimplementing mktemp(), del instead of skip support. Once SynchronousTestCase is ported to Python 3, we should revert these changes.

Change History (14)

comment:1 Changed 2 years ago by itamar

  • Milestone set to Python 3.3 Minimal
  • Owner set to itamar

comment:2 Changed 2 years ago by itamar

  • Description modified (diff)
  • Summary changed from Once SynchronousTestCase is ported to Python 3, switch twisted.test.test_compat back to using it to Once SynchronousTestCase is ported to Python 3, switch test modules using pyunit back to using it

comment:3 Changed 2 years ago by itamar

  • Description modified (diff)

comment:4 Changed 2 years ago by itamar

There's a bunch more relevant modules now. Grepping for "5885" should find them all (or just look through list of test modules in _twistedpython3.py.)

comment:5 Changed 23 months ago by itamarst

  • Author set to itamarst
  • Branch set to branches/synctest-py3-5885

(In [35752]) Branching to 'synctest-py3-5885'

comment:6 Changed 23 months ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

Ready for review. http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/synctest-py3-5885 is running.

  1. Also covers #5884.
  2. Seems to fix the random test_log errors we were seeing.
  3. I had to semi-fix some brokenness in test_log.py to make the tests pass after switching to SynchronousTestCase.

comment:7 Changed 22 months ago by therve

  • Owner changed from exarkun to therve

comment:8 Changed 22 months ago by therve

  • Keywords review removed
  • Owner changed from therve to itamar
  1. You removed blank lines in from of test_doubleErrorDoesNotRemoveObserver and test_showwarning, but they should stay.
  1. Pyflakes:
    twisted/python/test/test_versions.py:12: 'tempfile' imported but unused
    twisted/python/test/test_utilpy3.py:12: '_PY3' imported but unused
    twisted/test/test_paths.py:11: 'tempfile' imported but unused
    twisted/test/test_compat.py:11: 'tempfile' imported but unused
    

Please merge once fixed.

comment:9 Changed 22 months ago by itamarst

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

(In [35792]) Merge synctest-py3-5885.

Author: itamar
Review: therve
Fixes: #5884, #5885

Remove some temporary code and use of pyunit that worked around unported trial infrastructure that is now available on Python 3.

comment:10 Changed 22 months ago by ralphm

  • Resolution fixed deleted
  • Status changed from closed to reopened

This change affects the initialization of _oldshowwarning in twisted.python.log. Because of it, the replacement for warnings.showwarning in startLoggingWithObserver never happens, and no warnings will be routed to the Twisted Logging System anymore. The following example shows the problem:

import warnings
import sys

from twisted.python import log

# log._oldshowwarning = None
log.startLogging(sys.stdout, setStdout=False)
warnings.warn("Oops", UserWarning)

The warning is printed by the default showwarning implementation (no timestamps, etc.):

2012-10-09 12:09:35+0200 [-] Log opened.
q2.py:8: UserWarning: Oops
  warnings.warn("Oops", UserWarning)

The commented-out line reverts to the correct behavior:

2012-10-09 12:10:04+0200 [-] Log opened.
2012-10-09 12:10:04+0200 [-] q2.py:8: exceptions.UserWarning: Oops

comment:11 Changed 22 months ago by exarkun

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

comment:12 Changed 22 months ago by itamar

I assume JP re-closed this because the issue was filed on the wrong ticket. I will, in any case, revert and then fix the appropriate code today.

comment:13 Changed 22 months ago by ralphm

No, it was because I reopened the issue manually instead of by reverting the change. JP mentioned you'd probably do that anyway. I chose this ticket as the merged branch had this ticket number.

comment:14 Changed 22 months ago by itamar

  1. Since I didn't do it earlier: thanks so much for reporting this!
  2. I guess I shouldn't be assuming (http://drinkydrink.com/noone.html); I thought it was the log ticket, my bad.
Note: See TracTickets for help on using tickets.