search

Lukasa / requests-ftp

An FTP transport adapter for use with the Python Requests library.
Other
69 stars 25 forks source link

[discussion] A new testing approach using sockets #22

Closed emanuil-tolev closed 7 years ago

emanuil-tolev commented 8 years ago

Dummy socket servers work really well in http client libraries (where I cribbed this from as per you suggestion @Lukasa ), but in those, each request completes on its own, in one socket connection, with one request and one response.

Now, with FTP this is also the case in terms of the protocol - we send USER, PASS, etc. separately. There is one noteable difference: separate data and control channels.

Before we even get to that, something that makes it even more difficult to use the dummy socket server approach to testing is that requests-ftp uses python's ftplib underneath. So when we session.retr("ftp://127.0.0.1:%d/anyfile" % ftpdummy.port) what happens is this:

  1. ftplib consumes a welcome message
  2. it sends "USER anonymous" if no auth provided
  3. it sends "TYPE I", switching to binary mode
  4. it sends "PASV", requesting details for a data conn
  5. it opens a connection to the host and port provided by PASV.
  6. [not gotten to this one yet] it most probably finally sends RETR via the control channel, causing data transfer on the data connection.

I've written code to handle the expected sequence of steps 1-4, in the FTPSocketDummyServer class you can see in this PR. Step 5 would require another thread serving DummyServer for the data connection.

The code looks brittle, and it doesn't really feel right to simulate this whole exchange with a dummy socket server - that's what pyftpdlib (which we use) already does, albeit with a whole load of baggage like accounts, passwords and user home directories.

For the purposes of PR #19 , I think it's probably best to simply make sure the bit that parses the response from the server is unit testable on its own, and write a unit test for it. Not involving any ftp traffic, just calling a python function and asserting what the result should be for inputs "226 Transfer Complete." and "226-Transfer Complete.\n226 Statistics: blah.".

@Lukasa I suppose the question here is - do you (still) think it's possible to use this kind of dummy socket server to test requests-ftp?

Or would a better approach be to aim to refactor as many bits of code as possible so it's testable in very simple string-based unit tests + keep pyftpdlib for "integration" tests?

emanuil-tolev commented 8 years ago

(not sure why the build is failing, I can do from dummyserver.server import FTPSocketDummyServer just fine - but the PR's not supposed to be mergeable functionality anyway, just show the state of dummy socket server I got to)

Lukasa commented 8 years ago

I'm open to just swapping to pyftplib for the underlying stuff. You're right: this does feel brittle.

emanuil-tolev commented 7 years ago

Superseded by #28