[REF] tests: `TestCursor` is now a proxy to a real `Cursor`

This allows rpc requests in `HttpCase` to use the cursor `self.cr`, which is
now shared between the Python test and the rpc requests.  This simplifies code
to prepare a JS test, and code to check the result of a JS tour.

Fixes #12237

10 files changed, 99 insertions, 106 deletions

diff --git a/addons/point_of_sale/tests/test_frontend.py b/addons/point_of_sale/tests/test_frontend.py
index b3dae4e9350..091d880bc6c 100644
--- a/addons/point_of_sale/tests/test_frontend.py
+++ b/addons/point_of_sale/tests/test_frontend.py
@@ -10,9 +10,7 @@ import odoo.tests
 
 class TestUi(odoo.tests.HttpCase):
     def test_01_pos_basic_order(self):
-        cr = self.registry.cursor()
-        assert cr == self.registry.test_cr
-        env = Environment(cr, self.uid, {})
+        env = self.env
 
         # By default parent_store computation is deferred until end of
         # tests. Pricelist items however are sorted based on these
@@ -28,9 +26,7 @@ class TestUi(odoo.tests.HttpCase):
                                                  'name': 'Account Receivable - Test',
                                                  'user_type_id': env.ref('account.data_account_type_receivable').id,
                                                  'reconcile': True})
-        field = self.env['ir.model.fields'].search([('name', '=', 'property_account_receivable_id'),
-                                                    ('model', '=', 'res.partner'),
-                                                    ('relation', '=', 'account.account')], limit=1)
+        field = env['ir.model.fields']._get('res.partner', 'property_account_receivable_id')
         env['ir.property'].create({'name': 'property_account_receivable_id',
                                    'company_id': main_company.id,
                                    'fields_id': field.id,
@@ -294,7 +290,6 @@ class TestUi(odoo.tests.HttpCase):
         # that are returned by the backend in module_boot. Without
         # this you end up with js, css but no qweb.
         env['ir.module.module'].search([('name', '=', 'point_of_sale')], limit=1).state = 'installed'
-        cr.release()
 
         self.phantom_js("/pos/web",
                         "odoo.__DEBUG__.services['web_tour.tour'].run('pos_pricelist')",
diff --git a/addons/website_crm/tests/test_website_crm.py b/addons/website_crm/tests/test_website_crm.py
index cc2db89fdc1..67e93625c20 100644
--- a/addons/website_crm/tests/test_website_crm.py
+++ b/addons/website_crm/tests/test_website_crm.py
@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo.api import Environment
 import odoo.tests
 
 
@@ -11,12 +10,9 @@ class TestWebsiteCrm(odoo.tests.HttpCase):
     def test_tour(self):
         self.phantom_js("/", "odoo.__DEBUG__.services['web_tour.tour'].run('website_crm_tour')", "odoo.__DEBUG__.services['web_tour.tour'].tours.website_crm_tour.ready")
 
-        # need environment using the test cursor as it's not committed
-        cr = self.registry.cursor()
-        assert cr is self.registry.test_cr
-        env = Environment(cr, self.uid, {})
-        record = env['crm.lead'].search([('description', '=', '### TOUR DATA ###')])
-        assert len(record) == 1
-        assert record.contact_name == 'John Smith'
-        assert record.email_from == 'john@smith.com'
-        assert record.partner_name == 'Odoo S.A.'
+        # check result
+        record = self.env['crm.lead'].search([('description', '=', '### TOUR DATA ###')])
+        self.assertEqual(len(record), 1)
+        self.assertEqual(record.contact_name, 'John Smith')
+        self.assertEqual(record.email_from, 'john@smith.com')
+        self.assertEqual(record.partner_name, 'Odoo S.A.')
diff --git a/addons/website_forum/tests/test_forum_process.py b/addons/website_forum/tests/test_forum_process.py
index 52f49be3133..367186ccbfd 100644
--- a/addons/website_forum/tests/test_forum_process.py
+++ b/addons/website_forum/tests/test_forum_process.py
@@ -9,9 +9,7 @@ class TestUi(odoo.tests.HttpCase):
         self.phantom_js("/", "odoo.__DEBUG__.services['web_tour.tour'].run('question')", "odoo.__DEBUG__.services['web_tour.tour'].tours.question.ready", login="admin")
 
     def test_02_demo_question(self):
-        with self.cursor() as test_cr:
-            env = self.env(cr=test_cr)
-            forum = env.ref('website_forum.forum_help')
-            demo = env.ref('base.user_demo')
-            demo.karma = forum.karma_post + 1
+        forum = self.env.ref('website_forum.forum_help')
+        demo = self.env.ref('base.user_demo')
+        demo.karma = forum.karma_post + 1
         self.phantom_js("/", "odoo.__DEBUG__.services['web_tour.tour'].run('forum_question')", "odoo.__DEBUG__.services['web_tour.tour'].tours.forum_question.ready", login="demo")
diff --git a/addons/website_hr_recruitment/tests/test_website_hr_recruitment.py b/addons/website_hr_recruitment/tests/test_website_hr_recruitment.py
index 169795e5f1c..1d406ea01b5 100644
--- a/addons/website_hr_recruitment/tests/test_website_hr_recruitment.py
+++ b/addons/website_hr_recruitment/tests/test_website_hr_recruitment.py
@@ -9,13 +9,9 @@ class TestWebsiteHrRecruitmentForm(odoo.tests.HttpCase):
     def test_tour(self):
         self.phantom_js("/", "odoo.__DEBUG__.services['web_tour.tour'].run('website_hr_recruitment_tour')", "odoo.__DEBUG__.services['web_tour.tour'].tours.website_hr_recruitment_tour.ready")
 
-        # get test cursor to read from same transaction browser is writing to
-        cr = self.registry.cursor()
-        assert cr == self.registry.test_cr
-        env = Environment(cr, self.uid, {})
-
-        record = env['hr.applicant'].search([('description', '=', '### HR RECRUITMENT TEST DATA ###')])
-        assert len(record) == 1
-        assert record.partner_name == "John Smith"
-        assert record.email_from == "john@smith.com"
-        assert record.partner_phone == '118.218'
+        # check result
+        record = self.env['hr.applicant'].search([('description', '=', '### HR RECRUITMENT TEST DATA ###')])
+        self.assertEqual(len(record), 1)
+        self.assertEqual(record.partner_name, "John Smith")
+        self.assertEqual(record.email_from, "john@smith.com")
+        self.assertEqual(record.partner_phone, '118.218')
diff --git a/addons/website_mail_channel/tests/test_unsubscribe.py b/addons/website_mail_channel/tests/test_unsubscribe.py
index 743232890a8..ec1a915f232 100644
--- a/addons/website_mail_channel/tests/test_unsubscribe.py
+++ b/addons/website_mail_channel/tests/test_unsubscribe.py
@@ -1,7 +1,5 @@
 # -*- coding: utf-8 -*-
-import requests
 
-from odoo.api import Environment
 from odoo.tests import common
 from odoo.tools.misc import mute_logger, ustr
 
@@ -9,15 +7,11 @@ from odoo.tools.misc import mute_logger, ustr
 class TestConfirmUnsubscribe(common.HttpCase):
     def setUp(self):
         super(TestConfirmUnsubscribe, self).setUp()
-
-        cr = self.registry.cursor()
-        # apparently HttpCase does not properly update self.env?
-        self.env2 = env = Environment(cr, self.uid, {})
-        self.partner = env['res.partner'].create({
+        self.partner = self.env['res.partner'].create({
             'name': 'Bob',
             'email': 'bob@bob.bob'
         })
-        self.mailing_list = env['mail.channel'].create({
+        self.mailing_list = self.env['mail.channel'].create({
             'name': 'Test Mailing List',
             'public': 'public',
         })
@@ -25,20 +19,17 @@ class TestConfirmUnsubscribe(common.HttpCase):
 
     def test_not_subscribed(self):
         """Test warning works"""
-        self.env2.cr.release()
-
         self._unsubscribe_check("The address %s is already unsubscribed" % self.partner.email)
 
     @mute_logger('odoo.addons.website.models.ir_ui_view')
     def test_not_subscribed_no_template(self):
         """ Test warning works on db without template (code update w/o module update) """
-        self.env2.ref('website_mail_channel.not_subscribed').unlink()
+        self.env.ref('website_mail_channel.not_subscribed').unlink()
         self.assertEquals(
-            self.env2['ir.model.data'].search_count([
+            self.env['ir.model.data'].search_count([
             ('module', '=', 'website_mail_channel'),
             ('name', '=', 'not_subscribed'),
         ]), 0, 'XID for template should have been deleted')
-        self.env2.cr.release()
 
         self._unsubscribe_check("The address %s is already unsubscribed or was never subscribed to any mailing list" % self.partner.email)
 
@@ -46,7 +37,6 @@ class TestConfirmUnsubscribe(common.HttpCase):
         self.mailing_list.sudo().write({
             'channel_partner_ids': [(4, self.partner.id, False)]
         })
-        self.env2.cr.release()
         self.token = 'XXX'
 
         self._unsubscribe_check("Invalid or expired confirmation link.")
@@ -55,7 +45,6 @@ class TestConfirmUnsubscribe(common.HttpCase):
         self.mailing_list.sudo().write({
             'channel_partner_ids': [(4, self.partner.id, False)]
         })
-        self.env2.cr.release()
 
         self._unsubscribe_check("You have been correctly unsubscribed")
 
diff --git a/odoo/addons/base/tests/test_ir_actions.py b/odoo/addons/base/tests/test_ir_actions.py
index 7c5836fa97e..355ddaa143d 100644
--- a/odoo/addons/base/tests/test_ir_actions.py
+++ b/odoo/addons/base/tests/test_ir_actions.py
@@ -242,7 +242,7 @@ class TestCustomFields(common.TransactionCase):
         super(TestCustomFields, self).setUp()
 
         # use a test cursor instead of a real cursor
-        self.registry.enter_test_mode()
+        self.registry.enter_test_mode(self.cr)
         self.addCleanup(self.registry.leave_test_mode)
 
         # do not reload the registry after removing a field
diff --git a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
index c97ac8d0f95..3d372a6dcd2 100644
--- a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
+++ b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
@@ -332,22 +332,21 @@ class TestAssetsBundleInBrowser(HttpCase):
     def test_02_js_interpretation_inline(self):
         """ Checks that the javascript of a bundle is correctly interpretet when mixed with inline.
         """
-        with self.registry.cursor() as test_cursor:
-            view_arch = """
-            <data>
-                <xpath expr="." position="inside">
-                    <script type="text/javascript">
-                        var d = 4;
-                    </script>
-                </xpath>
-            </data>
-            """
-            self.env(cr=test_cursor)['ir.ui.view'].create({
-                'name': 'test bundle inheritance inline js',
-                'type': 'qweb',
-                'arch': view_arch,
-                'inherit_id': self.browse_ref('test_assetsbundle.bundle1').id,
-            })
+        view_arch = """
+        <data>
+            <xpath expr="." position="inside">
+                <script type="text/javascript">
+                    var d = 4;
+                </script>
+            </xpath>
+        </data>
+        """
+        self.env['ir.ui.view'].create({
+            'name': 'test bundle inheritance inline js',
+            'type': 'qweb',
+            'arch': view_arch,
+            'inherit_id': self.browse_ref('test_assetsbundle.bundle1').id,
+        })
 
         self.phantom_js(
             "/test_assetsbundle/js",
diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py
index 1d6dbe69726..6dd6556298b 100644
--- a/odoo/modules/registry.py
+++ b/odoo/modules/registry.py
@@ -15,6 +15,7 @@ import threading
 
 import odoo
 from .. import SUPERUSER_ID
+from odoo.sql_db import TestCursor
 from odoo.tools import (assertion_report, config, existing_tables,
                         lazy_classproperty, lazy_property, table_exists,
                         topological_sort, OrderedSet)
@@ -123,8 +124,9 @@ class Registry(Mapping):
         self.db_name = db_name
         self._db = odoo.sql_db.db_connect(db_name)
 
-        # special cursor for test mode; None means "normal" mode
+        # cursor for test mode; None means "normal" mode
         self.test_cr = None
+        self.test_lock = None
 
         # Indicates that the registry is
         self.loaded = False             # whether all modules are loaded
@@ -443,10 +445,11 @@ class Registry(Mapping):
         """ Test whether the registry is in 'test' mode. """
         return self.test_cr is not None
 
-    def enter_test_mode(self):
+    def enter_test_mode(self, cr):
         """ Enter the 'test' mode, where one cursor serves several requests. """
         assert self.test_cr is None
-        self.test_cr = self._db.test_cursor()
+        self.test_cr = cr
+        self.test_lock = threading.RLock()
         assert Registry._saved_lock is None
         Registry._saved_lock = Registry._lock
         Registry._lock = DummyRLock()
@@ -454,9 +457,8 @@ class Registry(Mapping):
     def leave_test_mode(self):
         """ Leave the test mode. """
         assert self.test_cr is not None
-        self.clear_caches()
-        self.test_cr.force_close()
         self.test_cr = None
+        self.test_lock = None
         assert Registry._saved_lock is not None
         Registry._lock = Registry._saved_lock
         Registry._saved_lock = None
@@ -465,14 +467,10 @@ class Registry(Mapping):
         """ Return a new cursor for the database. The cursor itself may be used
             as a context manager to commit/rollback and close automatically.
         """
-        cr = self.test_cr
-        if cr is not None:
-            # While in test mode, we use one special cursor across requests. The
-            # test cursor uses a reentrant lock to serialize accesses. The lock
-            # is granted here by cursor(), and automatically released by the
-            # cursor itself in its method close().
-            cr.acquire()
-            return cr
+        if self.test_cr is not None:
+            # When in test mode, we use a proxy object that uses 'self.test_cr'
+            # underneath.
+            return TestCursor(self.test_cr, self.test_lock)
         return self._db.cursor()
 
 
diff --git a/odoo/sql_db.py b/odoo/sql_db.py
index 50e4ac5dce7..8414dcf3dcc 100644
--- a/odoo/sql_db.py
+++ b/odoo/sql_db.py
@@ -10,6 +10,7 @@ the ORM does, in fact.
 
 from contextlib import contextmanager
 from functools import wraps
+import itertools
 import logging
 import time
 import uuid
@@ -425,42 +426,68 @@ class Cursor(object):
     def closed(self):
         return self._closed
 
-class TestCursor(Cursor):
-    """ A cursor to be used for tests. It keeps the transaction open across
-        several requests, and simulates committing, rolling back, and closing.
-    """
-    def __init__(self, *args, **kwargs):
-        super(TestCursor, self).__init__(*args, **kwargs)
-        # in order to simulate commit and rollback, the cursor maintains a
-        # savepoint at its last commit
-        self.execute("SAVEPOINT test_cursor")
-        # we use a lock to serialize concurrent requests
-        self._lock = threading.RLock()
 
-    def acquire(self):
-        self._lock.acquire()
+class TestCursor(object):
+    """ A pseudo-cursor to be used for tests, on top of a real cursor. It keeps
+        the transaction open across requests, and simulates committing, rolling
+        back, and closing:
+
+              test cursor           | queries on actual cursor
+            ------------------------+---------------------------------------
+              cr = TestCursor(...)  | SAVEPOINT test_cursor_N
+                                    |
+              cr.execute(query)     | query
+                                    |
+              cr.commit()           | SAVEPOINT test_cursor_N
+                                    |
+              cr.rollback()         | ROLLBACK TO SAVEPOINT test_cursor_N
+                                    |
+              cr.close()            | ROLLBACK TO SAVEPOINT test_cursor_N
+                                    |
 
-    def release(self):
-        self._lock.release()
+    """
+    _savepoint_seq = itertools.count()
 
-    def force_close(self):
-        super(TestCursor, self).close()
+    def __init__(self, cursor, lock):
+        self._closed = False
+        self._cursor = cursor
+        # we use a lock to serialize concurrent requests
+        self._lock = lock
+        self._lock.acquire()
+        # in order to simulate commit and rollback, the cursor maintains a
+        # savepoint at its last commit
+        self._savepoint = "test_cursor_%s" % next(self._savepoint_seq)
+        self._cursor.execute('SAVEPOINT "%s"' % self._savepoint)
 
     def close(self):
         if not self._closed:
-            self.rollback()             # for stuff that has not been committed
-        self.release()
+            self._closed = True
+            self._cursor.execute('ROLLBACK TO SAVEPOINT "%s"' % self._savepoint)
+            self._lock.release()
 
     def autocommit(self, on):
         _logger.debug("TestCursor.autocommit(%r) does nothing", on)
 
     def commit(self):
-        self.execute("RELEASE SAVEPOINT test_cursor")
-        self.execute("SAVEPOINT test_cursor")
+        self._cursor.execute('SAVEPOINT "%s"' % self._savepoint)
 
     def rollback(self):
-        self.execute("ROLLBACK TO SAVEPOINT test_cursor")
-        self.execute("SAVEPOINT test_cursor")
+        self._cursor.execute('ROLLBACK TO SAVEPOINT "%s"' % self._savepoint)
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        if exc_type is None:
+            self.commit()
+        self.close()
+
+    def __getattr__(self, name):
+        value = getattr(self._cursor, name)
+        if callable(value) and self._closed:
+            raise psycopg2.OperationalError('Unable to use a closed cursor.')
+        return value
+
 
 class LazyCursor(object):
     """ A proxy object to a cursor. The cursor itself is allocated only if it is
@@ -633,11 +660,6 @@ class Connection(object):
         _logger.debug('create %scursor to %r', cursor_type, self.dsn)
         return Cursor(self.__pool, self.dbname, self.dsn, serialized=serialized)
 
-    def test_cursor(self, serialized=True):
-        cursor_type = serialized and 'serialized ' or ''
-        _logger.debug('create test %scursor to %r', cursor_type, self.dsn)
-        return TestCursor(self.__pool, self.dbname, self.dsn, serialized=serialized)
-
     # serialized_cursor is deprecated - cursors are serialized by default
     serialized_cursor = cursor
 
diff --git a/odoo/tests/common.py b/odoo/tests/common.py
index 005b5a1cfcc..df4178ee0e9 100644
--- a/odoo/tests/common.py
+++ b/odoo/tests/common.py
@@ -326,7 +326,7 @@ class HttpCase(TransactionCase):
     def setUp(self):
         super(HttpCase, self).setUp()
         if self.registry_test_mode:
-            self.registry.enter_test_mode()
+            self.registry.enter_test_mode(self.cr)
             self.addCleanup(self.registry.leave_test_mode)
         # setup a magic session_id that will be rollbacked
         self.session = odoo.http.root.session_store.new()