Refactor tests

[BenMatteson]

Some of the tests need to be rewritten to pass reliably (or so others can pass) (see TODOs). Alternately, a setup guide/script/something for the server that's to be tested against.

Some of the integration tests count on certain behavior from other tests which isn't currently happening. they should be rewritten to so the other tests can pass. (e.g. this test was written, but it does something else and is called after the chmod tests.)

Alternately, we drop the "unit test" pretext because in reality we've got a bunch of integration tests written like unit tests, some of which, mine included, casually call functions that are "tested" later in the same way. so we could instead roll them together into test cases that actually test sequences of actions like mkdir -> chmod -> rm, rather than enforcing specific side effects in the faux unit tests. But that's more work...

[ppdom]

I added a fix for the TODOs in MkdirCommandTestCase in my latest PR.

[BenMatteson]

Cool. I'm also realizing I think I misunderstood the intention of those todos, I was thinking that they were suggesting that the tests for mkdir should create the file (for example) for some reason... but there's still some testing work to do, so I'm just going to rewrite this task.

[eddiekelleypdx]

The original intentions behind the TODOs was to try to structure test runs of different tests to occur before/after each other (e.g., so that mkdir can create a directory for the ls test, rmdir can be used to cleanup that directory, etc.)... In hindsight, that may prove to be difficult (although it seems possible).

Another, more modular strategy might be to implement the setUp() and tearDown() methods within each SFTPTestCase class to create/remove resources as needed before/after running each test case. That way, developers can create the directories or files that they need for their specific test case, and not have to rely on other developers' tests?

[BenMatteson]

oh, ok, so I was reading it right the first time (at least yours). if we want to rewrite the tests to enforce that, that's fine. I kinda prefer the idea of having all the code that deals with whatever file put together, because it makes each test more robust, and self contained, but as long as everything is happening in the suite it should be fine.

[eddiekelleypdx]

Related to this issue: errors are occurring when running integration tests with a non-empty 'downloads' folder in the repo directory:

======================================================================
ERROR: tearDownClass (__main__.ChmodCommandTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/FTP_tests.py", line 53, in tearDownClass
    rmdir(DOWNLOADS_DIRECTORY)
OSError: [Errno 66] Directory not empty: 'downloads'

To get around this issue, we can most likely use something like shutil.rmtree() instead of rmdir().