Ticket #5885 defect closed fixed

Opened 20 months ago

Last modified 19 months ago

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) (diff)

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

1

Changed 20 months ago by itamar

  • owner set to itamar
  • milestone set to Python 3.3 Minimal

2

Changed 20 months 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

3

Changed 20 months ago by itamar

  • description modified (diff)

4

Changed 20 months 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.)

5

Changed 19 months ago by itamarst

  • branch set to branches/synctest-py3-5885
  • branch_author set to itamarst

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

6

Changed 19 months ago by itamar

  • owner changed from itamar to exarkun
  • keywords review added

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.

7

Changed 19 months ago by therve

  • owner changed from exarkun to therve

8

Changed 19 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.

9

Changed 19 months ago by itamarst

  • status changed from new to closed
  • resolution set to fixed

(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.

10

Changed 19 months ago by ralphm

  • status changed from closed to reopened
  • resolution fixed deleted

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

11

Changed 19 months ago by exarkun

  • status changed from reopened to closed
  • resolution set to fixed

12

Changed 19 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.

13

Changed 19 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.

14

Changed 19 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.