#3939 ftplib tests

[Benjamin, 2008-09-23 13:18:52]

Overall, great work! I just have a few comments. Also, it would be nice to see a
few tests for bad server responses.

http://codereview.appspot.com/5698/diff/1/2
File Lib/test/test_ftplib.py (right):

http://codereview.appspot.com/5698/diff/1/2#newcode16
Line 16: HOST = test_support.HOST
This should be from "test.test_support import HOST".

http://codereview.appspot.com/5698/diff/1/2#newcode28
Line 28: asyncore.dispatcher.__init__(self)
"super()" should be used here.

http://codereview.appspot.com/5698/diff/1/2#newcode33
Line 33: self.handler = DummyFTPHandler
This should be a class attribute.

http://codereview.appspot.com/5698/diff/1/2#newcode318
Line 318: # we don't have a reference of the data received on the server
Couldn't you store the data in DummyFTPHandler and inspect it through
self.client.handler?

[billiejoex, 2008-09-23 15:44:26]

> http://codereview.appspot.com/5698/diff/1/2#newcode16
> Line 16: HOST = test_support.HOST
> This should be from "test.test_support import HOST".

Ok.

> http://codereview.appspot.com/5698/diff/1/2#newcode28
> Line 28: asyncore.dispatcher.__init__(self)
> "super()" should be used here.

How? 

> http://codereview.appspot.com/5698/diff/1/2#newcode33
> Line 33: self.handler = DummyFTPHandler
> This should be a class attribute.

Yeah, better.

> http://codereview.appspot.com/5698/diff/1/2#newcode318
> Line 318: # we don't have a reference of the data received on the server
> Couldn't you store the data in DummyFTPHandler and inspect it through
> self.client.handler?

Ok, I'll see to arrange that in some way.

On 2008/09/23 13:18:52, Benjamin wrote:
> Overall, great work! I just have a few comments. Also, it would be nice to see
a
> few tests for bad server responses.
> 
> http://codereview.appspot.com/5698/diff/1/2
> File Lib/test/test_ftplib.py (right):
> 
> http://codereview.appspot.com/5698/diff/1/2#newcode16
> Line 16: HOST = test_support.HOST
> This should be from "test.test_support import HOST".
> 
> http://codereview.appspot.com/5698/diff/1/2#newcode28
> Line 28: asyncore.dispatcher.__init__(self)
> "super()" should be used here.
> 
> http://codereview.appspot.com/5698/diff/1/2#newcode33
> Line 33: self.handler = DummyFTPHandler
> This should be a class attribute.
> 
> http://codereview.appspot.com/5698/diff/1/2#newcode318
> Line 318: # we don't have a reference of the data received on the server
> Couldn't you store the data in DummyFTPHandler and inspect it through
> self.client.handler?

[Benjamin, 2008-09-23 22:56:30]

On 2008/09/23 15:44:26, billiejoex wrote:
> > http://codereview.appspot.com/5698/diff/1/2#newcode28
> > Line 28: asyncore.dispatcher.__init__(self)
> > "super()" should be used here.
> 
> How? 

super(DummyFTPServer, self).__init__()

[billiejoex, 2008-09-24 00:01:30]

On 2008/09/23 13:18:52, Benjamin wrote:

> http://codereview.appspot.com/5698/diff/1/2#newcode16
> Line 16: HOST = test_support.HOST
> This should be from "test.test_support import HOST".

Done.

> http://codereview.appspot.com/5698/diff/1/2#newcode28
> Line 28: asyncore.dispatcher.__init__(self)
> "super()" should be used here.

Done as recommended by Benjamin.

> http://codereview.appspot.com/5698/diff/1/2#newcode33
> Line 33: self.handler = DummyFTPHandler
> This should be a class attribute.

Done.

> http://codereview.appspot.com/5698/diff/1/2#newcode318
> Line 318: # we don't have a reference of the data received on the server
> Couldn't you store the data in DummyFTPHandler and inspect it through
> self.client.handler?

Done.

> Also, it would be nice to see a few tests for bad server responses.

I added two tests for rename() and delete().
All the other ftplib.FTP methods implicitly uses ftplib.FTP.sendcmd/voidcmd
which are already tested against bad server responses in
TestFTPClass.test_exceptions and TestFTPClass.test_voidcmd.


I'm attaching the new patch on the Python bug tracker.