Opened 2 years ago

Closed 2 years ago

#5830 task closed fixed (fixed)

Make twisted/test/test_text pass under Python 3

Reported by: vperic Owned by: vperic
Priority: normal Milestone: Python-3.x
Component: core Keywords: py3k
Cc: thijs Branch: branches/test_text-5830
(diff, github, buildbot, log)
Author: vperic Launchpad Bug:

Description

As before, to actually make this actually run you need to import unittest from the library, not from trial, and then "python3.2 -m unittest twisted.test.test_text" will run the tests.

The changes:

Replace calls to functions from the string module in favor of string
methods (Refs: #4999); don't use the types module in favor of the
"isinstance" function - types was changed in Python 3 to not include
some base types, like dicts, lists and tuples.

This also required the addition of some tests.

Attachments (2)

test_text.patch (5.0 KB) - added by vperic 2 years ago.
First version
test_text.2.patch (5.0 KB) - added by vperic 2 years ago.
Second version

Download all attachments as: .zip

Change History (16)

Changed 2 years ago by vperic

First version

comment:1 Changed 2 years ago by vperic

  • Type changed from enhancement to task

comment:2 Changed 2 years ago by antoine

Changing exact type tests to isinstance() will change semantics: what happens e.g. to a tuple subclass?

comment:3 follow-up: Changed 2 years ago by vperic

As in, a tuple subclass would be caught now but not before? I guess you are right, but I could argue that the new behaviour is more correct. Right now, subclasses of those types are handled just right any other element, so printed just as a string, which is not what they're supposed to do per the doctest of stringyString; with the new code, they'll also be printed "one element per line".

The function itself isn't used anywhere, so I'm not sure what the original author intended.

Still, if I was to keep the old semantics, how should I fix this? The types module is changed in Python 3.

comment:4 in reply to: ↑ 3 Changed 2 years ago by antoine

Replying to vperic:

Still, if I was to keep the old semantics, how should I fix this?

type(foo) is tuple

comment:5 Changed 2 years ago by vperic

  • Keywords review removed
  • Owner set to vperic

D'oh, you're right. Thanks! I'll update the patch to do just that; dropping the review tag for now.

comment:6 Changed 2 years ago by vperic

  • Keywords review added
  • Owner vperic deleted

Right, fixed that, thanks antoine! Putting back up for review.

Changed 2 years ago by vperic

Second version

comment:7 Changed 2 years ago by thijs

  • Cc thijs added

Could you create a branch for the patch so we can run it on buildbot. This should rule out any test failures. Maybe once #4999 landed?

comment:8 Changed 2 years ago by vperic

  • Owner set to vperic

Yes, definitely, I'll update this once #4999 is in to contain just the py3k changes (which are minimal, really).

comment:9 Changed 2 years ago by itamar

  • Keywords review removed

#4999 was merged, so this can now be updated.

comment:10 Changed 2 years ago by vperic

  • Author set to vperic
  • Branch set to branches/test_text-5830

(In [35184]) Create branch test_text-5830

comment:11 Changed 2 years ago by vperic

I've ordered a test run on just twisted.test.test_text (as it's the only touched module). After removing the added tests, this is actually a very, very simple patch. Build results.

comment:12 Changed 2 years ago by vperic

  • Keywords review added
  • Owner vperic deleted

Ok, it just occurred to me that no one actually gave this a "+1" so I'll put it up for review formally.

comment:13 Changed 2 years ago by thijs

  • Keywords review removed
  • Owner set to vperic

Please merge, thanks.

comment:14 Changed 2 years ago by vperic

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

(In [35186]) Merge test_text-5830: Make t.test.test_text pass under Python 3

Author: vperic
Reviewers: antoine, thijs
Fixes: #5830

Don't use the types module in favor of the "type(object) is sometype"
syntagm - types was changed in Python 3 to not include some base types,
like dicts, lists and tuples.

Note: See TracTickets for help on using tickets.