Skip to content
Snippets Groups Projects
Commit 0fa99e31 authored by Adrien Widart (awt)'s avatar Adrien Widart (awt)
Browse files

[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 a40511cd

closes odoo/odoo#106252

Signed-off-by: default avatarRaphael Collet <rco@odoo.com>
parent 11d023f0
No related branches found
No related tags found
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment