From 503ed0502995f871f47e8074f9ea150dd6ddf7b4 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Mon, 3 Oct 2022 14:40:25 +0000 Subject: [PATCH] [IMP] tests: add generic Basecase.start for patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using patcher.start() can easily lead to incorrect cleanup. -> after a copy paste, patcher is working, but stop is forgotten -> stop is present, but won't be called if something fails during the test This commit add an utility `start(patcher)` to always have the add cleanup. Using a standard way to start the patcher with an automated addCleanup should prevent this kind of mistake. This is why this commit also replaces all valid patch.start() (followed immediately by a addCleanup) closes odoo/odoo#102873 X-original-commit: 7d5a193d86316965a0908c65cfacfb607dc3f3ad Related: odoo/enterprise#32618 Signed-off-by: Christophe Monniez (moc) Signed-off-by: Xavier Dollé (xdo) --- addons/account_payment/tests/common.py | 5 +---- addons/bus/tests/common.py | 3 +-- addons/crm/tests/common.py | 7 +------ addons/gamification/tests/test_karma_tracking.py | 14 ++++---------- addons/link_tracker/tests/common.py | 3 +-- addons/payment/tests/common.py | 11 +++-------- .../tests/test_purchase_lead_time.py | 3 +-- addons/purchase_stock/tests/test_stockvaluation.py | 14 ++++---------- addons/test_base_automation/tests/test_flow.py | 6 ++---- addons/web/tests/test_assets_xml.py | 6 +----- addons/web/tests/test_db_manager.py | 3 +-- addons/web/tests/test_profiler.py | 4 +--- addons/website/tests/test_configurator.py | 6 ++---- .../tests/test_partner_assign.py | 3 +-- addons/website_links/tests/test_ui.py | 3 +-- .../tests/test_website_sale_pricelist.py | 6 ++---- odoo/addons/base/tests/test_module.py | 3 +-- odoo/addons/test_populate/tests/test_populate.py | 3 +-- odoo/tests/common.py | 11 +++++++++++ 19 files changed, 40 insertions(+), 74 deletions(-) diff --git a/addons/account_payment/tests/common.py b/addons/account_payment/tests/common.py index f7326ee8ab1..43b6bf69799 100644 --- a/addons/account_payment/tests/common.py +++ b/addons/account_payment/tests/common.py @@ -53,11 +53,8 @@ class AccountPaymentCommon(PaymentCommon, AccountTestInvoicingCommon): }) def setUp(self): + self.enable_reconcile_after_done_patcher = False super().setUp() - # Disable _reconcile_after_done patcher - self.reconcile_after_done_patcher.stop() - self.is_patcher_started = False - #=== Utils ===# @classmethod diff --git a/addons/bus/tests/common.py b/addons/bus/tests/common.py index 0e04ae9b30b..c11959e4b05 100644 --- a/addons/bus/tests/common.py +++ b/addons/bus/tests/common.py @@ -43,8 +43,7 @@ class WebsocketCase(common.HttpCase): '_serve_forever', wraps=_mocked_serve_forever ) - self._serve_forever_patch.start() - self.addCleanup(self._serve_forever_patch.stop) + self.startPatcher(self._serve_forever_patch) def tearDown(self): self._close_websockets() diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 23c93498e7e..b6270c4b147 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -574,12 +574,7 @@ class TestLeadConvertCommon(TestCrmCommon): cls.lead_1.write({'date_open': Datetime.from_string('2020-01-15 11:30:00')}) cls.crm_lead_dt_patcher = patch('odoo.addons.crm.models.crm_lead.fields.Datetime', wraps=Datetime) - cls.crm_lead_dt_mock = cls.crm_lead_dt_patcher.start() - - @classmethod - def tearDownClass(cls): - cls.crm_lead_dt_patcher.stop() - super(TestLeadConvertCommon, cls).tearDownClass() + cls.crm_lead_dt_mock = cls.startClassPatcher(cls.crm_lead_dt_patcher) @classmethod def _switch_to_multi_membership(cls): diff --git a/addons/gamification/tests/test_karma_tracking.py b/addons/gamification/tests/test_karma_tracking.py index 12946435ad7..75a210fa012 100644 --- a/addons/gamification/tests/test_karma_tracking.py +++ b/addons/gamification/tests/test_karma_tracking.py @@ -79,7 +79,7 @@ class TestKarmaTrackingCommon(common.TransactionCase): def test_consolidation_cron(self): self.patcher = patch('odoo.addons.gamification.models.gamification_karma_tracking.fields.Date', wraps=fields.Date) - self.mock_datetime = self.patcher.start() + self.mock_datetime = self.startPatcher(self.patcher) self.mock_datetime.today.return_value = date(self.test_date.year, self.test_date.month + 1, self.test_date.day) self._create_trackings(self.test_user, 20, 2, self.test_date, days_delta=30) @@ -97,8 +97,6 @@ class TestKarmaTrackingCommon(common.TransactionCase): ]) self.assertEqual(len(unconsolidated), 6) # 5 for test user 2, 1 for test user - self.patcher.stop() - def test_consolidation_monthly(self): Tracking = self.env['gamification.karma.tracking'] base_test_user_karma = self.test_user.karma @@ -204,7 +202,7 @@ class TestComputeRankCommon(common.TransactionCase): pass patch_email = patch('odoo.addons.mail.models.mail_template.MailTemplate.send_mail', _patched_send_mail) - patch_email.start() + cls.startClassPatcher(patch_email) cls.users = cls.env['res.users'] for k in range(-5, 1030, 30): @@ -236,8 +234,6 @@ class TestComputeRankCommon(common.TransactionCase): 'karma_min': 1000, }) - patch_email.stop() - def test_00_initial_compute(self): self.assertEqual(len(self.users), 35) @@ -291,10 +287,9 @@ class TestComputeRankCommon(common.TransactionCase): number_of_users = len(_self & self.users) patch_bulk = patch('odoo.addons.gamification.models.res_users.Users._recompute_rank', _patched_recompute_rank) - patch_bulk.start() + self.startPatcher(patch_bulk) self.rank_3.karma_min = 700 self.assertEqual(number_of_users, 7, "Should just recompute for the 7 users between 500 and 700") - patch_bulk.stop() def test_03_test_bulk_call(self): self.assertEqual(len(self.users), 35) @@ -303,7 +298,7 @@ class TestComputeRankCommon(common.TransactionCase): raise patch_bulk = patch('odoo.addons.gamification.models.res_users.Users._recompute_rank_bulk', _patched_check_in_bulk) - patch_bulk.start() + self.startPatcher(patch_bulk) # call on 5 users should not trigger the bulk function self.users[0:5]._recompute_rank() @@ -312,4 +307,3 @@ class TestComputeRankCommon(common.TransactionCase): with self.assertRaises(Exception): self.users[0:50]._recompute_rank() - patch_bulk.stop() diff --git a/addons/link_tracker/tests/common.py b/addons/link_tracker/tests/common.py index 1de5ea1b3c5..f23cc30de48 100644 --- a/addons/link_tracker/tests/common.py +++ b/addons/link_tracker/tests/common.py @@ -18,8 +18,7 @@ class MockLinkTracker(common.BaseCase): return "Test_TITLE" link_tracker_title_patch = patch('odoo.addons.link_tracker.models.link_tracker.LinkTracker._get_title_from_url', wraps=_get_title_from_url) - link_tracker_title_patch.start() - self.addCleanup(link_tracker_title_patch.stop) + self.startPatcher(link_tracker_title_patch) def _get_href_from_anchor_id(self, body, anchor_id): """ Parse en html body to find the href of an element given its ID. """ diff --git a/addons/payment/tests/common.py b/addons/payment/tests/common.py index fe0cd679bce..9b9386010be 100644 --- a/addons/payment/tests/common.py +++ b/addons/payment/tests/common.py @@ -91,22 +91,17 @@ class PaymentCommon(TransactionCase): account_payment_module = cls.env['ir.module.module']._get('account_payment') cls.account_payment_installed = account_payment_module.state in ('installed', 'to upgrade') + cls.enable_reconcile_after_done_patcher = True def setUp(self): - def stop_patcher_without_fail(): - if self.is_patcher_started: - self.reconcile_after_done_patcher.stop() - super().setUp() - if self.account_payment_installed: + if self.account_payment_installed and self.enable_reconcile_after_done_patcher: # disable account payment generation if account_payment is installed # because the accounting setup of providers is not managed in this common self.reconcile_after_done_patcher = patch( 'odoo.addons.account_payment.models.payment_transaction.PaymentTransaction._reconcile_after_done', ) - self.reconcile_after_done_patcher.start() - self.is_patcher_started = True - self.addCleanup(stop_patcher_without_fail) + self.startPatcher(self.reconcile_after_done_patcher) #=== Utils ===# diff --git a/addons/purchase_stock/tests/test_purchase_lead_time.py b/addons/purchase_stock/tests/test_purchase_lead_time.py index 329fd82c014..852b5501b3f 100644 --- a/addons/purchase_stock/tests/test_purchase_lead_time.py +++ b/addons/purchase_stock/tests/test_purchase_lead_time.py @@ -274,7 +274,7 @@ class TestPurchaseLeadTime(PurchaseTestCommon): }) company.write({'po_lead': 0.00}) self.patcher = patch('odoo.addons.stock.models.stock_orderpoint.fields.Date', wraps=fields.Date) - self.mock_date = self.patcher.start() + self.mock_date = self.startPatcher(self.patcher) vendor = self.env['res.partner'].create({ 'name': 'Colruyt' @@ -360,7 +360,6 @@ class TestPurchaseLeadTime(PurchaseTestCommon): new_order = po_line.order_id.sorted('date_order')[-1] self.assertEqual(fields.Date.to_date(new_order.date_order), fields.Date.today() + timedelta(days=2)) self.assertEqual(new_order.order_line.product_uom_qty, 5.0) - self.patcher.stop() def test_supplier_lead_time(self): """ Basic stock configuration and a supplier with a minimum qty and a lead time """ diff --git a/addons/purchase_stock/tests/test_stockvaluation.py b/addons/purchase_stock/tests/test_stockvaluation.py index 2e42a118a79..630e8cc3768 100644 --- a/addons/purchase_stock/tests/test_stockvaluation.py +++ b/addons/purchase_stock/tests/test_stockvaluation.py @@ -692,8 +692,8 @@ class TestStockValuationWithCOA(AccountTestInvoicingCommon): patch('odoo.fields.Date.context_today', _today), ] - for p in patchers: - p.start() + for patcher in patchers: + self.startPatcher(patcher) # Proceed po = self.env['purchase.order'].create({ @@ -755,9 +755,6 @@ class TestStockValuationWithCOA(AccountTestInvoicingCommon): inv.action_post() - for p in patchers: - p.stop() - move_lines = inv.line_ids self.assertEqual(len(move_lines), 3) @@ -867,8 +864,8 @@ class TestStockValuationWithCOA(AccountTestInvoicingCommon): patch('odoo.fields.Datetime.now', _now), ] - for p in patchers: - p.start() + for patcher in patchers: + self.startPatcher(patcher) # Proceed po = self.env['purchase.order'].create({ @@ -921,9 +918,6 @@ class TestStockValuationWithCOA(AccountTestInvoicingCommon): inv.action_post() - for p in patchers: - p.stop() - self.assertRecordValues(inv.line_ids, [ # pylint: disable=C0326 {'balance': 15.0, 'amount_currency': 30.0, 'account_id': self.stock_input_account.id}, diff --git a/addons/test_base_automation/tests/test_flow.py b/addons/test_base_automation/tests/test_flow.py index 7304a98a6ec..e62f8502799 100644 --- a/addons/test_base_automation/tests/test_flow.py +++ b/addons/test_base_automation/tests/test_flow.py @@ -221,21 +221,19 @@ record['name'] = record.name + 'X'""", patch('odoo.addons.mail.models.mail_template.MailTemplate.send_mail', _patched_send_mail), ] - patchers[0].start() + self.startPatcher(patchers[0]) lead = self.create_lead() self.assertFalse(lead.priority) self.assertFalse(lead.deadline) - patchers[1].start() + self.startPatcher(patchers[1]) lead.write({'priority': True}) self.assertTrue(lead.priority) self.assertTrue(lead.deadline) - for patcher in patchers: - patcher.stop() self.assertEqual(send_mail_count, 1) diff --git a/addons/web/tests/test_assets_xml.py b/addons/web/tests/test_assets_xml.py index cf2c513f5c7..5c5866c0bbd 100644 --- a/addons/web/tests/test_assets_xml.py +++ b/addons/web/tests/test_assets_xml.py @@ -58,11 +58,7 @@ class TestStaticInheritanceCommon(odoo.tests.TransactionCase): """, } self._patch = patch.object(WebAsset, '_fetch_content', lambda asset: self.template_files[asset.url]) - self._patch.start() - - def tearDown(self): - super().tearDown() - self._patch.stop() + self.startPatcher(self._patch) def renderBundle(self, debug=False): files = [] diff --git a/addons/web/tests/test_db_manager.py b/addons/web/tests/test_db_manager.py index 51c3d8139f1..9af81f4f892 100644 --- a/addons/web/tests/test_db_manager.py +++ b/addons/web/tests/test_db_manager.py @@ -39,8 +39,7 @@ class TestDatabaseOperations(BaseCase): self.verify_admin_password_patcher = patch( 'odoo.tools.config.verify_admin_password', self.password.__eq__, ) - self.verify_admin_password_patcher.start() - self.addCleanup(self.verify_admin_password_patcher.stop) + self.startPatcher(self.verify_admin_password_patcher) self.db_name = config['db_name'] self.assertTrue(self.db_name) diff --git a/addons/web/tests/test_profiler.py b/addons/web/tests/test_profiler.py index 7ef1b3b5f3b..8455ac132dd 100644 --- a/addons/web/tests/test_profiler.py +++ b/addons/web/tests/test_profiler.py @@ -19,8 +19,7 @@ class ProfilingHttpCase(HttpCase): # its actual cursor), which prevents the profiling data from being # committed for real. cls.patcher = patch('odoo.sql_db.db_connect', return_value=cls.registry) - cls.patcher.start() - cls.addClassCleanup(cls.patcher.stop) + cls.startClassPatcher(cls.patcher) def profile_rpc(self, params=None): params = params or {} @@ -37,7 +36,6 @@ class ProfilingHttpCase(HttpCase): req.raise_for_status() return req.json() - @tagged('post_install', '-at_install', 'profiling') class TestProfilingWeb(ProfilingHttpCase): def test_profiling_enabled(self): diff --git a/addons/website/tests/test_configurator.py b/addons/website/tests/test_configurator.py index 5cf817e609a..fbed6b4bf02 100644 --- a/addons/website/tests/test_configurator.py +++ b/addons/website/tests/test_configurator.py @@ -45,12 +45,10 @@ class TestConfiguratorCommon(odoo.tests.HttpCase): iap_jsonrpc_mocked() iap_patch = patch('odoo.addons.iap.tools.iap_tools.iap_jsonrpc', iap_jsonrpc_mocked_configurator) - iap_patch.start() - self.addCleanup(iap_patch.stop) + self.startPatcher(iap_patch) patcher = patch('odoo.addons.website.models.ir_module_module.IrModuleModule._theme_upgrade_upstream', wraps=self._theme_upgrade_upstream) - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) @odoo.tests.common.tagged('post_install', '-at_install') class TestConfiguratorTranslation(TestConfiguratorCommon): diff --git a/addons/website_crm_partner_assign/tests/test_partner_assign.py b/addons/website_crm_partner_assign/tests/test_partner_assign.py index 03519d9e4b3..2dc127d5952 100644 --- a/addons/website_crm_partner_assign/tests/test_partner_assign.py +++ b/addons/website_crm_partner_assign/tests/test_partner_assign.py @@ -34,8 +34,7 @@ class TestPartnerAssign(TransactionCase): }.get(addr) patcher = patch('odoo.addons.base_geolocalize.models.base_geocoder.GeoCoder.geo_find', wraps=geo_find) - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) def test_opportunity_count(self): self.customer_uk.write({ diff --git a/addons/website_links/tests/test_ui.py b/addons/website_links/tests/test_ui.py index 883eec49e88..ede9fa6d026 100644 --- a/addons/website_links/tests/test_ui.py +++ b/addons/website_links/tests/test_ui.py @@ -15,8 +15,7 @@ class TestUi(odoo.tests.HttpCase): return 'Contact Us | My Website' patcher = patch('odoo.addons.link_tracker.models.link_tracker.LinkTracker._get_title_from_url', wraps=_get_title_from_url) - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) def test_01_test_ui(self): self.env['link.tracker'].search_or_create({ diff --git a/addons/website_sale/tests/test_website_sale_pricelist.py b/addons/website_sale/tests/test_website_sale_pricelist.py index 1214f1907ff..c16e3816e37 100644 --- a/addons/website_sale/tests/test_website_sale_pricelist.py +++ b/addons/website_sale/tests/test_website_sale_pricelist.py @@ -103,8 +103,7 @@ class TestWebsitePriceList(TransactionCase): 'current_pl': False, } patcher = patch('odoo.addons.website_sale.models.website.Website.get_pricelist_available', wraps=self._get_pricelist_available) - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) # Mock nedded because request.session doesn't exist during test def _get_pricelist_available(self, show_visible=False): @@ -284,8 +283,7 @@ def simulate_frontend_context(self, website_id=1): def get_request_website(): return self.env['website'].browse(website_id) patcher = patch('odoo.addons.website.models.ir_http.get_request_website', wraps=get_request_website) - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) @tagged('post_install', '-at_install') diff --git a/odoo/addons/base/tests/test_module.py b/odoo/addons/base/tests/test_module.py index a1f78138fdb..d4d774fdffe 100644 --- a/odoo/addons/base/tests/test_module.py +++ b/odoo/addons/base/tests/test_module.py @@ -19,8 +19,7 @@ class TestModuleManifest(BaseCase): cls.addons_path = cls._tmp_dir.name patcher = patch.object(odoo.addons, '__path__', [cls.addons_path]) - patcher.start() - cls.addClassCleanup(patcher.stop) + cls.startClassPatcher(patcher) def setUp(self): self.module_root = tempfile.mkdtemp(prefix='odoo-test-module-', dir=self.addons_path) diff --git a/odoo/addons/test_populate/tests/test_populate.py b/odoo/addons/test_populate/tests/test_populate.py index 1cd71ec20f8..40529a663f1 100644 --- a/odoo/addons/test_populate/tests/test_populate.py +++ b/odoo/addons/test_populate/tests/test_populate.py @@ -15,8 +15,7 @@ class TestPopulate(common.TransactionCase): def setUp(self): super(TestPopulate, self).setUp() patcher = patch.object(self.cr, 'commit') - patcher.start() - self.addCleanup(patcher.stop) + self.startPatcher(patcher) def test_dependency(self): ordered_models = Populate._get_ordered_models(self.env, ['test.populate']) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index f97085d873a..1a1ebec3ec8 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -465,6 +465,17 @@ class BaseCase(unittest.TestCase, metaclass=MetaCase): patcher.start() cls.addClassCleanup(patcher.stop) + def startPatcher(self, patcher): + mock = patcher.start() + self.addCleanup(patcher.stop) + return mock + + @classmethod + def startClassPatcher(cls, patcher): + mock = patcher.start() + cls.addClassCleanup(patcher.stop) + return mock + @contextmanager def with_user(self, login): """ Change user for a given test, like with self.with_user() ... """