Opened 2 years ago

Last modified 9 months ago

#5812 enhancement new

update print statement in twisted.test

Reported by: thijs Owned by: teotwaki
Priority: low Milestone: Python-3.x
Component: core Keywords:
Cc: thijs, sebastian.lauwers@… Branch: branches/print-tests-5812
(diff, github, buildbot, log)
Author: thijs Launchpad Bug:

Description

The print statement becomes print() in Python 3, and is supported in Python 2.6 and up.

twisted/test/test_newcred.py:105:        # print l[0].getBriefTraceback()
twisted/test/test_doc.py:50:                # print e
twisted/test/test_doc.py:74:                # print moduleName, "misbehaved:", e
twisted/test/test_doc.py:94:                # print e
twisted/test/test_persisted.py:340:        # print repr(uj.__dict__)
twisted/test/process_signal.py:6:print 'ok, signal us'
twisted/test/test_threads.py:331:    print 'threaded call'
twisted/test/process_linger.py:8:print "here is some text"
twisted/test/process_linger.py:10:print "goodbye"
twisted/test/test_process.py:788:        if self.verbose: print "closing stdin [%d]" % num
twisted/test/test_process.py:793:        if self.verbose: print self.pp[0].finished, self.pp[1].finished
twisted/test/test_process.py:799:        if self.verbose: print "starting processes"
twisted/test/test_process.py:821:        if self.verbose: print "kill [%d] with SIGTERM" % num
twisted/test/test_process.py:826:        if self.verbose: print self.pp[0].finished, self.pp[1].finished
twisted/test/test_process.py:829:        if self.verbose: print "starting processes"
twisted/test/test_process.py:836:        if self.verbose: print "starting processes"
twisted/test/test_process.py:843:        if self.verbose: print "starting processes"
twisted/test/test_process.py:872:            #print "len", len(self.data)
twisted/test/test_process.py:880:                #print "state2", self.state
twisted/test/process_twisted.py:27:        print "connection made"
twisted/test/process_twisted.py:33:        print "readConnectionLost"
twisted/test/process_twisted.py:36:        print "writeConnectionLost"
twisted/test/process_twisted.py:39:        print "connectionLost", reason
twisted/test/test_tcp.py:853:            print "getPeer returned non-TCP socket:", peer
twisted/test/test_tcp.py:857:            print "getHost returned non-TCP socket:", us
twisted/test/test_log.py:740:        print "This",
twisted/test/test_log.py:741:        print "is a test"
twisted/test/test_log.py:768:        print unicodeString
twisted/test/process_fds.py:12:if debug: print >>stderr, "this is stderr"
twisted/test/process_fds.py:15:if debug: print >>stderr, "read(0):", abcd
twisted/test/process_fds.py:19:if debug: print >>stderr, "os.write(1, righto)"
twisted/test/process_fds.py:24:if debug: print >>stderr, "read(3):", efgh
twisted/test/process_fds.py:28:if debug: print >>stderr, "os.close(4)"
twisted/test/process_fds.py:32:if debug: print >>stderr, "read(5):", eof
twisted/test/process_fds.py:36:if debug: print >>stderr, "os.write(1, closed)"
twisted/test/process_fds.py:39:if debug: print >>stderr, "sys.exit(0)"
twisted/test/test_paths.py:287:            # print fspath, '=>', arcpath
twisted/test/iosim.py:219:            print '-- GLUG --'
twisted/test/iosim.py:225:            print '.'
twisted/test/iosim.py:228:                print 'C: '+repr(cData)
twisted/test/iosim.py:230:                print 'S: '+repr(sData)
twisted/test/iosim.py:240:                print '* C'
twisted/test/iosim.py:247:                print '* S'
twisted/test/test_dirdbm.py:37:        # print s['key']
twisted/test/test_banana.py:206:        # print repr(crashString)
twisted/test/process_cmdline.py:5:    print x
twisted/test/crash_test_dummy.py:17:        #print 'X',self.x,'doing!'
twisted/test/test_internet.py:1014:        print 'done', res
twisted/test/test_internet.py:1017:        print 'failed'

Attachments (6)

print-core-tests-5812.patch (12.2 KB) - added by thijs 2 years ago.
0001-Converted-commented-print-statements-to-print-functi.patch (5.7 KB) - added by teotwaki 10 months ago.
0002-Converted-print-statement-to-print-function-in-test_.patch (720 bytes) - added by teotwaki 10 months ago.
0003-Added-print_function-import-from-__future__.patch (5.6 KB) - added by teotwaki 10 months ago.
print-tests-5812-rebase-conflict-fixes.patch (11.3 KB) - added by teotwaki 10 months ago.
This patch is a rebase on top of TOT of the old print-test-5812 branch, fixing conflicts.
print-tests-5812-imports-and-comments.patch (18.0 KB) - added by teotwaki 10 months ago.
This patch includes the previous patch, but also adds the correct future_statements for files that use print_function and converts commented print_statements

Download all attachments as: .zip

Change History (19)

comment:1 Changed 2 years ago by thijs

Should those prints that are commented out be fixed as well or removed?

comment:2 Changed 2 years ago by vperic

I'd personally leave them as is (or delete them if they are obviously bogus).. on the other hand, there's like 10 of them so might as well convert them, no reason to stall this ticket because of that. BTW, you can use the 2to3 fixer to make these changes automatically: 2to3 -f print -w -n twisted/test and then just add the __future__ imports manually. At least all of these changes will be covered. :)

Changed 2 years ago by thijs

comment:3 Changed 2 years ago by thijs

  • Keywords review added

Attached patch replaces the active print() statements in twisted.test

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

In twisted.test.test_internet, there is a string, "resolve_helper", which then gets written to a file and executes, which has print statements. I guess it should be updated also.

Other than that, looks good to me, but last time I reviewed something I had to revert the change, so I'll let someone else take a look. :)

comment:5 in reply to: ↑ 4 Changed 2 years ago by thijs

Replying to vperic:

In twisted.test.test_internet, there is a string, "resolve_helper", which then gets written to a file and executes, which has print statements. I guess it should be updated also.

Other than that, looks good to me, but last time I reviewed something I had to revert the change, so I'll let someone else take a look. :)

I should've made a branch and run it against buildbot, let me do that here as well.

comment:6 Changed 2 years ago by thijs

  • Author set to thijs
  • Branch set to branches/print-tests-5812

(In [35052]) Branching to 'print-tests-5812'

comment:7 Changed 2 years ago by thijs

(In [35056]) apply print-core-tests-5812.patch, add news file. refs #5812

comment:8 Changed 2 years ago by thijs

The prints in resolve_helper were fixed and build results are here.

comment:9 Changed 2 years ago by khorn

  • Owner set to khorn
  • Status changed from new to assigned

comment:10 Changed 2 years ago by khorn

  • Keywords review removed
  • Owner changed from khorn to thijs
  • Status changed from assigned to new

Looks pretty good, except:

The following files don't have:

from __future__ import print_function

and probably should for 2.6 compatibility.

twisted/test/iosim.py
twisted/test/process_cmdline.py
twisted/test/process_linger.py
twisted/test/process_signal.py
twisted/test/test_internet.py

comment:11 Changed 10 months ago by teotwaki

  • Cc sebastian.lauwers@… added
  • Keywords review added

Hi thijs,

Attached are a few patches that provide the changes requested in the comments.

Changed 10 months ago by teotwaki

This patch is a rebase on top of TOT of the old print-test-5812 branch, fixing conflicts.

Changed 10 months ago by teotwaki

This patch includes the previous patch, but also adds the correct future_statements for files that use print_function and converts commented print_statements

comment:12 Changed 10 months ago by teotwaki

To be clear, please ignore the three patches I posted yesterday (I don't know how to remove them).

comment:13 Changed 9 months ago by adiroiban

  • Keywords review removed
  • Owner changed from thijs to teotwaki

Hi and thanks for the work in this!

I have reviewed the latest patch only. Looks good.

# print('X',self.x,'doing!')

and

# print(skey?)
# print(e)
# print(l[0].getBriefTraceback())
# print(repr(uj.dict))
# print("len", len(self.data))
# print("state2", self.state)

Do we need to keep these lines commented?

If needed, maybe add another comment to explain why it is needed.


Minor comment: In twisted/test/test_tcp.py have imports from future, but I don't see print method used in there.

---

Not sure if this indentation is compliant with code convention:

pyArgs = [pyExe, "-u", "-c", 
         "from __future__ import print_function; print('hello')"]

maybe

pyArgs = [
    pyExe, "-u", "-c", 
    "from __future__ import print_function; print('hello')",
    ] 

---

Please check the comments and let me know if you think that make sense!

Thanks!

Note: See TracTickets for help on using tickets.