# Test on remove function

let's say I want to create a 'utils' package with several functions working on files and folders. I want to do it once properly so I can use them in all my personal projects without feeling to re invent the wheel each time. I also want to learn 2 items by the way:

• exception handling, already read a lot about EAFP vs LBYL, I think the following function suits well to EAFP.
• unit tests with pytest tool, it seems simple enough to start with.

My function is:

def removeanything(src):
"""
remove files or folders
"""
try:
os.remove(src)
print('File is removed')
shutil.rmtree(src)
print('Folder is removed')
except FileNotFoundError:
print('File/folder is not existing')
except PermissionError:
print('Not allowed to suppress this file')


Do you have any comment to do regarding my exception handling? Did I forget one or more exceptions?

To go ahead, I have written this test function for pytest:

from pathlib import Path
import utils
#import pytest

def test_removeanything(tmp_path):
d = tmp_path / '.tmp_dir'
d.mkdir()
f = d / '.tmp_file'
f.touch()

# Test permission
d.chmod(mode=0o555)
utils.removeanything(f)
assert Path.exists(f)

# Test file deletion
d.chmod(mode=0o777)
utils.removeanything(f)
assert not Path.exists(f)

# Test folder deletion
utils.removeanything(d)
assert not Path.exists(d)



As I am pretty newbie at testing, I would like to know if I include all reasonable things to assert for my function? Are there other way to do it more properly? Is there a way to assert if expected error has been indeed raised by my function?

Thanks!

• Just a general comment: I wouldn't make a function to remove files or directories, it may be dangerous. You can separate your function in two (or pass a flag) and make sure you will remove a file or a directory separately. I'm telling you this because I already had this problem. I removed an entire directory with results that took days to complete just because I passed the wrong path to a function like this one. Of course this is up to you, others may disagree. Jan 8 '20 at 8:53
• you can check the tempfile module to make a temporary directory Jan 9 '20 at 8:56
• I have rolled back your latest edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Jan 9 '20 at 9:23

You can be more clear in the doc comment about what the function does:

• What is the input? (string, path)
• Does it return anything? (no)
• Does it throw exceptions? (yes, because os.remove and shutil.rmtree may throw)
• What is the state after calling the function? (file/folder has been removed, except if there was a permission error or another error)

Exceptions:

Someone who calls removeanything("myfile") may expect that myfile does not exist anymore after the function call. However, in case of a permission error, it still does exist. I think this is an exceptional situation, so I recommend that you do not catch the PermissionError and instead propagate it to the caller.

Output:

Currently, the function communicates via print statements. This means that callers of the function have no way of finding out what actually happened. You could add a return value that indicates whether a file, a directory, or nothing was removed. Then you may think about removing the print statement, or enabling/disabling it via function argument, because users may want to remove files silently.

Tests:

It may be useful to separate the single test test_removeanything into multiple tests test_removeanything_deletes_file, test_removeanything_deletes_directory, test_removeanything_handles_permission_error. This way, if a test fails, the test name gives you some more information about what went wrong.

Often, functions that remove directories require them to be non-empty. Therefore, it makes sense to test the removal of both empty and non-empty directories.

If you change removeanything so that the PermissionError propagates to the user, you can use pytest.raises to test whether the exception was raised correctly.

Misc:

I think the name removeanything can be more specific. After all, the function does not remove a CD from my CD drive ;)

• Very detailed answer! Thanks, I will update my code this way, it sounds relevant.
– FTG
Jan 8 '20 at 12:44

I have updated my code this way, using pytest.fixtures with temp file/folders provided by pytest.

from pathlib import Path
import os
import utils
import pytest

@pytest.fixture
def fd(tmp_path):
d = tmp_path / '.tmp_dir'
d.mkdir()
f = d / '.tmp_file'
f.touch()
return f,d

# Test permission
def test_permissions(fd):
with pytest.raises(PermissionError):
f,d = fd
d.chmod(mode=0o555)
utils.removeanything(f)

# Test file deletion
def test_delete_file(fd):
f,d = fd
utils.removeanything(f)
assert not Path.exists(f)

# Test non empty folder deletion
def test_delete_folder_nonempty(fd):
f,d = fd
utils.removeanything(d)
assert not Path.exists(d)

# Test empty folder deletion
def test_delete_folder_empty(fd):
f,d = fd