[IMP] testing: forbid module operations during testing

With this commit, module operations such as install, upgrades and
uninstalls are henceforth forbidden inside unit tests and instead such
tests must be performed with standalone Odoo scripts.

This is done because module operations during tests are not
transactional, this can leave the registry in an unclean state and
further tests may be affected by this, it also creates a new registry
which complicates registry cleanup if anything crashes,
because the registry to be cleaned up is not the same one that crashed.

Instead, what should be done is a script that imports odoo as a library,
and loads the database necessary then performs whichever operations
necessary. This script should contain a single function with a single
parameter (env) and should be decorated with
@odoo.tests.common.standalone in order to be executed properly, this
decorator accepts any amount of positional parameters as tags that can
be specified when calling the script in order to execute only a select
subset of scripts.

Special tags are: 'all' and <module_name>, these are generated
automatically, the first will execute ALL scripts available whereas
<module_name> will execute all scripts introduced by said module.

When calling the test_module_operations script, only scripts found in
*installed* modules will be executed, script discovery is only possible
if the code is loaded therefore it is only possible if the module is
installed.

closes odoo/odoo#49669

Signed-off-by: Adrian Torres (adt) <adt@odoo.com>

5 files changed, 165 insertions, 108 deletions

diff --git a/addons/account/tests/test_account_all_l10n.py b/addons/account/tests/test_account_all_l10n.py
index 533dba0f414..fbf9887bdd1 100644
--- a/addons/account/tests/test_account_all_l10n.py
+++ b/addons/account/tests/test_account_all_l10n.py
@@ -1,53 +1,35 @@
 # -*- coding: utf-8 -*-
 import logging
 
-import odoo
-from odoo import api
-from odoo.tests.common import SingleTransactionCase
-from odoo.tests import tagged
+from odoo.tests import standalone
 
 
 _logger = logging.getLogger(__name__)
 
 
-@tagged('-standard', '-at_install', 'post_install', 'l10nall')
-class TestAllL10n(SingleTransactionCase):
+@standalone('all_l10n')
+def test_all_l10n(env):
     """ This test will install all the l10n_* modules.
     As the module install is not yet fully transactional, the modules will
     remain installed after the test.
     """
-
-    @classmethod
-    def setUpClass(cls):
-        super(TestAllL10n, cls).setUpClass()
-
-        # the installation of l10n_* modules below instanciates a NEW registry, so we
-        # have to restore the old one in order to perform the cleanups
-        @cls.addClassCleanup
-        def cleanup():
-            cls.registry.registries[cls.registry.db_name] = cls.registry
-
-        l10n_mods = cls.env['ir.module.module'].search([
-            ('name', 'like', 'l10n%'),
-            ('state', '=', 'uninstalled'),
-        ])
-        _logger.info("Modules to install: %s" % [x.name for x in l10n_mods])
-        l10n_mods.button_immediate_install()
-        # Now that new modules are installed, we have to reset the environment
-        api.Environment.reset()
-        cls.env = api.Environment(cls.cr, odoo.SUPERUSER_ID, {})
-
-    def test_all_l10n(self):
-        coas = self.env['account.chart.template'].search([])
-        for coa in coas:
-            cname = 'company_%s' % str(coa.id)
-            comp = self.env['res.company'].create({
-                'name': cname,
-            })
-            self.env.user.company_ids += comp
-            self.env.user.company_id = comp
-            msg = 'Testing COA: %s (company: %s)' % (coa.name, cname)
-            _logger.info(msg)
-            with self.subTest(msg=msg):
-                with self.cr.savepoint():
-                    coa.try_loading()
+    l10n_mods = env['ir.module.module'].search([
+        ('name', 'like', 'l10n%'),
+        ('state', '=', 'uninstalled'),
+    ])
+    l10n_mods.button_immediate_install()
+    env.reset()     # clear the set of environments
+    env = env()     # get an environment that refers to the new registry
+
+    coas = env['account.chart.template'].search([])
+    for coa in coas:
+        cname = 'company_%s' % str(coa.id)
+        company = env['res.company'].create({'name': cname})
+        env.user.company_ids += company
+        env.user.company_id = company
+        _logger.info('Testing COA: %s (company: %s)' % (coa.name, cname))
+        try:
+            with env.cr.savepoint():
+                coa.try_loading()
+        except Exception:
+            _logger.error("Error when creating COA %s", coa.name, exc_info=True)
diff --git a/addons/test_website/tests/test_views_during_module_operation.py b/addons/test_website/tests/test_views_during_module_operation.py
index e960291bf22..c078afc03fd 100644
--- a/addons/test_website/tests/test_views_during_module_operation.py
+++ b/addons/test_website/tests/test_views_during_module_operation.py
@@ -1,78 +1,83 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo.tests import TransactionCase, tagged
+from odoo.tests import standalone
 
 
-@tagged('-at_install', 'post_install')
-class TestViewsDuringModuleOperation(TransactionCase):
-    def test_01_cow_views_unlink_on_module_update(self):
-        """ Ensure COW views are correctly removed during module update.
-        Not removing the view could lead to traceback:
-        - Having a view A
-        - Having a view B that inherits from a view C
-        - View B t-call view A
-        - COW view B
-        - Delete view A and B from module datas and update it
-        - Rendering view C will crash since it will render child view B that
-          t-call unexisting view A
-        """
+@standalone('cow_views')
+def test_01_cow_views_unlink_on_module_update(env):
+    """ Ensure COW views are correctly removed during module update.
+    Not removing the view could lead to traceback:
+    - Having a view A
+    - Having a view B that inherits from a view C
+    - View B t-call view A
+    - COW view B
+    - Delete view A and B from module datas and update it
+    - Rendering view C will crash since it will render child view B that
+      t-call unexisting view A
+    """
 
-        View = self.env['ir.ui.view']
-        Imd = self.env['ir.model.data']
+    View = env['ir.ui.view']
+    Imd = env['ir.model.data']
 
-        update_module_base_view = self.env.ref('test_website.update_module_base_view')
-        update_module_view_to_be_t_called = View.create({
-            'name': 'View to be t-called',
-            'type': 'qweb',
-            'arch': '<div>I will be t-called</div>',
-            'key': 'test_website.update_module_view_to_be_t_called',
-        })
-        update_module_child_view = View.create({
-            'name': 'Child View',
-            'mode': 'extension',
-            'inherit_id': update_module_base_view.id,
-            'arch': '''
-                <div position="inside">
-                    <t t-call="test_website.update_module_view_to_be_t_called"/>
-                </div>
-            ''',
-            'key': 'test_website.update_module_child_view',
-        })
+    update_module_base_view = env.ref('test_website.update_module_base_view')
+    update_module_view_to_be_t_called = View.create({
+        'name': 'View to be t-called',
+        'type': 'qweb',
+        'arch': '<div>I will be t-called</div>',
+        'key': 'test_website.update_module_view_to_be_t_called',
+    })
+    update_module_child_view = View.create({
+        'name': 'Child View',
+        'mode': 'extension',
+        'inherit_id': update_module_base_view.id,
+        'arch': '''
+            <div position="inside">
+                <t t-call="test_website.update_module_view_to_be_t_called"/>
+            </div>
+        ''',
+        'key': 'test_website.update_module_child_view',
+    })
 
-        # Create IMD so when updating the module the views will be removed (not found in file)
-        Imd.create({
-            'module': 'test_website',
-            'name': 'update_module_view_to_be_t_called',
-            'model': 'ir.ui.view',
-            'res_id': update_module_view_to_be_t_called.id,
-        })
-        Imd.create({
-            'module': 'test_website',
-            'name': 'update_module_child_view',
-            'model': 'ir.ui.view',
-            'res_id': update_module_child_view.id,
-        })
+    # Create IMD so when updating the module the views will be removed (not found in file)
+    Imd.create({
+        'module': 'test_website',
+        'name': 'update_module_view_to_be_t_called',
+        'model': 'ir.ui.view',
+        'res_id': update_module_view_to_be_t_called.id,
+    })
+    Imd.create({
+        'module': 'test_website',
+        'name': 'update_module_child_view',
+        'model': 'ir.ui.view',
+        'res_id': update_module_child_view.id,
+    })
 
-        # Trigger COW on child view
-        update_module_child_view.with_context(website_id=1).write({'name': 'Child View (W1)'})
+    # Trigger COW on child view
+    update_module_child_view.with_context(website_id=1).write({'name': 'Child View (W1)'})
 
-        # Ensure views are correctly setup
-        self.assertEqual(View.search_count([('type', '=', 'qweb'), ('key', '=', update_module_child_view.key)]), 2)
-        self.assertTrue(self.env.ref(update_module_view_to_be_t_called.key))
-        self.assertTrue(self.env.ref(update_module_base_view.key))
+    # Ensure views are correctly setup
+    msg = "View '%s' does not exist!"
+    assert View.search_count([
+        ('type', '=', 'qweb'),
+        ('key', '=', update_module_child_view.key)
+    ]) == 2, msg % update_module_child_view.key
+    assert bool(env.ref(update_module_view_to_be_t_called.key)),\
+        msg % update_module_view_to_be_t_called.key
+    assert bool(env.ref(update_module_base_view.key)), msg % update_module_base_view.key
 
-        # the upgrade of the module below instanciates a NEW registry, so we
-        # have to restore the old one in order to perform the cleanups
-        @self.addCleanup
-        def cleanup():
-            self.registry.registries[self.registry.db_name] = self.registry
+    # Upgrade the module
+    test_website_module = env['ir.module.module'].search([('name', '=', 'test_website')])
+    test_website_module.button_immediate_upgrade()
+    env.reset()     # clear the set of environments
+    env = env()     # get an environment that refers to the new registry
 
-        # Update the module
-        test_website_module = self.env['ir.module.module'].search([('name', '=', 'test_website')])
-        test_website_module.button_immediate_upgrade()
+    # Ensure generic views got removed
+    view = env.ref('test_website.update_module_view_to_be_t_called', raise_if_not_found=False)
+    assert not view, "Generic view did not get removed!"
 
-        # Ensure generic views got removed
-        self.assertFalse(self.env.ref('test_website.update_module_view_to_be_t_called', raise_if_not_found=False))
-        # Ensure specific COW views got removed
-        self.assertEqual(View.search_count([('type', '=', 'qweb'), ('key', '=', 'test_website.update_module_child_view')]), 0)
+    # Ensure specific COW views got removed
+    assert not env['ir.ui.view'].search_count([
+        ('type', '=', 'qweb'),
+        ('key', '=', 'test_website.update_module_child_view'),
+    ]), "Specific COW views did not get removed!"
diff --git a/odoo/addons/base/models/ir_module.py b/odoo/addons/base/models/ir_module.py
index 70f0b1d08ec..f140a4d8250 100644
--- a/odoo/addons/base/models/ir_module.py
+++ b/odoo/addons/base/models/ir_module.py
@@ -11,6 +11,7 @@ import os
 import pkg_resources
 import shutil
 import tempfile
+import threading
 import zipfile
 
 import requests
@@ -563,6 +564,13 @@ class Module(models.Model):
         }
 
     def _button_immediate_function(self, function):
+        if getattr(threading.currentThread(), 'testing', False):
+            raise RuntimeError(
+                "Module operations inside tests are not transactional and thus forbidden.\n"
+                "If you really need to perform module operations to test a specific behavior, it "
+                "is best to write it as a standalone script, and ask the runbot/metastorm team "
+                "for help."
+            )
         try:
             # This is done because the installation/uninstallation/upgrade can modify a currently
             # running cron job and prevent it from finishing, and since the ir_cron table is locked
diff --git a/odoo/tests/common.py b/odoo/tests/common.py
index 33c8f0ed695..d9792910ca4 100644
--- a/odoo/tests/common.py
+++ b/odoo/tests/common.py
@@ -31,6 +31,7 @@ from contextlib import contextmanager
 from datetime import datetime, date
 from unittest.mock import patch
 
+from collections import defaultdict
 from decorator import decorator
 from itertools import zip_longest as izip_longest
 from lxml import etree, html
@@ -76,6 +77,29 @@ def get_db_name():
     return db
 
 
+standalone_tests = defaultdict(list)
+
+
+def standalone(*tags):
+    """ Decorator for standalone test functions.  This is somewhat dedicated to
+    tests that install, upgrade or uninstall some modules, which is currently
+    forbidden in regular test cases.  The function is registered under the given
+    ``tags`` and the corresponding Odoo module name.
+    """
+    def register(func):
+        # register func by odoo module name
+        if func.__module__.startswith('odoo.addons.'):
+            module = func.__module__.split('.')[2]
+            standalone_tests[module].append(func)
+        # register func with aribitrary name, if any
+        for tag in tags:
+            standalone_tests[tag].append(func)
+        standalone_tests['all'].append(func)
+        return func
+
+    return register
+
+
 # For backwards-compatibility - get_db_name() should be used instead
 DB = get_db_name()
 
diff --git a/odoo/tests/test-uninstalls.py b/odoo/tests/test_module_operations.py
index 1025ca2b5bd..1ce9d4e3226 100755
--- a/odoo/tests/test-uninstalls.py
+++ b/odoo/tests/test_module_operations.py
@@ -3,14 +3,16 @@ import argparse
 import logging
 import os
 import sys
+import time
 
 sys.path.append(os.path.abspath(os.path.join(__file__,'../../../')))
 
 import odoo
-from odoo.tools import topological_sort
+from odoo.tools import topological_sort, unique
 from odoo.netsvc import init_logger
+from odoo.tests import standalone_tests
 
-_logger = logging.getLogger(__name__)
+_logger = logging.getLogger('test_module_operations')
 
 BLACKLIST = {
     'auth_ldap', 'document_ftp', 'base_gengo', 'website_gengo', 'website_instantclick', 'pad',
@@ -56,6 +58,10 @@ def parse_args():
         help="Comma-separated list of paths to directories containing extra Odoo modules")
     parser.add_argument("--uninstall", "-U", type=str,
         help="Comma-separated list of modules to uninstall/reinstall")
+    parser.add_argument("--standalone", type=str,
+        help="Launch standalone scripts tagged with @standalone. Accepts a list of "
+             "module names or tags separated by commas. 'all' will run all available scripts."
+    )
     return parser.parse_args()
 
 
@@ -107,6 +113,36 @@ def test_uninstall(args):
         install(args.database, module_id, module_name)
 
 
+def test_scripts(args):
+    """ Tries to launch standalone scripts tagged with @post_testing """
+    # load the registry once for script discovery
+    registry = odoo.registry(args.database)
+    for module_name in registry._init_modules:
+        # import tests for loaded modules
+        odoo.modules.module.get_test_modules(module_name)
+
+    # fetch and filter scripts to test
+    funcs = list(unique(
+        func
+        for tag in args.standalone.split(',')
+        for func in standalone_tests[tag]
+    ))
+
+    start_time = time.time()
+    for index, func in enumerate(funcs, start=1):
+        with odoo.api.Environment.manage():
+            with odoo.registry(args.database).cursor() as cr:
+                env = odoo.api.Environment(cr, odoo.SUPERUSER_ID, {})
+                _logger.info("Executing standalone script: %s (%d / %d)",
+                             func.__name__, index, len(funcs))
+                try:
+                    func(env)
+                except Exception:
+                    _logger.error("Standalone script %s failed", func.__name__, exc_info=True)
+
+    _logger.info("%d standalone scripts executed in %.2fs" % (len(funcs), time.time() - start_time))
+
+
 if __name__ == '__main__':
     args = parse_args()
 
@@ -121,5 +157,7 @@ if __name__ == '__main__':
 
     if args.uninstall:
         test_uninstall(args)
+    elif args.standalone:
+        test_scripts(args)
     else:
         test_full(args)