Fixups for PR #16

* ObjectPool winoffice files are now make_dangerous
* safe_copy now catches IOErrors only
* Use os.makedirs(exist_ok=True) instead of checking for existence in safe_copy
and create_metadata_file
* Added stubs for two tests related to safe_copy

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.make_dangerous('WinOffice file containing an object pool')
-                    self.add_description('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')

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?
+            os.makedirs(self.dst_dir, exist_ok=True)
-            if not os.path.exists(self.dst_dir):
-                os.makedirs(self.dst_dir)
             shutil.copy(src, dst)
-        except Exception as e:
+        except IOError as e:
-            # TODO: what exceptions are we expecting to catch here?
+            # 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, exist_ok=True)
-                    os.makedirs(self.dst_dir)
                 # TODO: shouldn't mutate state and also return something
                 self.metadata_file_path = self.dst_path + ext
                 return self.metadata_file_path

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