From 86852d954ea6bf4e01722b57d568fde029d3235f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 31 Aug 2026 10:56:28 +0530 Subject: [PATCH] test: narrow shared fixture hardening (#58581) --- erpnext/hooks.py | 2 - erpnext/tests/bootstrap_test_data.py | 7 +- erpnext/tests/test_utils.py | 88 ------ erpnext/tests/utils.py | 393 ++++++++++++++------------- 4 files changed, 209 insertions(+), 281 deletions(-) delete mode 100644 erpnext/tests/test_utils.py diff --git a/erpnext/hooks.py b/erpnext/hooks.py index 88dc828bb55..51ccc25d50d 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -70,8 +70,6 @@ after_install = "erpnext.setup.install.after_install" after_app_install = "erpnext.setup.install.after_app_install" after_app_uninstall = "erpnext.setup.install.after_app_uninstall" -before_tests = "erpnext.tests.utils.bootstrap_test_data" - boot_session = "erpnext.startup.boot.boot_session" notification_config = "erpnext.startup.notifications.get_notification_config" get_help_messages = "erpnext.utilities.activation.get_help_messages" diff --git a/erpnext/tests/bootstrap_test_data.py b/erpnext/tests/bootstrap_test_data.py index 139b0537938..713c0bdf564 100644 --- a/erpnext/tests/bootstrap_test_data.py +++ b/erpnext/tests/bootstrap_test_data.py @@ -1,4 +1,3 @@ -# This file is solely to bootstrap shared test data from CI. -from erpnext.tests.utils import bootstrap_test_data - -bootstrap_test_data() +# This file is solely to trigger BootStrapTestData from CI +# utils.py module import instantiates BootStrapTestData +from erpnext.tests.utils import ERPNextTestSuite diff --git a/erpnext/tests/test_utils.py b/erpnext/tests/test_utils.py deleted file mode 100644 index dcb23fffef1..00000000000 --- a/erpnext/tests/test_utils.py +++ /dev/null @@ -1,88 +0,0 @@ -# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors -# License: GNU General Public License v3. See license.txt - -import unittest -from unittest.mock import patch - -import frappe - -from erpnext.tests.utils import ( - BootstrapTestData, - ERPNextTestSuite, - change_settings, - if_lending_app_installed, - if_lending_app_not_installed, -) - - -class TestERPNextTestUtils(ERPNextTestSuite): - def test_make_records_reuses_item_price_when_rate_changes(self): - fixture = BootstrapTestData.__new__(BootstrapTestData) - filters = {"item_code": "_Test Item", "price_list": "_Test Price List Rest of the World"} - item_price = frappe.db.get_value("Item Price", filters, "name") - self.assertIsNotNone(item_price) - frappe.db.set_value("Item Price", item_price, "price_list_rate", 999) - - fixture.make_item_price() - - self.assertEqual(frappe.db.count("Item Price", filters), 1) - self.assertEqual(frappe.db.get_value("Item Price", filters, "price_list_rate"), 10) - - def test_make_custom_doctype_repairs_each_missing_doctype(self): - fixture = BootstrapTestData.__new__(BootstrapTestData) - existing_doctypes = {"Shelf", "Rack", "Pallet", "Inv Site"} - - with ( - patch.object( - frappe.db, - "exists", - side_effect=lambda doctype, name: doctype == "DocType" and name in existing_doctypes, - ), - patch("erpnext.tests.utils.frappe.get_doc") as get_doc, - ): - fixture.make_custom_doctype() - - created_doctypes = [call.args[0]["name"] for call in get_doc.call_args_list] - self.assertCountEqual(created_doctypes, ["Store", "Order Assignment"]) - - def test_change_settings_restores_values_after_error(self): - original = frappe.db.get_single_value("Stock Settings", "auto_indent") - changed = 0 if original else 1 - - with self.assertRaisesRegex(RuntimeError, "expected failure"): - with change_settings("Stock Settings", auto_indent=changed): - self.assertEqual(frappe.db.get_single_value("Stock Settings", "auto_indent"), changed) - raise RuntimeError("expected failure") - - self.assertEqual(frappe.db.get_single_value("Stock Settings", "auto_indent"), original) - - def test_lending_decorators_preserve_names_and_skip(self): - with patch("erpnext.tests.utils.frappe.get_installed_apps", return_value=[]): - - @if_lending_app_installed - def requires_lending(): - return True - - @if_lending_app_not_installed - def excludes_lending(): - return True - - self.assertEqual(requires_lending.__name__, "requires_lending") - self.assertEqual(excludes_lending.__name__, "excludes_lending") - with self.assertRaises(unittest.SkipTest): - requires_lending() - self.assertTrue(excludes_lending()) - - with patch("erpnext.tests.utils.frappe.get_installed_apps", return_value=["lending"]): - - @if_lending_app_installed - def requires_lending(): - return True - - @if_lending_app_not_installed - def excludes_lending(): - return True - - self.assertTrue(requires_lending()) - with self.assertRaises(unittest.SkipTest): - excludes_lending() diff --git a/erpnext/tests/utils.py b/erpnext/tests/utils.py index 4f3b4c599a6..690faac0e8d 100644 --- a/erpnext/tests/utils.py +++ b/erpnext/tests/utils.py @@ -1,20 +1,19 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt -import copy import unittest from contextlib import contextmanager from typing import Any, NewType import frappe +from frappe import _ from frappe.core.doctype.report.report import get_report_module_dotted_path from frappe.custom.doctype.custom_field.custom_field import create_custom_fields from frappe.tests.utils import load_test_records_for -from frappe.utils import compare, now_datetime, today +from frappe.utils import now_datetime, today ReportFilters = dict[str, Any] ReportName = NewType("ReportName", str) -_test_data_bootstrapped = False def execute_script_report( @@ -60,20 +59,30 @@ def execute_script_report( def if_lending_app_installed(function): """Decorator to check if lending app is installed""" - return unittest.skipUnless("lending" in frappe.get_installed_apps(), "lending is not installed")(function) + + def wrapper(*args, **kwargs): + if "lending" in frappe.get_installed_apps(): + return function(*args, **kwargs) + return + + return wrapper def if_lending_app_not_installed(function): """Decorator to check if lending app is not installed""" - return unittest.skipIf("lending" in frappe.get_installed_apps(), "lending is installed")(function) + + def wrapper(*args, **kwargs): + if "lending" not in frappe.get_installed_apps(): + return function(*args, **kwargs) + return + + return wrapper -class BootstrapTestData: +class BootStrapTestData: def __init__(self): - lock_name = f"{frappe.local.site}:erpnext-test-data" - with frappe.db.advisory_lock(lock_name, timeout=300): - self.make_presets() - self.make_master_data() + self.make_presets() + self.make_master_data() def make_presets(self): from frappe.desk.page.setup_wizard.install_fixtures import update_genders, update_salutations @@ -246,56 +255,25 @@ class BootstrapTestData: stock_settings.enable_serial_and_batch_no_for_item = 1 stock_settings.save() - def make_records(self, key, records, update_fields=()): - """Create shared fixtures once and repair explicitly mutable values.""" - if not records: - return - if not key: - raise ValueError("make_records expects at least one identity field") + def make_records(self, key, records): + doctype = records[0].get("doctype") - doctypes = {record.get("doctype") for record in records} - if len(doctypes) != 1 or None in doctypes: - raise ValueError("make_records expects records for exactly one DocType") + def get_filters(record): + filters = {} + for x in key: + filters[x] = record.get(x) + return filters - doctype = doctypes.pop() - for record in records: - filters = {fieldname: record.get(fieldname) for fieldname in key} - if not any(value is not None for value in filters.values()): - raise ValueError(f"make_records expects an identity for {doctype}") - - if name := frappe.db.exists(doctype, filters): - self._update_fixture_values(doctype, name, record, update_fields) - else: - frappe.get_doc(record).insert(ignore_if_duplicate=True) - - @staticmethod - def _update_fixture_values(doctype, name, record, update_fields): - if not update_fields: - return - - doc = frappe.get_doc(doctype, name) - changed = False - for fieldname in update_fields: - if fieldname not in record: - continue - - expected = record[fieldname] - field = doc.meta.get_field(fieldname) - fieldtype = field.fieldtype if field else None - if compare(doc.get(fieldname), "=", expected, fieldtype): - continue - - doc.set(fieldname, expected) - changed = True - - if changed: - doc.save(ignore_permissions=True) + for x in records: + filters = get_filters(x) + if not frappe.db.exists(doctype, filters): + frappe.get_doc(x).insert() def make_price_list(self): records = [ { "doctype": "Price List", - "price_list_name": "Standard Buying", + "price_list_name": _("Standard Buying"), "enabled": 1, "buying": 1, "selling": 0, @@ -303,7 +281,7 @@ class BootstrapTestData: }, { "doctype": "Price List", - "price_list_name": "Standard Selling", + "price_list_name": _("Standard Selling"), "enabled": 1, "buying": 0, "selling": 1, @@ -359,11 +337,7 @@ class BootstrapTestData: "selling": 0, }, ] - self.make_records( - ["price_list_name"], - records, - update_fields=("enabled", "selling", "buying", "currency", "price_not_uom_dependant"), - ) + self.make_records(["price_list_name", "enabled", "selling", "buying", "currency"], records) def make_monthly_distribution(self): records = [ @@ -467,7 +441,7 @@ class BootstrapTestData: "parent_department": "All Departments", }, ] - self.make_records(["department_name", "company"], records) + self.make_records(["department_name"], records) def make_role(self): records = [ @@ -616,7 +590,7 @@ class BootstrapTestData: "user_id": "test2@example.com", }, ] - self.make_records(["user_id"], records) + self.make_records(["first_name"], records) def make_sales_person(self): records = [ @@ -752,11 +726,8 @@ class BootstrapTestData: } ) - self.make_records( - ["year"], - records, - update_fields=("year_start_date", "year_end_date", "is_short_year"), - ) + key = ["year_start_date", "year_end_date"] + self.make_records(key, records) def make_payment_term(self): records = [ @@ -1944,7 +1915,7 @@ class BootstrapTestData: "company": "_Test Company", }, ] - self.make_records(["item_code"], records, update_fields=("item_name",)) + self.make_records(["item_code", "item_name"], records) def make_product_bundle(self): from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle @@ -2573,7 +2544,7 @@ class BootstrapTestData: }, { "doctype": "Item Price", - "price_list": "Standard Selling", + "price_list": _("Standard Selling"), "item_code": "Loyal Item", "price_list_rate": 10000, }, @@ -2584,11 +2555,7 @@ class BootstrapTestData: "price_list_rate": 10000, }, ] - self.make_records( - ["item_code", "price_list", "customer", "supplier"], - records, - update_fields=("price_list_rate", "valid_from", "valid_upto", "uom", "packing_unit", "batch_no"), - ) + self.make_records(["item_code", "price_list", "price_list_rate"], records) def make_currency_exchange(self): """Seed current-dated USD<->INR rates so foreign-currency documents @@ -2620,16 +2587,7 @@ class BootstrapTestData: "for_selling": 1, }, ] - identity_fields = ("from_currency", "to_currency", "for_buying", "for_selling") - for record in records: - filters = {fieldname: record.get(fieldname) for fieldname in identity_fields} - name = frappe.db.get_value("Currency Exchange", filters, "name", order_by="date desc") - if name: - self._update_fixture_values( - "Currency Exchange", name, record, update_fields=("date", "exchange_rate") - ) - else: - frappe.get_doc(record).insert(ignore_if_duplicate=True) + self.make_records(["from_currency", "to_currency", "date", "for_buying", "for_selling"], records) def make_operation(self): records = [ @@ -2755,87 +2713,160 @@ class BootstrapTestData: self.make_records(["finance_book_name"], records) def make_custom_doctype(self): - for doctype, fieldname, label in ( - ("Shelf", "shelf_name", "Shelf Name"), - ("Rack", "rack_name", "Rack Name"), - ("Pallet", "pallet_name", "Pallet Name"), - ("Inv Site", "site_name", "Site Name"), - ("Store", "store_name", "Store Name"), - ): - self._make_simple_custom_doctype(doctype, fieldname, label) + if not frappe.db.exists("DocType", "Shelf"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Shelf", + "module": "Stock", + "custom": 1, + "naming_rule": "By fieldname", + "autoname": "field:shelf_name", + "fields": [{"label": "Shelf Name", "fieldname": "shelf_name", "fieldtype": "Data"}], + "permissions": [ + { + "role": "System Manager", + "permlevel": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + } + ], + } + ).insert(ignore_permissions=True) - self._make_order_assignment_doctype() + if not frappe.db.exists("DocType", "Rack"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Rack", + "module": "Stock", + "custom": 1, + "naming_rule": "By fieldname", + "autoname": "field:rack_name", + "fields": [{"label": "Rack Name", "fieldname": "rack_name", "fieldtype": "Data"}], + "permissions": [ + { + "role": "System Manager", + "permlevel": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + } + ], + } + ).insert(ignore_permissions=True) - @staticmethod - def _make_simple_custom_doctype(doctype, fieldname, label): - if frappe.db.exists("DocType", doctype): - return + if not frappe.db.exists("DocType", "Pallet"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Pallet", + "module": "Stock", + "custom": 1, + "naming_rule": "By fieldname", + "autoname": "field:pallet_name", + "fields": [{"label": "Pallet Name", "fieldname": "pallet_name", "fieldtype": "Data"}], + "permissions": [ + { + "role": "System Manager", + "permlevel": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + } + ], + } + ).insert(ignore_permissions=True) - frappe.get_doc( - { - "doctype": "DocType", - "name": doctype, - "module": "Stock", - "custom": 1, - "naming_rule": "By fieldname", - "autoname": f"field:{fieldname}", - "fields": [{"label": label, "fieldname": fieldname, "fieldtype": "Data"}], - "permissions": [ - { - "role": "System Manager", - "permlevel": 0, - "read": 1, - "write": 1, - "create": 1, - "delete": 1, - } - ], - } - ).insert(ignore_permissions=True, ignore_if_duplicate=True) + if not frappe.db.exists("DocType", "Inv Site"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Inv Site", + "module": "Stock", + "custom": 1, + "naming_rule": "By fieldname", + "autoname": "field:site_name", + "fields": [{"label": "Site Name", "fieldname": "site_name", "fieldtype": "Data"}], + "permissions": [ + { + "role": "System Manager", + "permlevel": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + } + ], + } + ).insert(ignore_permissions=True) - @staticmethod - def _make_order_assignment_doctype(): - if frappe.db.exists("DocType", "Order Assignment"): - return + if not frappe.db.exists("DocType", "Store"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Store", + "module": "Stock", + "custom": 1, + "naming_rule": "By fieldname", + "autoname": "field:store_name", + "fields": [{"label": "Store Name", "fieldname": "store_name", "fieldtype": "Data"}], + "permissions": [ + { + "role": "System Manager", + "permlevel": 0, + "read": 1, + "write": 1, + "create": 1, + "delete": 1, + } + ], + } + ).insert(ignore_permissions=True) - frappe.get_doc( - { - "doctype": "DocType", - "name": "Order Assignment", - "module": "Buying", - "custom": 1, - "autoname": "field:po", - "fields": [ - { - "label": "PO", - "fieldname": "po", - "fieldtype": "Link", - "options": "Purchase Order", - }, - { - "label": "Supplier", - "fieldname": "supplier", - "fieldtype": "Data", - "fetch_from": "po.supplier", - }, - ], - "permissions": [ - { - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "share": 1, - "write": 1, - }, - {"read": 1, "role": "Supplier"}, - ], - } - ).insert(ignore_permissions=True, ignore_if_duplicate=True) + if not frappe.db.exists("DocType", "Order Assignment"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Order Assignment", + "module": "Buying", + "custom": 1, + "autoname": "field:po", + "fields": [ + { + "label": "PO", + "fieldname": "po", + "fieldtype": "Link", + "options": "Purchase Order", + }, + { + "label": "Supplier", + "fieldname": "supplier", + "fieldtype": "Data", + "fetch_from": "po.supplier", + }, + ], + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1, + }, + {"read": 1, "role": "Supplier"}, + ], + } + ).insert(ignore_if_duplicate=True) def make_address(self): records = [ @@ -3015,21 +3046,7 @@ class BootstrapTestData: self.make_records(["store_name"], records) -# Keep the old spelling for test helpers in downstream apps. -BootStrapTestData = BootstrapTestData - - -def bootstrap_test_data(): - global _test_data_bootstrapped - if _test_data_bootstrapped: - return - - BootstrapTestData() - _test_data_bootstrapped = True - - -# Downstream apps create their fixtures while importing this module. -bootstrap_test_data() +BootStrapTestData() class ERPNextTestSuite(unittest.TestCase): @@ -3043,7 +3060,6 @@ class ERPNextTestSuite(unittest.TestCase): @classmethod def setUpClass(cls): - bootstrap_test_data() cls.globalTestRecords = {} def tearDown(self): @@ -3070,21 +3086,24 @@ class ERPNextTestSuite(unittest.TestCase): @ERPNextTestSuite.registerAs(staticmethod) @contextmanager def change_settings(doctype, settings_dict=None, /, **settings) -> None: - """Temporarily change fields in a settings DocType.""" + """Temporarily: change settings in a settings doctype.""" + import copy + if settings_dict is None: settings_dict = settings - settings_doc = frappe.get_doc(doctype) - previous_settings = {key: copy.deepcopy(settings_doc.get(key)) for key in settings_dict} + settings = frappe.get_doc(doctype) + previous_settings = copy.deepcopy(settings_dict) + for key in previous_settings: + previous_settings[key] = getattr(settings, key) for key, value in settings_dict.items(): - settings_doc.set(key, value) - settings_doc.save(ignore_permissions=True) + setattr(settings, key, value) + settings.save(ignore_permissions=True) - try: - yield - finally: - settings_doc = frappe.get_doc(doctype) - for key, value in previous_settings.items(): - settings_doc.set(key, value) - settings_doc.save(ignore_permissions=True) + yield + + settings = frappe.get_doc(doctype) + for key, value in previous_settings.items(): + setattr(settings, key, value) + settings.save(ignore_permissions=True)