From 8cf094ed0dc4624feb4eaa137edafd1b624b25ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 12 May 2021 13:56:38 +0000 Subject: [PATCH] [FIX] crm: improve assign test asserts Purpose is to avoid randomly failing tess depending on how leads are allocated through teams. We achieve that purpose by * creating more leads, easing their repartition through teams; * adding a lower and upper bound test for team allocation to ensure weights are globally ok; * changing member assign domain to fit more closely test lead data; We also improve test by using the ``crm.assignment.commit.bundle`` config parameter that has an impact on lead duplicates removing and therefore performances. LINKS Task ID-2528906 COM PR odoo/odoo#70756 Followup of Task ID-2489951 (assign process improvements) Followup of odoo/odoo#70172 (smoother assign process) X-original-commit: 8fdf97b6498adc0a2fb15e360ebbbaa1672534e5 --- addons/crm/tests/common.py | 2 +- addons/crm/tests/test_crm_lead_assignment.py | 49 ++++++++++++++------ addons/crm/tests/test_performances.py | 21 ++++----- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index c4e43df1236..2147169e8cc 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -503,7 +503,7 @@ class TestLeadConvertCommon(TestCrmCommon): }) cls.sales_team_convert_m1.write({ 'assignment_max': 30, - 'assignment_domain': [('priority', 'in', ['2', '3'])] + 'assignment_domain': [('probability', '>=', 20)] }) cls.sales_team_convert_m2 = cls.env['crm.team.member'].create({ 'user_id': cls.user_sales_manager.id, diff --git a/addons/crm/tests/test_crm_lead_assignment.py b/addons/crm/tests/test_crm_lead_assignment.py index 40e624387d2..014cbe797e3 100644 --- a/addons/crm/tests/test_crm_lead_assignment.py +++ b/addons/crm/tests/test_crm_lead_assignment.py @@ -28,8 +28,8 @@ 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() - cls.bundle_size = 5 - cls.env['ir.config_parameter'].set_param('crm.assignment.bundle', '%s' % cls.bundle_size) + 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') def assertInitialData(self): @@ -43,7 +43,7 @@ class TestLeadAssignCommon(TestLeadConvertCommon): self.assertEqual(self.sales_team_1_m3.assignment_domain, "[('probability', '>=', 20)]") self.assertEqual(self.sales_team_convert.assignment_domain, "[('priority', 'in', ['1', '2', '3'])]") - self.assertEqual(self.sales_team_convert_m1.assignment_domain, "[('priority', 'in', ['2', '3'])]") + self.assertEqual(self.sales_team_convert_m1.assignment_domain, "[('probability', '>=', 20)]") self.assertEqual(self.sales_team_convert_m2.assignment_domain, False) # start afresh @@ -148,6 +148,8 @@ class TestLeadAssign(TestLeadAssignCommon): # re-assign existing leads, check monthly count is updated existing_leads[-2:]._handle_salesmen_assignment(user_ids=self.user_sales_manager.ids) + # commit probability and related fields + leads.flush() self.members.invalidate_cache(fnames=['lead_month_count']) self.assertEqual(self.sales_team_1_m3.lead_month_count, 12) @@ -199,10 +201,15 @@ class TestLeadAssign(TestLeadAssignCommon): # teams assign leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) # ensure order + self.assertEqual(len(leads), 122) leads_st1 = leads.filtered_domain([('team_id', '=', self.sales_team_1.id)]) leads_stc = leads.filtered_domain([('team_id', '=', self.sales_team_convert.id)]) - self.assertLessEqual(len(leads_st1), 128) - self.assertLessEqual(len(leads_stc), 96) + + # check random globally assigned enough leads to team + self.assertLessEqual(len(leads_st1), 83) # 75 * 122 / 165 * 1.5 (because random) + self.assertLessEqual(len(leads_stc), 100) # 90 * 122 / 165 * 1.5 (because random) + self.assertGreaterEqual(len(leads_st1), 27) # 75 * 122 / 165 * 0.5 (because random) + self.assertGreaterEqual(len(leads_stc), 33) # 90 * 122 / 165 * 0.5 (because random) self.assertEqual(len(leads_st1) + len(leads_stc), len(leads)) # Make sure all lead are assigned # salespersons assign @@ -230,7 +237,7 @@ class TestLeadAssign(TestLeadAssignCommon): lead_type='lead', user_ids=[False], partner_ids=[False], - count=100 + count=150 ) # commit probability and related fields leads.flush() @@ -252,9 +259,14 @@ class TestLeadAssign(TestLeadAssignCommon): leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) # ensure order leads_st1 = leads.filtered_domain([('team_id', '=', self.sales_team_1.id)]) leads_stc = leads.filtered_domain([('team_id', '=', self.sales_team_convert.id)]) - self.assertEqual(len(leads_st1) + len(leads_stc), 100) # 2 * 2 * 75 / 30.0 - self.assertLessEqual(len(leads_st1), 100) # 2 * 2 * 75 / 30.0 - self.assertLessEqual(len(leads_stc), 66) # 2 * 2 * 90 / 30.0 + self.assertEqual(len(leads), 150) + + # check random globally assigned enough leads to team + self.assertLessEqual(len(leads_st1), 102) # 75 * 150 / 165 * 1.5 (because random) + self.assertLessEqual(len(leads_stc), 123) # 90 * 150 / 165 * 1.5 (because random) + self.assertGreaterEqual(len(leads_st1), 34) # 75 * 150 / 165 * 0.5 (because random) + self.assertGreaterEqual(len(leads_stc), 41) # 90 * 150 / 165 * 0.5 (because random) + self.assertEqual(len(leads_st1) + len(leads_stc), len(leads)) # Make sure all lead are assigned # salespersons assign self.members.invalidate_cache(fnames=['lead_month_count']) @@ -269,7 +281,7 @@ class TestLeadAssign(TestLeadAssignCommon): """ Test assignment on a more high volume oriented test set in order to test more real life use cases. """ # create leads enough to assign one month of work - _lead_count, _email_dup_count, _partner_count = 500, 50, 150 + _lead_count, _email_dup_count, _partner_count = 600, 50, 150 leads = self._create_leads_batch( lead_type='lead', user_ids=[False], @@ -283,7 +295,6 @@ class TestLeadAssign(TestLeadAssignCommon): # assign for one month, aka a lot self.env.ref('crm.ir_cron_crm_lead_assign').write({'interval_type': 'days', 'interval_number': 30}) - self.env['ir.config_parameter'].set_param('crm.assignment.bundle', '20') # create a third team sales_team_3 = self.env['crm.team'].create({ 'name': 'Sales Team 3', @@ -313,9 +324,9 @@ class TestLeadAssign(TestLeadAssignCommon): 'assignment_max': 15, 'assignment_domain': [('probability', '>=', 10)], }) - sales_teams = self.sales_teams | sales_team_3 + sales_teams = self.sales_teams + sales_team_3 self.assertEqual(sum(team.assignment_max for team in sales_teams), 300) - self.assertEqual(len(leads), 550) + self.assertEqual(len(leads), 650) # assign probability to leads (bypass auto probability as purpose is not to test pls) leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) # ensure order @@ -333,6 +344,18 @@ class TestLeadAssign(TestLeadAssignCommon): leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) self.assertEqual(leads.team_id, sales_teams) self.assertEqual(leads.user_id, sales_teams.member_ids) + self.assertEqual(len(leads), 600) + + # check random globally assigned enough leads to team + leads_st1 = leads.filtered_domain([('team_id', '=', self.sales_team_1.id)]) + leads_st2 = leads.filtered_domain([('team_id', '=', self.sales_team_convert.id)]) + leads_st3 = leads.filtered_domain([('team_id', '=', sales_team_3.id)]) + self.assertLessEqual(len(leads_st1), 225) # 75 * 600 / 300 * 1.5 (because random) + self.assertLessEqual(len(leads_st2), 270) # 90 * 600 / 300 * 1.5 (because random) + self.assertLessEqual(len(leads_st3), 405) # 135 * 600 / 300 * 1.5 (because random) + self.assertGreaterEqual(len(leads_st1), 75) # 75 * 600 / 300 * 0.5 (because random) + self.assertGreaterEqual(len(leads_st2), 90) # 90 * 600 / 300 * 0.5 (because random) + self.assertGreaterEqual(len(leads_st3), 135) # 135 * 600 / 300 * 0.5 (because random) # salespersons assign self.members.invalidate_cache(fnames=['lead_month_count']) diff --git a/addons/crm/tests/test_performances.py b/addons/crm/tests/test_performances.py index 1f1324cbb41..99786f840ce 100644 --- a/addons/crm/tests/test_performances.py +++ b/addons/crm/tests/test_performances.py @@ -24,7 +24,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon): @classmethod def setUpClass(cls): super(TestLeadAssignPerf, cls).setUpClass() - random.seed(2042) + random.seed('crm_assign') @mute_logger('odoo.models.unlink', 'odoo.addons.crm.models.crm_team', 'odoo.addons.crm.models.crm_team_member') def test_assign_perf_duplicates(self): @@ -50,7 +50,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon): leads.flush() with self.with_user('user_sales_manager'): - with self.assertQueryCount(user_sales_manager=1333): # crm only: ?? + with self.assertQueryCount(user_sales_manager=1336): # crm only: ?? self.env['crm.team'].browse(self.sales_teams.ids)._action_assign_leads(work_days=2) # teams assign @@ -87,18 +87,18 @@ class TestLeadAssignPerf(TestLeadAssignCommon): sliced_leads = leads[idx:len(leads):5] for lead in sliced_leads: lead.probability = (idx + 1) * 10 * ((int(lead.priority) + 1) / 2) + # commit probability and related fields + leads.flush() with self.with_user('user_sales_manager'): - with self.assertQueryCount(user_sales_manager=580): # crm only: 580 (seems reproducible) + with self.assertQueryCount(user_sales_manager=586): # crm only: ?? (583 often) self.env['crm.team'].browse(self.sales_teams.ids)._action_assign_leads(work_days=2) # teams assign leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) # ensure order leads_st1 = leads.filtered_domain([('team_id', '=', self.sales_team_1.id)]) leads_stc = leads.filtered_domain([('team_id', '=', self.sales_team_convert.id)]) - self.assertEqual(len(leads_st1) + len(leads_stc), 100) # 2 * 2 * 75 / 30.0 - self.assertLessEqual(len(leads_st1), 100) # 2 * 2 * 75 / 30.0 - self.assertLessEqual(len(leads_stc), 66) # 2 * 2 * 90 / 30.0 + self.assertEqual(len(leads_st1) + len(leads_stc), 100) # salespersons assign self.members.invalidate_cache(fnames=['lead_month_count']) @@ -106,14 +106,14 @@ class TestLeadAssignPerf(TestLeadAssignCommon): self.assertMemberAssign(self.sales_team_1_m2, 4) # 15 max on 2 days (1) + compensation (2.8) self.assertMemberAssign(self.sales_team_1_m3, 4) # 15 max on 2 days (1) + compensation (2.8) self.assertMemberAssign(self.sales_team_convert_m1, 8) # 30 max on 15 (2) + compensation (5.6) - self.assertMemberAssign(self.sales_team_convert_m2, 15) # 60 max on 15 (4) + compsantion (11.2) + self.assertMemberAssign(self.sales_team_convert_m2, 15) # 60 max on 15 (4) + compensation (11.2) @mute_logger('odoo.models.unlink', 'odoo.addons.crm.models.crm_team', 'odoo.addons.crm.models.crm_team_member') def test_assign_perf_populated(self): """ Test assignment on a more high volume oriented test set in order to have more insights on query counts. """ # create leads enough to have interesting counters - _lead_count, _email_dup_count, _partner_count = 500, 50, 150 + _lead_count, _email_dup_count, _partner_count = 600, 50, 150 leads = self._create_leads_batch( lead_type='lead', user_ids=[False], @@ -127,7 +127,6 @@ class TestLeadAssignPerf(TestLeadAssignCommon): # assign for one month, aka a lot self.env.ref('crm.ir_cron_crm_lead_assign').write({'interval_type': 'days', 'interval_number': 30}) - self.env['ir.config_parameter'].set_param('crm.assignment.bundle', '20') # create a third team sales_team_3 = self.env['crm.team'].create({ 'name': 'Sales Team 3', @@ -159,7 +158,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon): }) sales_teams = self.sales_teams | sales_team_3 self.assertEqual(sum(team.assignment_max for team in sales_teams), 300) - self.assertEqual(len(leads), 550) + self.assertEqual(len(leads), 650) # assign probability to leads (bypass auto probability as purpose is not to test pls) leads = self.env['crm.lead'].search([('id', 'in', leads.ids)]) # ensure order @@ -171,7 +170,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon): leads.flush() with self.with_user('user_sales_manager'): - with self.assertQueryCount(user_sales_manager=5385): # crm only: ?? + with self.assertQueryCount(user_sales_manager=5861): # crm only: ?? self.env['crm.team'].browse(sales_teams.ids)._action_assign_leads(work_days=30) # teams assign