Log the actual exception that caused a ParseError, reduce code duplic…

…ation parsing XMl files

python/publish/github_action.py

@@ -62,8 +62,11 @@ def warning(self, message: str, file: Optional[str] = None, line: Optional[int]
             params.update(col=column)
         self._command(self._file, 'warning', message, params)
 
-    def error(self, message: str, file: Optional[str] = None, line: Optional[int] = None, column: Optional[int] = None):
-        logger.error(message)
+    def error(self,
+              message: str,
+              file: Optional[str] = None, line: Optional[int] = None, column: Optional[int] = None,
+              exception: Optional[BaseException] = None):
+        logger.error(message, exc_info=exception)
 
         params = {}
         if file is not None:

python/publish/junit.py

@@ -120,50 +120,61 @@ def close(self) -> Element:
 
 
 JUnitTree = etree.ElementTree
-JUnitTreeOrException = Union[JUnitTree, BaseException]
-ParsedJUnitFile = Tuple[str, JUnitTreeOrException]
+JUnitTreeOrParseError = Union[JUnitTree, ParseError]
+JUnitXmlOrParseError = Union[JUnitXml, ParseError]
+ParsedJUnitFile = Tuple[str, JUnitTreeOrParseError]
+
+
+def safe_parse_xml_file(path: str, parse: Callable[[str], JUnitTree]) -> JUnitTreeOrParseError:
+    """Parses an xml file and returns either a JUnitTree or a ParseError."""
+    if not os.path.exists(path):
+        return ParseError.from_exception(path, FileNotFoundError(f'File does not exist.'))
+    if os.stat(path).st_size == 0:
+        return ParseError.from_exception(path, Exception(f'File is empty.'))
+
+    try:
+        return parse(path)
+    except BaseException as e:
+        return ParseError.from_exception(path, e)
+
+
+def progress_safe_parse_xml_file(files: Iterable[str],
+                            parse: Callable[[str], JUnitTree],
+                            progress: Callable[[ParsedJUnitFile], ParsedJUnitFile]) -> Iterable[ParsedJUnitFile]:
+    return [progress((file, safe_parse_xml_file(file, parse))) for file in files]
 
 
 def parse_junit_xml_files(files: Iterable[str],
                           drop_testcases: bool = False,
                           progress: Callable[[ParsedJUnitFile], ParsedJUnitFile] = lambda x: x) -> Iterable[ParsedJUnitFile]:
     """Parses junit xml files."""
-    def parse(path: str) -> JUnitTreeOrException:
-        """Parses a junit xml file and returns either a JUnitTree or an Exception."""
-        if not os.path.exists(path):
-            return FileNotFoundError(f'File does not exist.')
-        if os.stat(path).st_size == 0:
-            return Exception(f'File is empty.')
-
-        try:
-            if drop_testcases:
-                builder = DropTestCaseBuilder()
-                parser = etree.XMLParser(target=builder, encoding='utf-8', huge_tree=True)
-                return etree.parse(path, parser=parser)
-            return etree.parse(path)
-        except BaseException as e:
-            return e
+    def parse(path: str) -> JUnitTree:
+        if drop_testcases:
+            builder = DropTestCaseBuilder()
+            parser = etree.XMLParser(target=builder, encoding='utf-8', huge_tree=True)
+            return etree.parse(path, parser=parser)
+        return etree.parse(path)
 
-    return [progress((result_file, parse(result_file))) for result_file in files]
+    return progress_safe_parse_xml_file(files, parse, progress)
 
 
 def process_junit_xml_elems(trees: Iterable[ParsedJUnitFile], time_factor: float = 1.0) -> ParsedUnitTestResults:
-    def create_junitxml(filepath: str, tree: JUnitTree) -> Union[JUnitXml, JUnitXmlError]:
+    def create_junitxml(filepath: str, tree: JUnitTree) -> JUnitXmlOrParseError:
         try:
             instance = JUnitXml.fromroot(tree.getroot())
             instance.filepath = filepath
             return instance
         except JUnitXmlError as e:
-            return e
+            return ParseError.from_exception(filepath, e)
 
-    processed = [(result_file, create_junitxml(result_file, tree) if not isinstance(tree, BaseException) else tree)
+    processed = [(result_file, create_junitxml(result_file, tree) if not isinstance(tree, ParseError) else tree)
                   for result_file, tree in trees]
     junits = [(result_file, junit)
               for result_file, junit in processed
-              if not isinstance(junit, BaseException)]
-    errors = [ParseError.from_exception(result_file, exception)
-              for result_file, exception in processed
-              if isinstance(exception, BaseException)]
+              if not isinstance(junit, ParseError)]
+    errors = [error
+              for _, error in processed
+              if isinstance(error, ParseError)]
 
     suites = [(result_file, suite)
               for result_file, junit in junits

python/publish/nunit.py

@@ -1,29 +1,19 @@
-import os
 import pathlib
 from typing import Iterable, Callable
 
 from lxml import etree
 
-from publish.junit import JUnitTreeOrException, ParsedJUnitFile
+from publish.junit import JUnitTree, ParsedJUnitFile, progress_safe_parse_xml_file
 
-with (pathlib.Path(__file__).parent / 'xslt' / 'nunit3-to-junit.xslt').open('r', encoding='utf-8') as r:
+with (pathlib.Path(__file__).resolve().parent / 'xslt' / 'nunit3-to-junit.xslt').open('r', encoding='utf-8') as r:
     transform_nunit_to_junit = etree.XSLT(etree.parse(r))
 
 
 def parse_nunit_files(files: Iterable[str],
                       progress: Callable[[ParsedJUnitFile], ParsedJUnitFile] = lambda x: x) -> Iterable[ParsedJUnitFile]:
     """Parses nunit files."""
-    def parse(path: str) -> JUnitTreeOrException:
-        """Parses an nunit file and returns either a JUnitTree or an Exception."""
-        if not os.path.exists(path):
-            return FileNotFoundError(f'File does not exist.')
-        if os.stat(path).st_size == 0:
-            return Exception(f'File is empty.')
+    def parse(path: str) -> JUnitTree:
+        nunit = etree.parse(path)
+        return transform_nunit_to_junit(nunit)
 
-        try:
-            nunit = etree.parse(path)
-            return transform_nunit_to_junit(nunit)
-        except BaseException as e:
-            return e
-
-    return [progress((result_file, parse(result_file))) for result_file in files]
+    return progress_safe_parse_xml_file(files, parse, progress)

python/publish/trx.py

@@ -1,29 +1,19 @@
-import os
 import pathlib
 from typing import Iterable, Callable
 
 from lxml import etree
 
-from publish.junit import JUnitTreeOrException, ParsedJUnitFile
+from publish.junit import JUnitTree, ParsedJUnitFile, progress_safe_parse_xml_file
 
-with (pathlib.Path(__file__).parent / 'xslt' / 'trx-to-junit.xslt').open('r', encoding='utf-8') as r:
+with (pathlib.Path(__file__).resolve().parent / 'xslt' / 'trx-to-junit.xslt').open('r', encoding='utf-8') as r:
     transform_trx_to_junit = etree.XSLT(etree.parse(r))
 
 
 def parse_trx_files(files: Iterable[str],
                     progress: Callable[[ParsedJUnitFile], ParsedJUnitFile] = lambda x: x) -> Iterable[ParsedJUnitFile]:
     """Parses trx files."""
-    def parse(path: str) -> JUnitTreeOrException:
-        """Parses a trx file and returns either a JUnitTree or an Exception."""
-        if not os.path.exists(path):
-            return FileNotFoundError(f'File does not exist.')
-        if os.stat(path).st_size == 0:
-            return Exception(f'File is empty.')
+    def parse(path: str) -> JUnitTree:
+        trx = etree.parse(path)
+        return transform_trx_to_junit(trx)
 
-        try:
-            trx = etree.parse(path)
-            return transform_trx_to_junit(trx)
-        except BaseException as e:
-            return e
-
-    return [progress((result_file, parse(result_file))) for result_file in files]
+    return progress_safe_parse_xml_file(files, parse, progress)

python/publish/unittestresults.py

@@ -1,5 +1,5 @@
-from collections import defaultdict
 import dataclasses
+from collections import defaultdict
 from dataclasses import dataclass
 from typing import Optional, List, Mapping, Any, Union, Dict, Callable
 from xml.etree.ElementTree import ParseError as XmlParseError
@@ -31,6 +31,7 @@ class ParseError:
     message: str
     line: Optional[int] = None
     column: Optional[int] = None
+    exception: Optional[BaseException] = None
 
     @staticmethod
     def from_exception(file: str, exception: BaseException):
@@ -44,8 +45,8 @@ def from_exception(file: str, exception: BaseException):
                 msg = f'File is not a valid XML file:\n{msg}'
             elif msg.startswith('Invalid format.'):
                 msg = f'File is not a valid JUnit file:\n{msg}'
-            return ParseError(file=file, message=msg, line=line, column=column)
-        return ParseError(file=file, message=str(exception))
+            return ParseError(file=file, message=msg, line=line, column=column, exception=exception)
+        return ParseError(file=file, message=str(exception), exception=exception)
 
 
 @dataclass(frozen=True)

python/publish/xunit.py

@@ -1,29 +1,19 @@
-import os
 import pathlib
 from typing import Iterable, Callable
 
 from lxml import etree
 
-from publish.junit import JUnitTreeOrException, ParsedJUnitFile
+from publish.junit import JUnitTree, ParsedJUnitFile, progress_safe_parse_xml_file
 
-with (pathlib.Path(__file__).parent / 'xslt' / 'xunit-to-junit.xslt').open('r', encoding='utf-8') as r:
+with (pathlib.Path(__file__).resolve().parent / 'xslt' / 'xunit-to-junit.xslt').open('r', encoding='utf-8') as r:
     transform_xunit_to_junit = etree.XSLT(etree.parse(r))
 
 
 def parse_xunit_files(files: Iterable[str],
                       progress: Callable[[ParsedJUnitFile], ParsedJUnitFile] = lambda x: x) -> Iterable[ParsedJUnitFile]:
     """Parses xunit files."""
-    def parse(path: str) -> JUnitTreeOrException:
-        """Parses an xunit file and returns either a JUnitTree or an Exception."""
-        if not os.path.exists(path):
-            return FileNotFoundError(f'File does not exist.')
-        if os.stat(path).st_size == 0:
-            return Exception(f'File is empty.')
+    def parse(path: str) -> JUnitTree:
+        xunit = etree.parse(path)
+        return transform_xunit_to_junit(xunit)
 
-        try:
-            xunit = etree.parse(path)
-            return transform_xunit_to_junit(xunit)
-        except BaseException as e:
-            return e
-
-    return [progress((result_file, parse(result_file))) for result_file in files]
+    return progress_safe_parse_xml_file(files, parse, progress)

python/publish_unit_test_results.py

@@ -144,7 +144,7 @@ def main(settings: Settings, gha: GithubAction) -> None:
 
     # get the unit test results
     parsed = parse_files(settings, gha)
-    [gha.error(message=f'Error processing result file: {error.message}', file=error.file, line=error.line, column=error.column)
+    [gha.error(message=f'Error processing result file: {error.message}', file=error.file, line=error.line, column=error.column, exception=error.exception)
      for error in parsed.errors]
 
     # process the parsed results

python/test/files/junit-xml/empty.exception

@@ -1 +1 @@
-Exception: 'File is empty.'
+ParseError: file='files/junit-xml/empty.xml', message='File is empty.', line=None, column=None, exception=Exception('File is empty.')

python/test/files/junit-xml/non-junit.results

@@ -3,7 +3,8 @@ publish.unittestresults.ParsedUnitTestResults(
   errors=[
     publish.unittestresults.ParseError(
       file='non-junit.xml',
-      message='Invalid format.'
+      message='Invalid format.',
+      exception=junitparser.junitparser.JUnitXmlError('Invalid format.')
     )
   ],
   suites=0,

python/test/files/junit-xml/non-xml.exception

@@ -1 +1 @@
-XMLSyntaxError: "Start tag expected, '<' not found, line 1, column 1"
+ParseError: file='files/junit-xml/non-xml.xml', message="Start tag expected, '<' not found, line 1, column 1 (non-xml.xml, line 1)", line=None, column=None, exception=XMLSyntaxError("Start tag expected, '<' not found, line 1, column 1")

python/test/files/junit-xml/pytest/corrupt-xml.exception

@@ -1 +1 @@
-XMLSyntaxError: 'Premature end of data in tag skipped line 9, line 11, column 22'
+ParseError: file='files/junit-xml/pytest/corrupt-xml.xml', message='Premature end of data in tag skipped line 9, line 11, column 22 (corrupt-xml.xml, line 11)', line=None, column=None, exception=XMLSyntaxError('Premature end of data in tag skipped line 9, line 11, column 22')

python/test/files/nunit/nunit3/jenkins/NUnit-issue17521.exception

@@ -1 +1 @@
-XMLSyntaxError: 'Char 0x0 out of allowed range, line 33, column 16'
+ParseError: file='files/nunit/nunit3/jenkins/NUnit-issue17521.xml', message='Char 0x0 out of allowed range, line 33, column 16 (NUnit-issue17521.xml, line 33)', line=None, column=None, exception=XMLSyntaxError('Char 0x0 out of allowed range, line 33, column 16')

python/test/files/nunit/nunit3/jenkins/NUnit-issue47367.exception

@@ -1 +1 @@
-XMLSyntaxError: 'attributes construct error, line 5, column 109'
+ParseError: file='files/nunit/nunit3/jenkins/NUnit-issue47367.xml', message='attributes construct error, line 5, column 109 (NUnit-issue47367.xml, line 5)', line=None, column=None, exception=XMLSyntaxError('attributes construct error, line 5, column 109')

python/test/test_action_script.py

@@ -18,7 +18,7 @@
     get_settings, get_annotations_config, Settings, get_files, throttle_gh_request_raw, is_float, parse_files, main
 from test_utils import chdir
 
-test_files_path = pathlib.Path(__file__).parent / 'files'
+test_files_path = pathlib.Path(__file__).resolve().parent / 'files'
 
 event = dict(pull_request=dict(head=dict(sha='event_sha')))
 

python/test/test_action_yml.py

@@ -3,7 +3,7 @@
 
 import yaml
 
-project_root = pathlib.Path(__file__).parent.parent.parent
+project_root = pathlib.Path(__file__).resolve().parent.parent.parent
 
 
 class TestActionYml(unittest.TestCase):

python/test/test_cicd_yml.py

@@ -3,7 +3,7 @@
 
 import yaml
 
-project_root = pathlib.Path(__file__).parent.parent.parent
+project_root = pathlib.Path(__file__).resolve().parent.parent.parent
 
 
 class TestActionYml(unittest.TestCase):

python/test/test_junit.py

@@ -1,10 +1,11 @@
+import dataclasses
 import pathlib
 import re
 import sys
 import unittest
 from distutils.version import LooseVersion
 from glob import glob
-from typing import Optional, Union, List
+from typing import Optional, List
 
 import junitparser
 import prettyprinter as pp
@@ -15,11 +16,12 @@
 sys.path.append(str(pathlib.Path(__file__).resolve().parent))
 
 from publish.junit import parse_junit_xml_files, process_junit_xml_elems, get_results, get_result, get_content, \
-    get_message, Disabled, JUnitTree
+    get_message, Disabled, JUnitTreeOrParseError, ParseError
 from publish.unittestresults import ParsedUnitTestResults, UnitTestCase
 from test_utils import temp_locale
 
-test_files_path = pathlib.Path(__file__).parent / 'files' / 'junit-xml'
+test_path = pathlib.Path(__file__).resolve().parent
+test_files_path = test_path / 'files' / 'junit-xml'
 pp.install_extras()
 
 
@@ -51,7 +53,7 @@ def get_test_files() -> List[str]:
         raise NotImplementedError()
 
     @staticmethod
-    def parse_file(filename) -> Union[JUnitTree, BaseException]:
+    def parse_file(filename) -> JUnitTreeOrParseError:
         raise NotImplementedError()
 
     @staticmethod
@@ -70,9 +72,11 @@ def do_test_parse_and_process_files(self, filename: str):
                 with temp_locale(locale):
                     actual = self.parse_file(filename)
                     path = pathlib.Path(filename)
-                    if isinstance(actual, BaseException):
-                        expectation_path = path.parent / (path.stem + '.exception')
+                    if isinstance(actual, ParseError):
+                        # make file relative so the path in the exception file does not depend on where we checkout the sources
+                        actual = dataclasses.replace(actual, file=str(pathlib.Path(actual.file).relative_to(test_path).as_posix()))
                         actual = self.prettify_exception(actual)
+                        expectation_path = path.parent / (path.stem + '.exception')
                         self.assert_expectation(self.test, actual, expectation_path)
                     else:
                         xml_expectation_path = path.parent / (path.stem + '.junit-xml')
@@ -93,8 +97,10 @@ def update_expectations(cls):
         for filename in cls.get_test_files():
             print(f'- updating {filename}')
             actual = cls.parse_file(filename)
-            path = pathlib.Path(filename)
-            if isinstance(actual, BaseException):
+            path = pathlib.Path(filename).resolve()
+            if isinstance(actual, ParseError):
+                # make file relative so the path in the exception file does not depend on where we checkout the sources
+                actual = dataclasses.replace(actual, file=str(pathlib.Path(actual.file).relative_to(test_path).as_posix()))
                 with open(path.parent / (path.stem + '.exception'), 'w', encoding='utf-8') as w:
                     w.write(cls.prettify_exception(actual))
             else:
@@ -129,7 +135,7 @@ def get_test_files() -> List[str]:
         return glob(str(test_files_path / '**' / '*.xml'), recursive=True)
 
     @staticmethod
-    def parse_file(filename) -> Union[JUnitTree, BaseException]:
+    def parse_file(filename) -> JUnitTreeOrParseError:
         return list(parse_junit_xml_files([filename]))[0][1]
 
     def test_process_parse_junit_xml_files_with_no_files(self):