diff --git a/bin/filecheck.py b/bin/filecheck.py index 4642e75..540f043 100644 --- a/bin/filecheck.py +++ b/bin/filecheck.py @@ -365,8 +365,7 @@ class File(FileBase): self.make_dangerous('WinOffice file containing a macro') for i in indicators: if i.id == 'ObjectPool' and i.value: - # TODO: is having an ObjectPool suspicious? - self.add_description('WinOffice file containing an object pool') + self.make_dangerous('WinOffice file containing an object pool') elif i.id == 'flash' and i.value: self.make_dangerous('WinOffice file with embedded flash') self.add_description('WinOffice file') diff --git a/kittengroomer/helpers.py b/kittengroomer/helpers.py index 9b747de..a31abce 100644 --- a/kittengroomer/helpers.py +++ b/kittengroomer/helpers.py @@ -33,7 +33,7 @@ class FileBase(object): self.dst_dir = os.path.dirname(dst_path) self.filename = os.path.basename(src_path) self.size = self._get_size(src_path) - self.is_dangerous = False # Should this be some other value initially? + self.is_dangerous = False self.copied = False self.symlink_path = None self.description_string = [] # array of descriptions to be joined @@ -179,19 +179,17 @@ class FileBase(object): if dst is None: dst = self.dst_path try: - # TODO: maybe don't check and just mkdir? Is there a way to do this that won't raise an Exception? Can we make shutil automatically make dirs inbetween? - if not os.path.exists(self.dst_dir): - os.makedirs(self.dst_dir) + os.makedirs(self.dst_dir, exist_ok=True) shutil.copy(src, dst) - except Exception as e: - # TODO: what exceptions are we expecting to catch here? + except IOError as e: + # Probably means we can't write in the dest dir self.add_error(e, '') def force_ext(self, extension): """If dst_path does not end in `extension`, append .ext to it.""" new_ext = self._check_leading_dot(extension) if not self.filename.endswith(new_ext): - # TODO: do we want to log that the extension was changed? + # TODO: log that the extension was changed self.filename += new_ext if not self.get_property('extension') == new_ext: self.set_property('extension', new_ext) @@ -212,8 +210,7 @@ class FileBase(object): self.filename + "\": a file with that path exists.") else: - if not os.path.exists(self.dst_dir): - os.makedirs(self.dst_dir) + os.makedirs(self.dst_dir, exist_ok=True) # TODO: shouldn't mutate state and also return something self.metadata_file_path = self.dst_path + ext return self.metadata_file_path diff --git a/tests/test_kittengroomer.py b/tests/test_kittengroomer.py index 60cee71..1856022 100644 --- a/tests/test_kittengroomer.py +++ b/tests/test_kittengroomer.py @@ -237,7 +237,7 @@ class TestFileBase: assert text_file.extension == '.txt' assert '.txt.txt' not in text_file.dst_path - def test_safe_copy(self, src_dir_path, dest_dir_path): + def test_safe_copy_calls_copy(self, src_dir_path, dest_dir_path): """Calling safe_copy should copy the file from the correct path to the correct destination path.""" file_path = os.path.join(src_dir_path, 'test.txt') @@ -251,6 +251,16 @@ class TestFileBase: file.safe_copy() mock_copy.assert_called_once_with(file_path, dst_path) + def test_safe_copy_makedir_doesnt_exist(self): + """Calling safe_copy should create intermediate directories in the path + if they don't exist.""" + pass + + def test_safe_copy_makedir_exists(self): + """Calling safe_copy when some intermediate directories exist should + result in the creation of the full path and the file.""" + pass + def test_create_metadata_file_new(self): pass