#492 - redefine logic for verbose and quiet

 * Previous code will store all the wanrning (INFO, NOTSET, DEBUG) even
the `verbose` flag is not set in the log file. We should only store
errors for whatever users define instead of sotring everything
 * Update tests and changelog

Signed-off-by: Chin Yeung Li <[email protected]>

Author: chinyeungli

CHANGELOG.rst

@@ -20,6 +20,7 @@
     * Bump PyYAML to 6.0
     * Add '%" as a supported character
     * Update default template
+    * All errors are logged if and only if the `verbose` option is set. Otherwise, ony 'Critical' and 'Warning' errors will be showed/logged
 
 2021-04-02
     Release 6.0.0

src/attributecode/cmd.py

@@ -810,41 +810,44 @@ def report_errors(errors, quiet, verbose, log_file_loc=None):
     file.
     Return True if there were severe error reported.
     """
-    messages, severe_errors_count = get_error_messages(errors, quiet, verbose)
-    for msg in messages:
-        click.echo(msg)
-    if log_file_loc and errors:
-        log_msgs, _ = get_error_messages(errors, quiet=False, verbose=True)
-        with io.open(log_file_loc, 'w', encoding='utf-8', errors='replace') as lf:
-            lf.write('\n'.join(log_msgs))
-        click.echo("Error log: " + log_file_loc)
+    severe_errors_count = 0
+    if errors:
+        log_msgs, severe_errors_count = get_error_messages(errors, verbose)
+        if not quiet:
+            for msg in log_msgs:
+                click.echo(msg)
+        if log_file_loc:
+            with io.open(log_file_loc, 'w', encoding='utf-8', errors='replace') as lf:
+                lf.write('\n'.join(log_msgs))
+            click.echo("Error log: " + log_file_loc)
     return severe_errors_count
 
 
-def get_error_messages(errors, quiet=False, verbose=False):
+def get_error_messages(errors, verbose=False):
     """
     Return a tuple of (list of error message strings to report,
     severe_errors_count) given an `errors` list of Error objects and using the
-    `quiet` and `verbose` flags.
+    `verbose` flags.
     """
-    errors = unique(errors)
-    severe_errors = filter_errors(errors, WARNING)
+    if verbose:
+        severe_errors = errors
+    else:
+        severe_errors = filter_errors(errors, WARNING)
+
+    severe_errors = unique(severe_errors)
     severe_errors_count = len(severe_errors)
 
     messages = []
 
-    if severe_errors and not quiet:
+    if severe_errors:
         error_msg = 'Command completed with {} errors or warnings.'.format(severe_errors_count)
         messages.append(error_msg)
 
-    for severity, message in errors:
+    for severity, message in severe_errors:
         sevcode = severities.get(severity) or 'UNKNOWN'
         msg = '{sevcode}: {message}'.format(**locals())
-        if not quiet:
-            if verbose:
-                messages.append(msg)
-            elif severity >= WARNING:
-                messages.append(msg)
+        messages.append(msg)
+
     return messages, severe_errors_count
 
 ######################################################################

src/attributecode/gen.py

@@ -176,6 +176,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
         errors.append(Error(CRITICAL, msg))
         return errors, abouts
 
+    custom_fields_list = []
     for fields in inventory:
         # check does the input contains the required fields
         required_fields = model.About.required_fields
@@ -220,15 +221,19 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
             running_inventory=False,
             reference_dir=reference_dir,
         )
-        """
-        # 'about_resource' field will be generated during the process.
-        # No error need to be raise for the missing 'about_resource'.
-        for e in ld_errors:
-            if e.message == 'Field about_resource is required':
-                ld_errors.remove(e)
-        """
-        errors.extend(ld_errors)
+
+        for severity, message in ld_errors:
+            if 'Custom Field' in message:
+                field_name = message.replace('Custom Field: ', '').strip()
+                if not field_name in custom_fields_list:
+                    custom_fields_list.append(field_name)
+            else:
+                errors.append(Error(severity, message))
+
         abouts.append(about)
+    if custom_fields_list:
+        custom_fields_err_msg = 'Field ' + str(custom_fields_list) + ' is a custom field.'
+        errors.append(Error(INFO, custom_fields_err_msg))
     # Covert the license_score value from string to list of int
     # The licesne_score is not in the spec but is specify in the scancode license scan.
     # This key will be treated as a custom string field. Therefore, we need to

src/attributecode/model.py

@@ -928,7 +928,7 @@ def hydrate(self, fields):
                 continue
             """
 
-            msg = 'Field %(orig_name)s is a custom field.'
+            msg = 'Custom Field: %(orig_name)s'
             errors.append(Error(INFO, msg % locals()))
             # is this a known one?
             custom_field = self.custom_fields.get(name)
@@ -969,6 +969,7 @@ def process(self, fields, about_file_path, running_inventory=False,
         afp = self.about_file_path
 
         errors = self.hydrate(fields)
+
         # We want to copy the license_files before the validation
         if reference_dir and not from_attrib:
             copy_err = copy_license_notice_files(
@@ -1341,10 +1342,9 @@ def collect_inventory(location):
     for about_loc in about_locations:
         about_file_path = util.get_relative_path(input_location, about_loc)
         about = About(about_loc, about_file_path)
-        # Insert about_file_path reference to the error
         for severity, message in about.errors:
-            if 'is a custom field' in message:
-                field_name = message.replace('Field', '').replace('is a custom field.', '').strip()
+            if 'Custom Field' in message:
+                field_name = message.replace('Custom Field: ', '').strip()
                 if not field_name in custom_fields_list:
                     custom_fields_list.append(field_name)
             else:

src/attributecode/util.py

@@ -574,7 +574,7 @@ def filter_errors(errors, minimum_severity=WARNING):
     Return a list of unique `errors` Error object filtering errors that have a
     severity below `minimum_severity`.
     """
-    return unique([e for e in errors if e.severity >= minimum_severity])
+    return [e for e in errors if e.severity >= minimum_severity]
 
 
 def create_dir(location):

tests/test_cmd.py

@@ -44,10 +44,10 @@ def test_report_errors(capsys):
         Error(NOTSET, 'msg4'),
     ]
     ec = cmd.report_errors(errors, quiet=False, verbose=True, log_file_loc=None)
-    assert 3 == ec
+    assert 6 == ec
     out, err = capsys.readouterr()
     expected_out = [
-        'Command completed with 3 errors or warnings.',
+        'Command completed with 6 errors or warnings.',
         'CRITICAL: msg1',
         'ERROR: msg2',
         'INFO: msg3',
@@ -91,7 +91,7 @@ def test_report_errors_with_quiet_ignores_verbose_flag(capsys):
         Error(WARNING, 'msg4'),
     ]
     severe_errors_count = cmd.report_errors(errors, quiet=True, verbose=True)
-    assert severe_errors_count == 3
+    assert severe_errors_count == 6
     out, err = capsys.readouterr()
     assert '' == out
     assert '' == err
@@ -125,17 +125,19 @@ def test_report_errors_with_verbose_flag(capsys):
         Error(WARNING, 'msg4'),
     ]
     severe_errors_count = cmd.report_errors(errors, quiet=False, verbose=True)
-    assert severe_errors_count == 3
+    assert severe_errors_count == 6
     out, err = capsys.readouterr()
     expected_out = [
-        'Command completed with 3 errors or warnings.',
+        'Command completed with 6 errors or warnings.',
         'CRITICAL: msg1',
         'ERROR: msg2',
         'INFO: msg3',
         'WARNING: msg4',
         'DEBUG: msg4',
         'NOTSET: msg4'
     ]
+    print("@@@@@@@@@@@@@@@@@@@@@@@@")
+    print(out.splitlines(False))
     assert expected_out == out.splitlines(False)
     assert '' == err
 
@@ -157,7 +159,7 @@ def test_report_errors_can_write_to_logfile():
     with io.open(result_file, 'r', encoding='utf-8', errors='replace') as rf:
         result = rf.read()
     expected = [
-        'Command completed with 3 errors or warnings.',
+        'Command completed with 6 errors or warnings.',
         'CRITICAL: msg1',
         'ERROR: msg2',
         'INFO: msg3',
@@ -181,7 +183,7 @@ def test_report_errors_does_not_report_duplicate_errors(capsys):
         Error(CRITICAL, 'msg1'),
     ]
     severe_errors_count = cmd.report_errors(errors, quiet=True, verbose=True)
-    assert severe_errors_count == 3
+    assert severe_errors_count == 6
 
 
 def test_get_error_messages():
@@ -205,22 +207,6 @@ def test_get_error_messages():
     assert expected == emsgs
 
 
-def test_get_error_messages_quiet():
-    errors = [
-        Error(CRITICAL, 'msg1'),
-        Error(ERROR, 'msg2'),
-        Error(INFO, 'msg3'),
-        Error(WARNING, 'msg4'),
-        Error(DEBUG, 'msg4'),
-        Error(NOTSET, 'msg4'),
-    ]
-
-    emsgs, ec = cmd.get_error_messages(errors, quiet=True)
-    assert 3 == ec
-    expected = []
-    assert expected == emsgs
-
-
 def test_get_error_messages_verbose():
     errors = [
         Error(CRITICAL, 'msg1'),
@@ -232,9 +218,9 @@ def test_get_error_messages_verbose():
     ]
 
     emsgs, ec = cmd.get_error_messages(errors, verbose=True)
-    assert 3 == ec
+    assert 6 == ec
     expected = [
-        'Command completed with 3 errors or warnings.',
+        'Command completed with 6 errors or warnings.',
         'CRITICAL: msg1',
         'ERROR: msg2',
         'INFO: msg3',

tests/test_gen.py

@@ -103,10 +103,9 @@ def test_load_inventory_with_errors(self):
         base_dir = get_temp_dir()
         errors, abouts = gen.load_inventory(location, base_dir=base_dir)
         expected_errors = [
-            Error(INFO, 'Field resource is a custom field.'),
-            Error(INFO, 'Field test is a custom field.'),
             Error(CRITICAL, "Field name: ['confirmed copyright'] contains illegal name characters: 0 to 9, a to z, A to Z and _. (or empty spaces) and is ignored."),
-            Error(INFO, 'Field about_resource: Path')
+            Error(INFO, 'Field about_resource: Path'),
+            Error(INFO, "Field ['resource', 'test'] is a custom field.")
         ]
 
         for exp, err in zip(expected_errors, errors):
@@ -210,11 +209,12 @@ def test_generate(self):
         base_dir = get_temp_dir()
 
         errors, abouts = gen.generate(location, base_dir)
-        msg1 = 'Field custom1 is a custom field.'
-        msg2 = 'Field about_resource'
+        err_msg_list = []
+        for severity, message in errors:
+            err_msg_list.append(message)
+        msg1 = "Field ['custom1'] is a custom field."
 
-        assert msg1 in errors[0].message
-        assert msg2 in errors[1].message
+        assert msg1 in err_msg_list
 
         result = [a.dumps() for a in abouts][0]
         expected = (

tests/test_model.py

@@ -430,9 +430,9 @@ def check_About_hydrate(self, about, fields):
             'about_resource'])
 
         expected_errors = [
-            Error(INFO, 'Field date is a custom field.'),
-            Error(INFO, 'Field license_spdx is a custom field.'),
-            Error(INFO, 'Field license_text_file is a custom field.')]
+            Error(INFO, 'Custom Field: date'),
+            Error(INFO, 'Custom Field: license_spdx'),
+            Error(INFO, 'Custom Field: license_text_file')]
 
         errors = about.hydrate(fields)
 
@@ -529,9 +529,10 @@ def test_About_has_errors_for_illegal_custom_field_name(self):
         test_file = get_test_loc('test_model/parse/illegal_custom_field.about')
         a = model.About(test_file)
         expected_errors = [
-            Error(INFO, 'Field hydrate is a custom field.'),
+            Error(INFO, 'Custom Field: hydrate'),
             Error(CRITICAL, "Internal error with custom field: 'hydrate': 'illegal name'.")
         ]
+
         assert expected_errors == a.errors
         assert not hasattr(getattr(a, 'hydrate'), 'value')
         field = list(a.custom_fields.values())[0]
@@ -753,8 +754,8 @@ def test_About_dumps_does_all_non_empty_present_fields(self):
         test_file = get_test_loc('test_model/parse/complete2/about.ABOUT')
         a = model.About(test_file)
         expected_error = [
-            Error(INFO, 'Field custom1 is a custom field.'),
-            Error(INFO, 'Field custom2 is a custom field.'),
+            Error(INFO, 'Custom Field: custom1'),
+            Error(INFO, 'Custom Field: custom2'),
             Error(INFO, 'Field custom2 is present but empty.')
         ]
         assert sorted(expected_error) == sorted(a.errors)
@@ -793,8 +794,8 @@ def test_About_dumps_all_non_empty_fields(self):
         test_file = get_test_loc('test_model/parse/complete2/about.ABOUT')
         a = model.About(test_file)
         expected_error = [
-            Error(INFO, 'Field custom1 is a custom field.'),
-            Error(INFO, 'Field custom2 is a custom field.'),
+            Error(INFO, 'Custom Field: custom1'),
+            Error(INFO, 'Custom Field: custom2'),
             Error(INFO, 'Field custom2 is present but empty.')
         ]
         assert sorted(expected_error) == sorted(a.errors)
@@ -842,11 +843,11 @@ def test_load_can_load_unicode(self):
         file_path = posixpath.join(posixpath.dirname(test_file), 'nose-selecttests-0.3.zip')
         err_msg = 'Field about_resource: Path %s not found' % file_path
         errors = [
-            Error(INFO, 'Field dje_license is a custom field.'),
-            Error(INFO, 'Field license_text_file is a custom field.'),
-            Error(INFO, 'Field scm_tool is a custom field.'),
-            Error(INFO, 'Field scm_repository is a custom field.'),
-            Error(INFO, 'Field test is a custom field.'),
+            Error(INFO, 'Custom Field: dje_license'),
+            Error(INFO, 'Custom Field: license_text_file'),
+            Error(INFO, 'Custom Field: scm_tool'),
+            Error(INFO, 'Custom Field: scm_repository'),
+            Error(INFO, 'Custom Field: test'),
             Error(INFO, err_msg)]
 
         assert errors == a.errors