Merge pull request #938 from jfunez/validator_fix_lista_de_erros

Fixes no Stylechecker validator

Author: gustavofonseca

scielomanager/articletrack/tests/tests_pages.py

@@ -296,15 +296,8 @@ def get_xml_uri(self, attempt_id, target_name):
         self.assertIsNone(xml_data['annotations'])
         self.assertEqual(xml_data['uri'], expected_response['uri'])
         expect_errors = {
-            'error_lines': '1',
-            'results': [
-                {
-                    'column': 6,
-                    'line': 1,
-                    'message': u'Premature end of data in tag xml line 1, line 1, column 6',
-                    'level': 'ERROR'
-                }
-            ]
+            'error_lines': '',
+            'results': []
         }
         self.assertEqual(xml_data['validation_errors'], expect_errors)
         self.assertEqual(xml_data['file_name'], expected_response['filename'])
@@ -427,17 +420,7 @@ def get_xml_uri(self, attempt_id, target_name):
         self.assertIsNone(xml_data['annotations'])
         self.assertEqual(xml_data['uri'], expected_response['uri'])
         self.assertEqual(xml_data['file_name'], expected_response['filename'])
-        expect_errors = {
-            'error_lines': '1',
-            'results': [
-                {
-                    'column': 6,
-                    'line': 1,
-                    'message': u'Premature end of data in tag xml line 1, line 1, column 6',
-                    'level': 'ERROR'
-                }
-            ]
-        }
+        expect_errors = {'error_lines': '', 'results': []}
         self.assertEqual(xml_data['validation_errors'], expect_errors)
 
     def test_annotations_of_syntax_error(self):

scielomanager/validator/templates/validator/includes/xml_annotated.html

@@ -34,25 +34,25 @@ <h4><i class="icon-minus-sign"></i> {% trans "The XML could not be analyzed" %}<
 {% else %}
   {# xml analyzed and with annotations #}
   <div class="alert alert-block alert-error">
-    <h4 class="alert-heading"> {% trans "The XML have some errors" %}:</h4>
+    <h4 class="alert-heading">{{ xml_data.validation_errors.results|length }} {% trans "error(s) found while checking this XML" %}:</h4>
   </div>
   {% if xml_data.validation_errors %}
     <table id="validation_errors_table" class="table table-striped table-condensed">
       <thead>
         <tr>
-          <th>{% trans "Level" %}:</th>
-          <th>{% trans "Line" %}:</th>
-          <th>{% trans "Column" %}:</th>
+          <th class="span1">{% trans "Level" %}:</th>
           <th>{% trans "Message" %}:</th>
+          <th class="span2">{% trans "Line" %}:</th>
+          <th class="span2">{% trans "Column" %}:</th>
         </tr>
       </thead>
       <tbody>
         {% for error in xml_data.validation_errors.results %}
           <tr>
             <td><span class="label label-{% trans_status error.level to_label='True' %}">{{ error.level|upper }}</span></td>
+            <td><a class="error_message" href="#" data-search-term="{{ error.message }}">{{ error.message }}</a></td>
             <td>{{ error.line|default:"--" }}</td>
             <td>{{ error.column|default:"--" }}</td>
-            <td><a class="error_message" href="#" data-search-term="{{ error.message }}">{{ error.message }}</a></td>
           </tr>
         {% endfor %}
       </tbody>

scielomanager/validator/tests/tests_pages.py

@@ -150,6 +150,9 @@ def test_submite_invalid_url_then_form_is_not_valid(self):
         self.assertFalse(hasattr(response.context, 'results'))
 
     def test_submit_valid_url_then_form_is_valid(self):
+        """
+        Submitting a valid url, will not raise a validation error of the form.
+        """
         # with
         self._addWaffleFlag()
         self._mocker_replace_stylechecker()
@@ -166,17 +169,7 @@ def test_submit_valid_url_then_form_is_valid(self):
         self.assertEqual(response.status_code, 200)
         self.assertTrue(response.context['form'].is_valid())
         expected_results = {
-            'validation_errors': {
-                'error_lines': '1',
-                'results': [
-                    {
-                        'column': 6,
-                        'level': 'ERROR',
-                        'line': 1,
-                        'message': u'Premature end of data in tag xml line 1, line 1, column 6'
-                    }
-                ]
-            },
+            'validation_errors': {'error_lines': '', 'results': []},
             'annotations': None,
             'can_be_analyzed': (True, None)
         }

scielomanager/validator/utils.py

@@ -7,9 +7,11 @@
 
 
 class ErrorCollection(object):
-    _errors = []
 
-    def add_object_error(self, error_obj=None, line='--', column='--', message='', level="ERROR"):
+    def __init__(self):
+        self._errors = []
+
+    def add_object_error(self, error_obj=None, line='--', column='--', message='', level="ERROR", allow_repeted=True):
         if error_obj:
             line = getattr(error_obj, 'line', line)
             column = getattr(error_obj, 'column', column)
@@ -22,24 +24,23 @@ def add_object_error(self, error_obj=None, line='--', column='--', message='', l
             'message': message,
             'level': level,
         }
-        if error_data not in self._errors:
+        if (error_data not in self._errors) or allow_repeted:
+            # if error was not included yet, let's append it.
+            # if error already included but allow_repeted == True, it's ok to append it again.
             self._errors.append(error_data)
 
-    def add_exception_error(self, exception_instance):
+    def add_exception_error(self, exception_instance, allow_repeted=True):
         message = exception_instance.message
         if hasattr(exception_instance, 'position'):
             line, column = exception_instance.position
         else:
             line, column = None, None
-        self.add_object_error(error_obj=None, line=line, column=column, message=message)
+        self.add_object_error(error_obj=None, line=line, column=column, message=message, allow_repeted=allow_repeted)
 
     def add_list_of_errors(self, iterable, allow_repeted=True):
-        for error in iterable:
-            if error not in self._errors:
-                self.add_object_error(error)
-            elif allow_repeted:
-                # error already exist in _errors list, but is added again
-                self.add_object_error(error)
+        if iterable:
+            for error in iterable:
+                self.add_object_error(error, allow_repeted=allow_repeted)
 
     def get_list(self):
         return self._errors
@@ -60,7 +61,6 @@ class StyleCheckerAnalyzer(object):
     _can_be_analyzed = (False, "Can't be analyzed")
     _can_be_analyzed_as_exception = False
     _annotations = None
-    _validation_errors = {'results': [], 'error_lines': [], }
     _error_collection = None
 
     def __init__(self, target_input):
@@ -77,12 +77,20 @@ def __init__(self, target_input):
             self._can_be_analyzed = (False, "IOError while starting Stylechecker.XML(), please verify if the input is correct")
         except Exception as e:
             self._can_be_analyzed = (False, "Error while starting Stylechecker.XML()")
+        self._validation_errors = {'results': [], 'error_lines': [], }
         self._error_collection = ErrorCollection()
 
 
     def get_validation_errors(self):
+        """
+            returns a dict like { 'results' : ... , 'error_lines': ''}
+            'results' is a dict with a structure necessary to display errors table (error level, line, cols, message)
+            'error_lines' is a coma separated string that list the un-repeated numbers of lines, where the XML has a error annotation
+            (this list is used to highlight the line to the user with a plugin).
+        """
         self._validation_errors['results'] = self._error_collection.get_list()
-        self._validation_errors['error_lines'] = ", ".join(self._error_collection.get_lines())
+        lines_distinct = [str(l) for l in set(self._error_collection.get_lines())]
+        self._validation_errors['error_lines'] = ", ".join(lines_distinct)
         return self._validation_errors
 
 
@@ -95,22 +103,24 @@ def analyze(self):
         if self._can_be_analyzed_as_exception:
             # in case of exceptions: self._target_data is the exception
             self._annotations = self._target_data.message
-            self._error_collection.add_exception_error(self._target_data)
+            self._error_collection.add_exception_error(self._target_data, allow_repeted=False)
             results['can_be_analyzed'] = (True, None)
         elif self._can_be_analyzed[0]:
             try:
-                # TODO: improve it!
                 vs_status, vs_errors = self._target_data.validate_style()
                 v_status, v_errors = self._target_data.validate()
             except Exception as e:
                 self._annotations = e.message
-                self._error_collection.add_exception_error(e)
+                self._error_collection.add_exception_error(e, allow_repeted=False)
                 results['can_be_analyzed'] = (True, None)
             else:
                 if not vs_status or not v_status:  # have errors
                     self._target_data.annotate_errors()
                     self._annotations = str(self._target_data)
+                if not vs_status:
                     self._error_collection.add_list_of_errors(vs_errors)
+                if not v_status:
+                    self._error_collection.add_list_of_errors(v_errors)
                 results['can_be_analyzed'] = (True, None)
         else:
             results['can_be_analyzed'] = self._can_be_analyzed