Improve error string handling. Make test pass on Py2 #280

Signed-off-by: Philippe Ombredanne <[email protected]>

Author: pombredanne

src/attributecode/__init__.py

@@ -40,16 +40,45 @@
 limitations under the License.
 """
 
-Error = namedtuple('Error', ['severity', 'message'])
+class Error(namedtuple('Error', ['severity', 'message'])):
+    """
+    An Error data with a severity and message.
+    """
+    def __new__(self, severity, message):
+        if message:
+            if isinstance(message, basestring):
+                message = clean_string(message)
+            else:
+                message = clean_string(repr(message))
 
-def error_repr(self):
-    sev = severities[self.severity]
-    msg = self.message
-    return 'Error(%(sev)s, %(msg)r)' % locals()
+        return super(Error, self).__new__(
+            Error, severity, message)
 
-Error.__repr__ = error_repr
+    def __repr__(self, *args, **kwargs):
+        sev = severities[self.severity]
+        msg = clean_string(repr(self.message))
+        return 'Error(%(sev)s, %(msg)s)' % locals()
 
 
+def clean_string(s):
+    """
+    Return a cleaned string for `s`, stripping eventual "u" prefixes
+    from unicode representations.
+    """
+    if not s:
+        return s
+    if s.startswith(('u"', "u'")):
+        msg = s.lstrip('u')
+    msg = s.replace('[u"', '["')
+    msg = s.replace("[u'", "['")
+    msg = s.replace("(u'", "('")
+    msg = s.replace("(u'", "('")
+    msg = s.replace("{u'", "{'")
+    msg = s.replace("{u'", "{'")
+    msg = s.replace(" u'", " '")
+    msg = s.replace(" u'", " '")
+    return msg
+
 # modeled after the logging levels
 CRITICAL = 50
 ERROR = 40

src/attributecode/attrib.py

@@ -166,11 +166,11 @@ def generate_and_save(abouts, output_location, mapping, template_loc=None,
                 about_list = attributecode.util.get_about_file_path(mapping, inventory_location)
             except Exception:
                 # 'about_file_path' key/column doesn't exist
-                msg = (u"The required key: 'about_file_path' does not exist. Generation halted.")
+                msg = u"The required key: 'about_file_path' does not exist. Generation halted."
                 errors.append(Error(ERROR, msg))
                 return errors
         else:
-            msg = (u'Only .csv and .json are supported for the "INVENTORY_LOCATOIN". Generation halted.')
+            msg = u'Only .csv and .json are supported for the "INVENTORY_LOCATOIN". Generation halted.'
             errors.append(Error(ERROR, msg))
             return errors
 
@@ -191,12 +191,12 @@ def generate_and_save(abouts, output_location, mapping, template_loc=None,
 
         if not_match_path:
             if len(not_match_path) == len(about_files_list):
-                msg = ("None of the paths in the provided 'inventory_location' match with the 'LOCATION'.")
+                msg = "None of the paths in the provided 'inventory_location' match with the 'LOCATION'."
                 errors.append(Error(ERROR, msg))
                 return errors
             else:
                 for path in not_match_path:
-                    msg = ('Path: ' + path + ' cannot be found.')
+                    msg = 'Path: ' + path + ' cannot be found.'
                     errors.append(Error(ERROR, msg))
 
         for about in abouts:

src/attributecode/gen.py

@@ -22,8 +22,12 @@
 from collections import OrderedDict
 import logging
 import posixpath
+import sys
 
-import unicodecsv
+if sys.version_info >= (3, 0):
+    from unicodecsv.py3 import UnicodeReader
+else:
+    from unicodecsv.py2 import UnicodeReader
 
 from attributecode import ERROR
 from attributecode import CRITICAL
@@ -51,7 +55,7 @@ def check_duplicated_columns(location):
     """
     location = add_unc(location)
     with codecs.open(location, 'rb', encoding='utf-8', errors='ignore') as csvfile:
-        reader = unicodecsv.UnicodeReader(csvfile)
+        reader = UnicodeReader(csvfile)
         columns = reader.next()
         columns = [col for col in columns]
 
@@ -120,7 +124,7 @@ def load_inventory(mapping, location, base_dir, license_notice_text_location=Non
             errors.extend(dup_about_paths_err)
             return errors, abouts
     except:
-        msg = ("The essential field 'about_file_path' is not found.")
+        msg = "The essential field 'about_file_path' is not found."
         errors.append(Error(CRITICAL, msg))
         return errors, abouts
 

src/attributecode/model.py

@@ -181,9 +181,10 @@ def serialize(self):
                     value = u'    '.join(value)
             else:
                 value = u''.join(value)
-            serialized = u'%(name)s: %(value)s' % locals()
-        else:
-            serialized = u'%(name)s:' % locals()
+
+        serialized = u'%(name)s:' % locals()
+        if value:
+            serialized += ' ' + '%(value)s' % locals()
         return serialized
 
     def serialized_value(self):
@@ -600,7 +601,7 @@ def has_content(self):
 
     def _serialized_value(self):
         # default normalized values for serialization
-        if self.value is True:
+        if self.value:
             return u'yes'
         elif self.value is False:
             return u'no'
@@ -921,7 +922,8 @@ def hydrate(self, fields):
                         errors.append(Error(INFO, msg % locals()))
         return errors
 
-    def process(self, fields, about_file_path, running_inventory=False, base_dir=None, license_notice_text_location=None):
+    def process(self, fields, about_file_path, running_inventory=False,
+                base_dir=None, license_notice_text_location=None):
         """
         Hydrate and validate a sequence of field name/value tuples from an
         ABOUT file. Return a list of errors.
@@ -935,15 +937,19 @@ def process(self, fields, about_file_path, running_inventory=False, base_dir=Non
 
         # We want to copy the license_files before the validation
         if license_notice_text_location:
-            copy_license_notice_files(fields, base_dir, license_notice_text_location, afp)
+            copy_license_notice_files(
+                fields, base_dir, license_notice_text_location, afp)
 
         # we validate all fields, not only these hydrated
         all_fields = self.all_fields()
-        validation_errors = validate_fields(all_fields, about_file_path, running_inventory, self.base_dir, self.license_notice_text_location)
+        validation_errors = validate_fields(
+            all_fields, about_file_path, running_inventory,
+            self.base_dir, self.license_notice_text_location)
         errors.extend(validation_errors)
 
-        # do not forget to resolve about resource paths
-        # The 'about_resource' field is now a ListField and those do not need to resolve
+        # do not forget to resolve about resource paths The
+        # 'about_resource' field is now a ListField and those do not
+        # need to resolve
         # self.about_resource.resolve(self.about_file_path)
         return errors
 

tests/test_util.py

@@ -212,13 +212,17 @@ def test_check_file_names_with_no_invalid_char_return_no_error(self):
         assert expected == result
 
     def test_check_file_names_with_invalid_chars_return_errors(self):
-        paths = ['locations/file',
-                 'locations/file with space',
-                 'locations/dir1/dir2/file1',
-                 'locations/dir2/file1']
+        paths = [
+            'locations/file',
+            'locations/file with space',
+            'locations/dir1/dir2/file1',
+            'locations/dir2/file1'
+        ]
 
         expected = [Error(CRITICAL, "Invalid characters '  ' in file name at: 'locations/file with space'")]
         result = util.check_file_names(paths)
+
+        assert expected[0].message == result[0].message
         assert expected == result
 
     def test_get_about_locations(self):