From 0fa99e31273bd695a9410170f06da61be3a3d9ea Mon Sep 17 00:00:00 2001
From: "Adrien Widart (awt)" <awt@odoo.com>
Date: Tue, 22 Nov 2022 12:57:11 +0000
Subject: [PATCH] [FIX] core: clean env between tests
This commit description is in three parts: a generic explanation, a real
use case and the solutions
**Explanations**
1. When getting an odoo `Environment`, we first generate a tuple used as
an environment identifier:
https://github.com/odoo/odoo/blob/7475bcbef601b69e11c88a2ebb5fa39d7fcb52ab/odoo/api.py#L446
Then, there are two possibilities:
1. If such environment already exists in the environments list of the
transaction, we reuse it:
https://github.com/odoo/odoo/blob/7475bcbef601b69e11c88a2ebb5fa39d7fcb52ab/odoo/api.py#L448-L452
2. Else, we create a new one and store it in the transaction:
https://github.com/odoo/odoo/blob/7475bcbef601b69e11c88a2ebb5fa39d7fcb52ab/odoo/api.py#L454-L464
2. The `company` attribute of an `Environment` object is a lazy property
https://github.com/odoo/odoo/blob/7475bcbef601b69e11c88a2ebb5fa39d7fcb52ab/odoo/api.py#L537-L538
It means that its value won't be recomputed unless we delete it
https://github.com/odoo/odoo/blob/083c70bbb63f27839b2c9a4b549e216947bc4dd1/odoo/tools/func.py#L12-L17
3. When running a `SavepointCase` test class, the `setUp` method ensures
that, after each test, we do cleaning:
https://github.com/odoo/odoo/blob/a50a65be19b682201fc010d33c1b1f0b90cdf4d3/odoo/tests/common.py#L652-L662
The functions are added in a stack. So, in the execution order, we
will
1. Clear the current environment and the registry
2. Reset the environments list with the ones that were existing
before the executed test
4. Suppose a test class TC that inherits `SavepointCase`. TC has a
special `setUpClass`:
1. We create a new user U and replace the environment with a new one,
E1, based on U. This user has a company Comp_U. Because of [1], the new
environment E1 is added to the environments list of the transaction.
2. For some reasons, we need to do some operations in `sudo` mode:
again, because of [1], a new environment E2 (same as E1 but with the
`su` flag to `True`) is created and added to the environments list of
the transaction
5. TC contains a first test T1. In that test, we create a company
Comp_tmp and set it as default one to U. Therefore, E1 and E2 are
updated (their `company` attribute is now Comp_tmp)
6. At the end of T1, because of [3]:
1. E1 is reset (but its lazy properties are not deleted)
2. The environments list of the transaction is reset and still
contains E1 and E2 as they have been created in the class setup (i.e.
before the test setup)
7. In a second test T2, there will be an inconsistency: both E1 and E2
have an incorrect value for their field `company`: Comp_tmp, which is
not the value defined on `self.env.user.company_id` (the value Comp_tmp
does not even exist anymore)
**Real use case**
- The class `AccountTestInvoicingCommon` is an inheritance of
`SavepointCase`. In its class setup, we create a user and set it on the
current environment. Later on, we create a company (the sudo mode will
be activated during the company creation process)
https://github.com/odoo/odoo/blob/1e69c4fe5f8dd8a92d92f350b6d6a9539157f206/addons/account/tests/common.py#L38-L52
- The class `TestPurchaseOrder` inherits `AccountTestInvoicingCommon`
- The test `:TestPurchaseOrder.test_06_on_time_rate` creates a company
and sets it as the one of the current user
https://github.com/odoo/odoo/blob/87ffe5983be7f0f4a926b458c4bd1f13001048ec/addons/purchase_stock/tests/test_purchase_order.py#L313-L316
- Therefore, all next tests will have an issue with the environment
(unless we force the writing of the company on the current user to
bypass the issue)
```py
self.assertEqual(self.env.user.company_id, self.env.company) # will fail
self.assertTrue(self.env.company.exists()) # will fail
```
**Solution**
The above issue has been fixed from Odoo 15 on, thanks to commit C1.
Thanks to that diff, at step [3.1] in the above explanations, the lazy
properties of E1 are deleted (so, because of [2], the `company`
attribute of E1 will be correct again). However, once C1 is applied, we
can still add some lines in T2 to fail the test:
```py
sudo_env = self.env.user.sudo().env
self.assertEqual(self.env.user.company_id, sudo_env.company) # will fail
self.assertTrue(sudo_env.company.exists()) # will fail
```
The reason: at the end of the test T1, we reset the environments list of
the transaction ([6.2]). However, E2 has been created before the test
setup (it was in the class setup, see [4.2]). So, after the list reset,
E2 is still in that list and still has the value Comp_tmp for its
`company` attribute.
So... first we can conclude that C1 is not enough. Second, once the
environments list of the transaction is reset ([3.2]), we also have to
reset each environment to ensure that their lazy properties are correct.
We can then remove [3.1] since it will be included in that new step.
C1 a40511cd29d0ded5f104edf3ad3f41dbf6ea2a29
closes odoo/odoo#106252
Signed-off-by: Raphael Collet <rco@odoo.com>
---
.../test_testing_utilities/tests/__init__.py | 1 +
.../test_testing_utilities/tests/test_env.py | 44 +++++++++++++++++++
odoo/api.py | 1 +
odoo/tests/common.py | 4 +-
4 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 odoo/addons/test_testing_utilities/tests/test_env.py
diff --git a/odoo/addons/test_testing_utilities/tests/__init__.py b/odoo/addons/test_testing_utilities/tests/__init__.py
index 78398a30bce9..2ae17dd13856 100644
--- a/odoo/addons/test_testing_utilities/tests/__init__.py
+++ b/odoo/addons/test_testing_utilities/tests/__init__.py
@@ -3,3 +3,4 @@ from . import test_methods
from . import test_lxml
from . import test_form_impl
from . import test_xml_tools
+from . import test_env
diff --git a/odoo/addons/test_testing_utilities/tests/test_env.py b/odoo/addons/test_testing_utilities/tests/test_env.py
new file mode 100644
index 000000000000..77b6353f9310
--- /dev/null
+++ b/odoo/addons/test_testing_utilities/tests/test_env.py
@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.tests.common import SavepointCase
+
+
+class TestEnv(SavepointCase):
+
+ @classmethod
+ def setUpClass(cls):
+ super(TestEnv, cls).setUpClass()
+ user = cls.env['res.users'].create({
+ 'name': 'superuser',
+ 'login': 'superuser',
+ 'password': 'superuser',
+ 'groups_id': [(6, 0, cls.env.user.groups_id.ids)],
+ })
+ cls.env = cls.env(user=user)
+
+ # make sure there is at least another environment in the current transaction
+ cls.sudo_env = cls.env(su=True)
+
+ def test_env_company_part_01(self):
+ """
+ The main goal of the test is actually to check the values of the
+ environment after this test execution (see test_env_company_part_02)
+ """
+ company = self.env['res.company'].create({
+ "name": "Test Company",
+ })
+ self.env.user.write({
+ 'company_id': company.id,
+ 'company_ids': [(4, company.id), (4, self.env.company.id)],
+ })
+ self.assertEqual(self.env.company, self.env.user.company_id)
+ self.assertTrue(self.env.company.exists())
+ self.assertEqual(self.sudo_env.company, self.env.user.company_id)
+ self.assertTrue(self.sudo_env.company.exists())
+
+ def test_env_company_part_02(self):
+ self.assertEqual(self.env.company, self.env.user.company_id)
+ self.assertTrue(self.env.company.exists())
+ self.assertEqual(self.sudo_env.company, self.env.user.company_id)
+ self.assertTrue(self.sudo_env.company.exists())
diff --git a/odoo/api.py b/odoo/api.py
index 606a06e71374..c95d3daa3e0c 100644
--- a/odoo/api.py
+++ b/odoo/api.py
@@ -616,6 +616,7 @@ class Environment(Mapping):
""" Clear all record caches, and discard all fields to recompute.
This may be useful when recovering from a failed ORM operation.
"""
+ lazy_property.reset_all(self)
self.cache.invalidate()
self.all.tocompute.clear()
self.all.towrite.clear()
diff --git a/odoo/tests/common.py b/odoo/tests/common.py
index 0778c059305d..b39a15617188 100644
--- a/odoo/tests/common.py
+++ b/odoo/tests/common.py
@@ -655,11 +655,13 @@ class SavepointCase(SingleTransactionCase):
# restore environments after the test to avoid invoking flush() with an
# invalid environment (inexistent user id) from another test
envs = self.env.all.envs
+ for env in list(envs):
+ self.addCleanup(env.clear)
+ # restore the set of known environments as it was at setUp
self.addCleanup(envs.update, list(envs))
self.addCleanup(envs.clear)
self.addCleanup(self.registry.clear_caches)
- self.addCleanup(self.env.clear)
self._savepoint_id = next(savepoint_seq)
self.cr.execute('SAVEPOINT test_%d' % self._savepoint_id)
--
GitLab