Opened 5 years ago

Closed 4 years ago

Last modified 4 years 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 Turner-Trauring Owned by: Itamar Turner-Trauring
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/synctest-py3-5885
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst

Description (last modified by Itamar Turner-Trauring)

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 5 years ago by Itamar Turner-Trauring

Milestone: Python 3.3 Minimal
Owner: set to Itamar Turner-Trauring

comment:2 Changed 5 years ago by Itamar Turner-Trauring

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

comment:3 Changed 5 years ago by Itamar Turner-Trauring

Description: modified (diff)

comment:4 Changed 5 years ago by Itamar Turner-Trauring

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 5 years ago by itamarst

Author: itamarst
Branch: branches/synctest-py3-5885

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

comment:6 Changed 5 years ago by Itamar Turner-Trauring

Keywords: review added
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

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 5 years ago by therve

Owner: changed from Jean-Paul Calderone to therve

comment:8 Changed 5 years ago by therve

Keywords: review removed
Owner: changed from therve to Itamar Turner-Trauring
  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 5 years ago by itamarst

Resolution: fixed
Status: newclosed

(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 4 years ago by ralphm

Resolution: fixed
Status: closedreopened

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 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: reopenedclosed

comment:12 Changed 4 years ago by Itamar Turner-Trauring

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 4 years 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 4 years ago by Itamar Turner-Trauring

  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.