Fix logging for errors and symlinks

bin/filecheck.py

@@ -586,32 +586,42 @@ class GroomerLogger(object):
             lf.write(b'\n')
 
     def add_file(self, file_path, file_props, in_tempdir=False):
-        """Add a file to the log. Takes a dict of file properties."""
+        """Add a file to the log. Takes a path and a dict of file properties."""
         depth = self._get_path_depth(file_path)
-        description_string = ', '.join(file_props['description_string'])
         file_hash = Logging.computehash(file_path)[:6]
-        if file_props['is_dangerous']:
+        if file_props['is_symlink']:
-            description_category = "Dangerous"
+            symlink_template = "+- NOT COPIED: symbolic link to {name} ({sha_hash})"
+            log_string = symlink_template.format(
+                name=file_props['symlink_path'],
+                sha_hash=file_hash
+            )
         else:
-            description_category = "Normal"
+            if file_props['is_dangerous']:
-        size_string = self._format_file_size(file_props['file_size'])
+                category = "Dangerous"
-        file_template = "+- {name} ({sha_hash}): {size}, type: {mt}/{st}. {desc}: {desc_str}"
+            else:
-        file_string = file_template.format(
+                category = "Normal"
-            name=file_props['filename'],
+            size_string = self._format_file_size(file_props['file_size'])
-            sha_hash=file_hash,
+            if not file_props['copied']:
-            size=size_string,
+                copied_string = 'NOT COPIED: '
-            mt=file_props['maintype'],
+            else:
-            st=file_props['subtype'],
+                copied_string = ''
-            desc=description_category,
+            file_template = "+- {copied}{name} ({sha_hash}): {size}, type: {mt}/{st}. {cat}: {desc_str}"
-            desc_str=description_string,
+            log_string = file_template.format(
-        )
+                copied=copied_string,
-        # TODO: finish adding Errors and check that they appear properly
+                name=file_props['filename'],
-        # if file_props['errors']:
+                sha_hash=file_hash,
-        #     error_string = ', '.join([str(key) for key in file_props['errors']])
+                size=size_string,
-        #     file_string.append(' Errors: ' + error_string)
+                mt=file_props['maintype'],
+                st=file_props['subtype'],
+                cat=category,
+                desc_str=file_props['description_string'],
+            )
+        if file_props['errors']:
+            error_string = ', '.join([str(key) for key in file_props['errors']])
+            log_string.append(' Errors: ' + error_string)
         if in_tempdir:
             depth -= 1
-        self._write_line_to_log(file_string, depth)
+        self._write_line_to_log(log_string, depth)
 
     def add_dir(self, dir_path):
         """Add a directory to the log"""
@@ -681,12 +691,13 @@ class KittenGroomerFileCheck(KittenGroomerBase):
         the file to the destionation key, and clean up temporary directory.
         """
         file.check()
-        if file.should_copy:
-            file.safe_copy()
-            file.set_property('copied', True)
-            file.write_log()
         if file.is_archive:
             self.process_archive(file)
+        else:
+            if file.should_copy:
+                file.safe_copy()
+                file.set_property('copied', True)
+            file.write_log()
         # TODO: Can probably handle cleaning up the tempdir better
         if hasattr(file, 'tempdir_path'):
             self.safe_rmtree(file.tempdir_path)

tests/test_filecheck_logging.py

@@ -12,12 +12,13 @@ def save_logs(groomer, test_description):
     test_log_path = 'tests/{}.log'.format(test_description)
     time_now = str(datetime.now().time()) + '\n'
     with open(test_log_path, 'wb+') as test_log:
-        log_header = divider.format('TEST LOG')
+        test_log_header = divider.format('TEST LOG')
-        test_log.write(bytes(log_header, encoding='utf-8'))
+        test_log.write(bytes(test_log_header, encoding='utf-8'))
         test_log.write(bytes(time_now, encoding='utf-8'))
         test_log.write(bytes(test_description, encoding='utf-8'))
         test_log.write(b'\n')
-        test_log.write(b'-' * 20 + b'\n')
+        log_header = divider.format('STD LOG')
+        test_log.write(bytes(log_header, encoding='utf-8'))
         with open(groomer.logger.log_path, 'rb') as logfile:
             log = logfile.read()
             test_log.write(log)

tests/test_kittengroomer.py

@@ -172,13 +172,18 @@ class TestFileBase:
     def test_add_new_description(self, text_file):
         """Adding a new description should add it to the list of description strings."""
         text_file.add_description('thing')
-        assert text_file.get_property('description_string') == ['thing']
+        assert text_file.get_property('description_string') == 'thing'
 
     def test_add_description_exists(self, text_file):
         """Adding a description that already exists shouldn't duplicate it."""
         text_file.add_description('thing')
         text_file.add_description('thing')
-        assert text_file.get_property('description_string') == ['thing']
+        assert text_file.get_property('description_string') == 'thing'
+
+    def test_add_multiple_descriptions(self, text_file):
+        text_file.add_description('thing')
+        text_file.add_description('foo')
+        assert text_file.get_property('description_string') == 'thing, foo'
 
     def test_add_description_not_string(self, text_file):
         """Adding a description that isn't a string should raise an error."""
@@ -205,7 +210,7 @@ class TestFileBase:
         """Marking a file as dangerous and passing in a description should add
         that description to the file."""
         text_file.make_dangerous('thing')
-        assert text_file.get_property('description_string') == ['thing']
+        assert text_file.get_property('description_string') == 'thing'
 
     def test_dangerous_file_mark_dangerous(self, text_file):
         """Marking a dangerous file as dangerous should do nothing, and the