[rfc] [patch] sftp unit tests without ssh

[Robey Pointer]

On 22 Jan 2006, at 20:09, John A Meinel wrote:

> Robey Pointer wrote:
>> Here's a patch to add a subclass of SFTPServer (called
>> SFTPServerWithoutSSH) that sets up a fake sftp server over  
>> loopback but
>> uses a special ssh vendor ("loopback") which just wraps a plain  
>> socket
>> instead of setting up both ends of an SSH Transport.  What this  
>> means is
>> that tests using SFTPServerWithoutSSH will use sftp without all the
>> overhead of actually setting up a secure channel.

[...]


>> === modified file 'bzrlib/tests/stub_sftp.py'
>> --- bzrlib/tests/stub_sftp.py	
>> +++ bzrlib/tests/stub_sftp.py	
>> @@ -68,6 +68,7 @@
>>              self.home = home[len(self.root):]
>>          if (len(self.home) > 0) and (self.home[0] == '/'):
>>              self.home = self.home[1:]
>> +        server._test_case.log('sftpserver - new connection')
>>
>>      def _realpath(self, path):
>>          return self.root + self.canonicalize(path)
>> @@ -117,7 +118,7 @@
>>          try:
>>              if hasattr(os, 'O_BINARY'):
>>                  flags |= os.O_BINARY
>> -            if (attr is not None) and hasattr(attr, 'st_mode'):
>> +            if getattr(attr, 'st_mode', None) is not None:
>>                  fd = os.open(path, flags, attr.st_mode)
>>              else:
>>                  fd = os.open(path, flags)
>
> I've already put this in jam-integration, but no big deal here.
> As an aside, we should change:
>   if hasattr(os, 'O_BINARY'):
>     flags |= os.O_BINARY
> to
>   flags |= getattr(os, 'O_BINARY', 0)
>
> But that isn't relevant to Robey.

Yeah, I only made that change because I needed it to have the tests  
pass when I was testing it.


>> +    def recv_ready(self):
>> +        return True
>> +
>> +
>
> Don't we need recv_ready()? I don't know if the code actually tests
> this, but I know it was a problem for SFTPSubprocess.
> I thought paramiko expected it to be available if pipelining was  
> enabled.

Yeah, it does.  I'm guessing you just didn't notice it when you first  
replied, but it's there. :)


>> @@ -867,23 +891,21 @@
>>          s, _ = self._socket.accept()
>>          # now close the listen socket
>>          self._socket.close()
>> -        self._callback(s, self.stop_event)
>> -
>> +        try:
>> +            self._callback(s, self.stop_event)
>> +        except Exception, x:
>> +            # probably a failed test, might be nice to log it  
>> somehow
>> +            pass
>> +
>
> can't you use at least 'mutter()' here? Though probably warn() is more
> appropriate.

Good call.  I was spacing on how to do logging from there, but warn()  
is exactly what I want.


> So +1, except I don't think you should swallow the Exception, at a
> minimum it needs to be printed.

Agreed.  Attached is a new patch, which changes that 'pass' to a  
'warn', and also asks paramiko to log into the 'bzr' logging tree  
during unit tests (because I think that may squelch periodic "no  
handler found for paramiko" messages that show up when running the  
tests).

robey
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sftp-without-ssh-patch.txt
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20060122/208cf335/attachment.txt