[REM] core: assertion reports

That's a not-very-useful subset of OdooTestResult, so:

* make results merge-able (aka add ability to update a result with the
  contents of another)
* remove support for test data files, and transmission of the
  assertion report thing through the data-files loading
* replace "legitimate" uses of assertion report by test result
* have run_unit_tests manipulate and return a result instead of weird
  flags & ternaries

7 files changed, 44 insertions, 81 deletions

diff --git a/odoo/modules/loading.py b/odoo/modules/loading.py
index a3adfe8492b..dfe315c95d9 100644
--- a/odoo/modules/loading.py
+++ b/odoo/modules/loading.py
@@ -23,7 +23,7 @@ _logger = logging.getLogger(__name__)
 _test_logger = logging.getLogger('odoo.tests')
 
 
-def load_data(cr, idref, mode, kind, package, report):
+def load_data(cr, idref, mode, kind, package):
     """
 
     kind: data, demo, test, init_xml, update_xml, demo_xml.
@@ -66,14 +66,14 @@ def load_data(cr, idref, mode, kind, package, report):
             noupdate = False
             if kind in ('demo', 'demo_xml') or (filename.endswith('.csv') and kind in ('init', 'init_xml')):
                 noupdate = True
-            tools.convert_file(cr, package.name, filename, idref, mode, noupdate, kind, report)
+            tools.convert_file(cr, package.name, filename, idref, mode, noupdate, kind)
     finally:
         if kind in ('demo', 'test'):
             threading.currentThread().testing = False
 
     return bool(filename)
 
-def load_demo(cr, package, idref, mode, report=None):
+def load_demo(cr, package, idref, mode):
     """
     Loads demo data for the specified package.
     """
@@ -83,7 +83,7 @@ def load_demo(cr, package, idref, mode, report=None):
     try:
         _logger.info("Module %s: loading demo", package.name)
         with cr.savepoint(flush=False):
-            load_data(cr, idref, mode, kind='demo', package=package, report=report)
+            load_data(cr, idref, mode, kind='demo', package=package)
         return True
     except Exception as e:
         # If we could not install demo data for this module
@@ -129,20 +129,6 @@ def load_module_graph(cr, graph, status=None, perform_checks=True,
        :param skip_modules: optional list of module names (packages) which have previously been loaded and can be skipped
        :return: list of modules that were installed or updated
     """
-    def load_test(idref, mode):
-        cr.execute("SAVEPOINT load_test_data_file")
-        try:
-            # return `None` if `load_data` didn't load any file
-            return load_data(cr, idref, mode, 'test', package, report) or None
-        except Exception:
-            _test_logger.exception(
-                'module %s: an exception occurred in a test', package.name)
-            return False
-        finally:
-            cr.execute("ROLLBACK TO SAVEPOINT load_test_data_file")
-            # avoid keeping stale xml_id, etc. in cache
-            odoo.registry(cr.dbname).clear_caches()
-
     if models_to_check is None:
         models_to_check = set()
 
@@ -232,8 +218,8 @@ def load_module_graph(cr, graph, status=None, perform_checks=True,
             if package.state == 'to upgrade':
                 # upgrading the module information
                 module.write(module.get_values_from_terp(package.data))
-            load_data(cr, idref, mode, kind='data', package=package, report=report)
-            demo_loaded = package.dbdemo = load_demo(cr, package, idref, mode, report)
+            load_data(cr, idref, mode, kind='data', package=package)
+            demo_loaded = package.dbdemo = load_demo(cr, package, idref, mode)
             cr.execute('update ir_module_module set demo=%s where id=%s', (demo_loaded, module_id))
             module.invalidate_cache(['demo'])
 
@@ -266,10 +252,9 @@ def load_module_graph(cr, graph, status=None, perform_checks=True,
             env = api.Environment(cr, SUPERUSER_ID, {})
             if not needs_update:
                 registry.setup_models(cr)
-            report.record_result(load_test(idref, mode))
             # Python tests
             env['ir.http']._clear_routing_map()     # force routing map to be rebuilt
-            report.record_result(odoo.tests.loader.run_unit_tests(module_name))
+            report.update(odoo.tests.loader.run_unit_tests(module_name))
             # tests may have reset the environment
             env = api.Environment(cr, SUPERUSER_ID, {})
             module = env['ir.module.module'].browse(module_id)
@@ -540,10 +525,10 @@ def load_modules(db, force_demo=False, status=None, update_module=False):
                 except Exception as e:
                     _logger.warning('invalid custom view(s) for model %s: %s', model, tools.ustr(e))
 
-        if report.failures:
-            _logger.error('At least one test failed when loading the modules.')
-        else:
+        if report.wasSuccessful():
             _logger.info('Modules loaded.')
+        else:
+            _logger.error('At least one test failed when loading the modules.')
 
         # STEP 8: call _register_hook on every model
         env = api.Environment(cr, SUPERUSER_ID, {})
diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py
index 19b631fc4c3..1126b05e82a 100644
--- a/odoo/modules/registry.py
+++ b/odoo/modules/registry.py
@@ -19,7 +19,7 @@ import psycopg2
 import odoo
 from .. import SUPERUSER_ID
 from odoo.sql_db import TestCursor
-from odoo.tools import (assertion_report, config, existing_tables, ignore,
+from odoo.tools import (config, existing_tables, ignore,
                         lazy_classproperty, lazy_property, sql, OrderedSet)
 from odoo.tools.lru import LRU
 
@@ -110,7 +110,7 @@ class Registry(Mapping):
         self.models = {}    # model name/model instance mapping
         self._sql_constraints = set()
         self._init = True
-        self._assertion_report = assertion_report.assertion_report()
+        self._assertion_report = odoo.tests.runner.OdooTestResult()
         self._fields_by_model = None
         self._ordinary_tables = None
         self._constraint_queue = deque()
diff --git a/odoo/service/server.py b/odoo/service/server.py
index 3f9d7376901..f6e0ee72b00 100644
--- a/odoo/service/server.py
+++ b/odoo/service/server.py
@@ -514,10 +514,10 @@ class ThreadedServer(CommonServer):
                 with Registry.registries._lock:
                     for db, registry in Registry.registries.d.items():
                         report = registry._assertion_report
-                        log = _logger.error if report.failures \
-                         else _logger.warning if not report.successes \
+                        log = _logger.error if not report.wasSuccessful() \
+                         else _logger.warning if not report.testsRun \
                          else _logger.info
-                        log("%d / %d tests failed when loading %s", report.failures, report.successes + report.failures, db)
+                        log("%s when loading %s", report, db)
             self.stop()
             return rc
 
@@ -1179,10 +1179,8 @@ def load_test_file_py(registry, test_file):
                     suite = OdooSuite(tests)
                     _logger.log(logging.INFO, 'running tests %s.', mod_mod.__name__)
                     result = runner.OdooTestRunner().run(suite)
-                    success = result.wasSuccessful()
-                    if hasattr(registry._assertion_report,'report_result'):
-                        registry._assertion_report.report_result(success)
-                    if not success:
+                    registry._assertion_report.update(result)
+                    if not result.wasSuccessful():
                         _logger.error('%s: at least one error occurred in a test', test_file)
                     return
     finally:
@@ -1220,11 +1218,11 @@ def preload_registries(dbnames):
                 with odoo.api.Environment.manage():
                     for module_name in module_names:
                         result = loader.run_unit_tests(module_name, position='post_install')
-                        registry._assertion_report.record_result(result)
+                        registry._assertion_report.update(result)
                 _logger.info("All post-tested in %.2fs, %s queries",
                              time.time() - t0, odoo.sql_db.sql_counter - t0_sql)
 
-            if registry._assertion_report.failures:
+            if not registry._assertion_report.wasSuccessful():
                 rc += 1
         except Exception:
             _logger.critical('Failed to initialize database `%s`.', dbname, exc_info=True)
diff --git a/odoo/tests/loader.py b/odoo/tests/loader.py
index c2cc53452fa..99df113a82e 100644
--- a/odoo/tests/loader.py
+++ b/odoo/tests/loader.py
@@ -9,7 +9,7 @@ import unittest
 import odoo
 from .. import tools
 from .common import TagsSelector, OdooSuite
-from .runner import OdooTestRunner
+from .runner import OdooTestRunner, OdooTestResult
 
 # backwards compatibility
 _logger = logging.getLogger('odoo.modules.module')
@@ -67,7 +67,8 @@ def run_unit_tests(module_name, position='at_install'):
     threading.currentThread().testing = True
     config_tags = TagsSelector(tools.config['test_tags'])
     position_tag = TagsSelector(position)
-    ran_tests = failures = False
+
+    results = OdooTestResult()
     for m in mods:
         tests = unwrap_suite(unittest.TestLoader().loadTestsFromModule(m))
         suite = OdooSuite(t for t in tests if position_tag.check(t) and config_tags.check(t))
@@ -77,23 +78,18 @@ def run_unit_tests(module_name, position='at_install'):
             t0_sql = odoo.sql_db.sql_counter
             _logger.info('%s running tests.', m.__name__)
             result = OdooTestRunner().run(suite)
+            results.update(result)
             log_level = logging.INFO
             if time.time() - t0 > 5:
                 log_level = logging.RUNBOT
             _logger.log(log_level, "%s ran %s tests in %.2fs, %s queries", m.__name__, result.testsRun, time.time() - t0, odoo.sql_db.sql_counter - t0_sql)
-            ran_tests = True
             if not result.wasSuccessful():
-                failures = True
                 _logger.error("Module %s: %d failures, %d errors", module_name, len(result.failures), len(result.errors))
 
     module.current_test = None
     threading.currentThread().testing = False
 
-    if failures:
-        return False
-    if ran_tests:
-        return True
-    return None
+    return results
 
 
 def unwrap_suite(test):
diff --git a/odoo/tests/runner.py b/odoo/tests/runner.py
index 23e285005bd..132124fe031 100644
--- a/odoo/tests/runner.py
+++ b/odoo/tests/runner.py
@@ -10,6 +10,21 @@ class OdooTestResult(unittest.result.TestResult):
     but replacing the "findCaller" in order to give the information we
     have based on the test object that is running.
     """
+    def __str__(self):
+        return f'{len(self.failures)} failed, {len(self.errors)} errors of {self.testsRun} tests'
+
+    def update(self, other):
+        """ Merges an other test result into this one, only updates contents
+
+        :type other: OdooTestResult
+        """
+        self.failures.extend(other.failures)
+        self.errors.extend(other.errors)
+        self.testsRun += other.testsRun
+        self.skipped.extend(other.skipped)
+        self.expectedFailures.extend(other.expectedFailures)
+        self.unexpectedSuccesses.extend(other.unexpectedSuccesses)
+        self.shouldStop = self.shouldStop or other.shouldStop
 
     def log(self, level, msg, *args, test=None, exc_info=None, extra=None, stack_info=False, caller_infos=None):
         """
diff --git a/odoo/tools/assertion_report.py b/odoo/tools/assertion_report.py
deleted file mode 100644
index 638d2f3aeea..00000000000
--- a/odoo/tools/assertion_report.py
+++ /dev/null
@@ -1,28 +0,0 @@
-
-class assertion_report(object):
-    """
-    Simple pair of success and failures counts (used to record XML
-    `assert` tags as well as unittest tests outcome (in this case, not
-    individual `assert`)).
-    """
-    def __init__(self):
-        self.successes = 0
-        self.failures = 0
-
-    def record_success(self):
-        self.successes += 1
-
-    def record_failure(self):
-        self.failures += 1
-
-    def record_result(self, result):
-        if result is None:
-            pass
-        elif result is True:
-            self.record_success()
-        elif result is False:
-            self.record_failure()
-
-    def __str__(self):
-        res = 'Assertions report: %s successes, %s failures' % (self.successes, self.failures)
-        return res
diff --git a/odoo/tools/convert.py b/odoo/tools/convert.py
index 54b9b0d9ef4..94a954bbf73 100644
--- a/odoo/tools/convert.py
+++ b/odoo/tools/convert.py
@@ -21,7 +21,7 @@ except ImportError:
     jingtrang = None
 
 import odoo
-from . import assertion_report, pycompat
+from . import pycompat
 from .config import config
 from .misc import file_open, unquote, ustr, SKIPPED_ELEMENT_TYPES
 from .translate import _
@@ -691,14 +691,11 @@ form: module.record_id""" % (xml_id,)
     def noupdate(self):
         return self._noupdate[-1]
 
-    def __init__(self, cr, module, idref, mode, report=None, noupdate=False, xml_filename=None):
+    def __init__(self, cr, module, idref, mode, noupdate=False, xml_filename=None):
         self.mode = mode
         self.module = module
         self.envs = [odoo.api.Environment(cr, SUPERUSER_ID, {})]
         self.idref = {} if idref is None else idref
-        if report is None:
-            report = assertion_report.assertion_report()
-        self.assertion_report = report
         self._noupdate = [noupdate]
         self.xml_filename = xml_filename
         self._tags = {
@@ -718,7 +715,7 @@ form: module.record_id""" % (xml_id,)
         self._tag_root(de)
     DATA_ROOTS = ['odoo', 'data', 'openerp']
 
-def convert_file(cr, module, filename, idref, mode='update', noupdate=False, kind=None, report=None, pathname=None):
+def convert_file(cr, module, filename, idref, mode='update', noupdate=False, kind=None, pathname=None):
     if pathname is None:
         pathname = os.path.join(module, filename)
     ext = os.path.splitext(filename)[1].lower()
@@ -729,7 +726,7 @@ def convert_file(cr, module, filename, idref, mode='update', noupdate=False, kin
         elif ext == '.sql':
             convert_sql_import(cr, fp)
         elif ext == '.xml':
-            convert_xml_import(cr, module, fp, idref, mode, noupdate, report)
+            convert_xml_import(cr, module, fp, idref, mode, noupdate)
         elif ext == '.js':
             pass # .js files are valid but ignored here.
         else:
@@ -794,5 +791,5 @@ def convert_xml_import(cr, module, xmlfile, idref=None, mode='init', noupdate=Fa
         xml_filename = xmlfile
     else:
         xml_filename = xmlfile.name
-    obj = xml_import(cr, module, idref, mode, report=report, noupdate=noupdate, xml_filename=xml_filename)
+    obj = xml_import(cr, module, idref, mode, noupdate=noupdate, xml_filename=xml_filename)
     obj.parse(doc.getroot())