[MERGE] crm: make TestLeadAssignPerf deterministic
Purpose
========
The performances tests in TestLeadAssignPerf have a margin for queries
because of some randomness in query counts.
Those margins avoid random failures but will hide small increments,
making the query count fail randomly in future build. This margin also
makes the update of query counts difficult.
This commit tries to identify the source of this randomness and proposes
some fixes.
Sources of randomness
=====================
Comparing different executions, the query can differ on two point:
1. During the flush, while updating the lead
1.1 the written user_id can vary (~1 additional query)
1.2 the written convert_date can changes (~4 additional queries)
1.3 the written date_last_stage_update can change (~1 additional query)
2. during _handle_salesmen_assignment
2.1 write/_message_auto_subscribe can change? (~1 additional query)
the point 1.2 and 1.3 can be easily reproduced, adding sleep,
especially a 0.1 sleep in convert_opportunity
Writing different values for date will lead to multiple execute
when flushing the records:
the orm cannot group records with different values
Proposed fixes
==============
A. Use `cr.now` instead of `fields.datetime.now`
Using the transaction time will avoid randomness linked to the change of
second during the transaction. This will fix 1.2 and 1.3
B. Sorting members
The members comes from a o2m and the order is not deterministic.
During the test with a subset of lead (10 instead of 100),
the two last members ('Martin Sales Manager' and 'Orteil Sales Own')
can come in different orders, leading to different user_id set on lead.
Instead of 4 different users, the lead where sometimes dispatch on
5 different users with this setup. (5 queries in flush() instead of 4)
This is fixed by adding a complete order (adding id) on crm.team.members
This will solve 1.1
C. The leads are also ordered by id if they have the same probability,
since it shouldn't hurt and make the search more deterministic.
2.1 was fixed by B or C (or is not fixed) not sure anymore.
Links
=====
Task-2796579
closes odoo/odoo#85525
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -316,12 +316,12 @@ class Lead(models.Model):
|
||||
@api.depends('user_id')
|
||||
def _compute_date_open(self):
|
||||
for lead in self:
|
||||
lead.date_open = fields.Datetime.now() if lead.user_id else False
|
||||
lead.date_open = self.env.cr.now() if lead.user_id else False
|
||||
|
||||
@api.depends('stage_id')
|
||||
def _compute_date_last_stage_update(self):
|
||||
for lead in self:
|
||||
lead.date_last_stage_update = fields.Datetime.now()
|
||||
lead.date_last_stage_update = self.env.cr.now()
|
||||
|
||||
@api.depends('create_date', 'date_open')
|
||||
def _compute_day_open(self):
|
||||
@@ -852,7 +852,7 @@ class Lead(models.Model):
|
||||
context.setdefault('default_team_id', self.team_id.id)
|
||||
# Set date_open to today if it is an opp
|
||||
default = default or {}
|
||||
default['date_open'] = fields.Datetime.now() if self.type == 'opportunity' else False
|
||||
default['date_open'] = self.env.cr.now() if self.type == 'opportunity' else False
|
||||
# Do not assign to an archived user
|
||||
if not self.user_id.active:
|
||||
default['user_id'] = False
|
||||
@@ -1526,8 +1526,8 @@ class Lead(models.Model):
|
||||
new_team_id = team_id if team_id else self.team_id.id
|
||||
upd_values = {
|
||||
'type': 'opportunity',
|
||||
'date_open': fields.Datetime.now(),
|
||||
'date_conversion': fields.Datetime.now(),
|
||||
'date_open': self.env.cr.now(),
|
||||
'date_conversion': self.env.cr.now(),
|
||||
}
|
||||
if customer != self.partner_id:
|
||||
upd_values['partner_id'] = customer.id if customer else False
|
||||
|
||||
@@ -138,7 +138,7 @@ class TeamMember(models.Model):
|
||||
['&', '&', ('user_id', '=', False), ('date_open', '=', False), ('team_id', '=', member.crm_team_id.id)]
|
||||
])
|
||||
|
||||
leads = self.env["crm.lead"].search(lead_domain, order='probability DESC', limit=lead_limit)
|
||||
leads = self.env["crm.lead"].search(lead_domain, order='probability DESC, id', limit=lead_limit)
|
||||
|
||||
to_assign = member._get_assignment_quota(work_days=work_days)
|
||||
members_data[member.id] = {
|
||||
|
||||
@@ -29,7 +29,9 @@ class TestLeadAssignCommon(TestLeadConvertCommon):
|
||||
|
||||
# don't mess with existing leads, unlink those assigned to users used here to make tests
|
||||
# repeatable (archive is not sufficient because of lost leads)
|
||||
cls.env['crm.lead'].with_context(active_test=False).search(['|', ('team_id', '=', False), ('user_id', 'in', cls.sales_teams.member_ids.ids)]).unlink()
|
||||
|
||||
with mute_logger('odoo.models.unlink'):
|
||||
cls.env['crm.lead'].with_context(active_test=False).search(['|', ('team_id', '=', False), ('user_id', 'in', cls.sales_teams.member_ids.ids)]).unlink()
|
||||
cls.bundle_size = 50
|
||||
cls.env['ir.config_parameter'].set_param('crm.assignment.commit.bundle', '%s' % cls.bundle_size)
|
||||
cls.env['ir.config_parameter'].set_param('crm.assignment.delay', '0')
|
||||
|
||||
@@ -492,8 +492,7 @@ class TestLeadConvertBatch(crm_common.TestLeadConvertMassCommon):
|
||||
@users('user_sales_manager')
|
||||
def test_lead_convert_batch_internals(self):
|
||||
""" Test internals of convert wizard, working in batch mode """
|
||||
date = Datetime.from_string('2020-01-20 16:00:00')
|
||||
self.crm_lead_dt_mock.now.return_value = date
|
||||
date = self.env.cr.now()
|
||||
|
||||
lead_w_partner = self.lead_w_partner
|
||||
lead_w_contact = self.lead_w_contact
|
||||
|
||||
@@ -48,7 +48,8 @@ class TestLeadAssignPerf(TestLeadAssignCommon):
|
||||
leads.flush()
|
||||
|
||||
with self.with_user('user_sales_manager'):
|
||||
with self.assertQueryCount(user_sales_manager=1283): # 1277-1283 generally - crm only: 1204
|
||||
self.env['res.users'].has_group('base.group_user') # warmup the cache to avoid inconsistency between community an enterprise
|
||||
with self.assertQueryCount(user_sales_manager=1279):
|
||||
self.env['crm.team'].browse(self.sales_teams.ids)._action_assign_leads(work_days=2)
|
||||
|
||||
# teams assign
|
||||
@@ -92,7 +93,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon):
|
||||
leads.flush()
|
||||
|
||||
with self.with_user('user_sales_manager'):
|
||||
with self.assertQueryCount(user_sales_manager=589): # 584-585 generally, sometimes 589
|
||||
with self.assertQueryCount(user_sales_manager=584):
|
||||
self.env['crm.team'].browse(self.sales_teams.ids)._action_assign_leads(work_days=2)
|
||||
|
||||
# teams assign
|
||||
@@ -174,7 +175,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon):
|
||||
leads.flush()
|
||||
|
||||
with self.with_user('user_sales_manager'):
|
||||
with self.assertQueryCount(user_sales_manager=6502): # 6494-6502 generally
|
||||
with self.assertQueryCount(user_sales_manager=6487):
|
||||
self.env['crm.team'].browse(sales_teams.ids)._action_assign_leads(work_days=30)
|
||||
|
||||
# teams assign
|
||||
|
||||
@@ -89,7 +89,7 @@ class EventRegistration(models.Model):
|
||||
for registration in self:
|
||||
if not registration.date_closed:
|
||||
if registration.state == 'done':
|
||||
registration.date_closed = fields.Datetime.now()
|
||||
registration.date_closed = self.env.cr.now()
|
||||
else:
|
||||
registration.date_closed = False
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ class CrmTeamMember(models.Model):
|
||||
_inherit = ['mail.thread']
|
||||
_description = 'Sales Team Member'
|
||||
_rec_name = 'user_id'
|
||||
_order = 'create_date ASC'
|
||||
_order = 'create_date ASC, id'
|
||||
_check_company_auto = True
|
||||
|
||||
crm_team_id = fields.Many2one(
|
||||
|
||||
+22
-16
@@ -93,6 +93,7 @@ class Speedscope:
|
||||
if sql:
|
||||
self.add_output([key], hide_gaps=True, display_name=f'{key} (no gap)')
|
||||
self.add_output([key], continuous=False, complete=False, display_name=f'{key} (density)')
|
||||
|
||||
else:
|
||||
self.add_output([key], display_name=key)
|
||||
return self
|
||||
@@ -144,7 +145,8 @@ class Speedscope:
|
||||
stack_ids.append(self.get_frame_id(frame))
|
||||
return stack_ids
|
||||
|
||||
def process(self, entries, continuous=True, hide_gaps=False, use_context=True):
|
||||
def process(self, entries, continuous=True, hide_gaps=False, use_context=True, constant_time=False):
|
||||
# constant_time parameters is mainly usefull to hide temporality when focussing on sql determinism
|
||||
entry_end = previous_end = None
|
||||
if not entries:
|
||||
return []
|
||||
@@ -157,25 +159,29 @@ class Speedscope:
|
||||
if last_entry['stack']:
|
||||
entries.append({'stack': [], 'start': last_entry['start'] + last_entry.get('time', 0)})
|
||||
|
||||
for entry in entries:
|
||||
previous_end = entry_end
|
||||
|
||||
if hide_gaps and previous_end:
|
||||
entry_start = previous_end
|
||||
for index, entry in enumerate(entries):
|
||||
if constant_time:
|
||||
entry_start = close_time = index
|
||||
else:
|
||||
entry_start = entry['start'] - frames_start
|
||||
previous_end = entry_end
|
||||
|
||||
if previous_end and previous_end > entry_start:
|
||||
# skip entry if entry starts after another entry end/
|
||||
continue
|
||||
if hide_gaps and previous_end:
|
||||
entry_start = previous_end
|
||||
else:
|
||||
entry_start = entry['start'] - frames_start
|
||||
|
||||
if previous_end:
|
||||
close_time = min(entry_start, previous_end)
|
||||
else:
|
||||
close_time = entry_start
|
||||
if previous_end and previous_end > entry_start:
|
||||
# skip entry if entry starts after another entry end
|
||||
continue
|
||||
|
||||
if previous_end:
|
||||
close_time = min(entry_start, previous_end)
|
||||
else:
|
||||
close_time = entry_start
|
||||
|
||||
entry_time = entry.get('time')
|
||||
entry_end = None if entry_time is None else entry_start + entry_time
|
||||
|
||||
entry_time = entry.get('time')
|
||||
entry_end = None if entry_time is None else entry_start + entry_time
|
||||
entry_stack_ids = self.stack_to_ids(
|
||||
entry['stack'] or [],
|
||||
use_context and entry.get('exec_context'),
|
||||
|
||||
Reference in New Issue
Block a user