BenMatteson / CS410_Agile_Group2

CS410 Agile Practice Project - FTP Client
0 stars 2 forks source link

Add rmdir command and unit tests #50

Closed ppdom closed 5 years ago

ppdom commented 5 years ago

FTP users should be able to delete directories on the remote server so that the remote directory tree can be managed via FTP.

Closes #3 Closes #4

Acceptance Criteria:

ppdom commented 5 years ago

After the latest merge some of the unit tests for the other commands weren't passing for me anymore

eddiekelleypdx commented 5 years ago

After the latest merge some of the unit tests for the other commands weren't passing for me anymore

There is an open issue for this (#49 Refactor tests), and pull request #53 includes some examples for per-TestCase setUp() and tearDown() of files/directories that are required for the integration tests in FTP_main.py to pass. I didn't implement those fixes for all TestCases - only for the new commands I was adding, and for some easy existing ones, so there's still some work to be done there.

All of the unit tests in Client_Unittest.py seem to be passing still, but it's necessary to run that command like this: export PYTHONPATH='..:.' ; python3 SFTPClient/Client_Unittest.py -v

brialee commented 5 years ago

After the latest merge from master, all the unittests related to rmdir passed.

brialee commented 5 years ago

I was able to delete a folder remotely for which there were only read permissions. The folder in question was empty. I was also able to remove a directory for which I had r/w permissions, however inside the directory was a file for which I had only read permissions.

ppdom commented 5 years ago

I looked into this and apparently as long as you have write and execute permissions on the parent directory you can still delete read only files/directories within said parent directory. I find this utterly bizarre and I had no idea file permissions worked that way in Linux.

To see this in action try these within our FTP program:

The following will delete b: mkdir a/b chmod a/b 444 rmdir a/b

But the following fails: mkdir x/y chmod x 444 rmdir x/y <-- fails rmdir x <-- fails too

eddiekelleypdx commented 5 years ago

If it weren't for that pesky Acceptance Criteria, this would seem to be working 100%... Since we made it up, can we modify the acceptance criteria (to include exceptions like 'unless user has write access on parent folder" or whatever matches this behavior)?

BenMatteson commented 5 years ago

I mean, I'd say yes in this case at least, we really did make that up out of nowhere, and 'obey the host systems permissions' (not that we have a choice?) seems like it was the intent of the requirement. that and I was iffy on that one to begin with. 😉

ppdom commented 5 years ago

I updated the acceptance criteria