From 0776f7f7fa1656f4b16a32ca414e493660d65ddb Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 26 Jun 2026 10:44:17 +0530 Subject: [PATCH] test: convert trivially-equivalent raw SQL to ORM helpers Convert test-only raw frappe.db.sql calls that have an exact ORM equivalent: full-table/filtered deletes -> frappe.db.delete, count -> frappe.db.count, row-existence assertions -> frappe.db.exists, single-row scalar fetches -> frappe.db.get_value, and simple equality/range-filter selects -> frappe.get_all. No behaviour change. Raw SQL that genuinely needs it is left as-is (dynamic identifiers, aggregates/group-by, positional as_list consumers, DB-catalog introspection). --- .../pos_invoice/test_pos_invoice_merge.py | 6 ++-- .../doctype/pos_profile/test_pos_profile.py | 16 ++++------ erpnext/manufacturing/doctype/bom/test_bom.py | 8 ++--- .../doctype/work_order/test_work_order.py | 7 ++--- .../projects/doctype/project/test_project.py | 8 ++--- .../doctype/sales_order/test_sales_order.py | 4 +-- .../material_request/test_material_request.py | 30 +++++++++---------- .../doctype/stock_entry/test_stock_entry.py | 30 ++++--------------- .../stock/doctype/warehouse/test_warehouse.py | 9 +++--- erpnext/support/doctype/issue/test_issue.py | 10 +++---- 10 files changed, 49 insertions(+), 79 deletions(-) diff --git a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice_merge.py b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice_merge.py index 36b8635e9f9..d79169c34a9 100644 --- a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice_merge.py +++ b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice_merge.py @@ -10,9 +10,9 @@ from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_pu class TestPOSInvoiceMerging(POSInvoiceTestMixin): def clear_pos_data(self): - frappe.db.sql("delete from `tabPOS Opening Entry`;") - frappe.db.sql("delete from `tabPOS Closing Entry`;") - frappe.db.sql("delete from `tabPOS Invoice`;") + frappe.db.delete("POS Opening Entry") + frappe.db.delete("POS Closing Entry") + frappe.db.delete("POS Invoice") def setUp(self): self.clear_pos_data() diff --git a/erpnext/accounts/doctype/pos_profile/test_pos_profile.py b/erpnext/accounts/doctype/pos_profile/test_pos_profile.py index 7c28504d5e8..ccae48bb116 100644 --- a/erpnext/accounts/doctype/pos_profile/test_pos_profile.py +++ b/erpnext/accounts/doctype/pos_profile/test_pos_profile.py @@ -25,15 +25,11 @@ class TestPOSProfile(ERPNextTestSuite): items = get_items_list(doc, doc.company) customers = get_customers_list(doc) - products_count = frappe.db.sql( - """ select count(name) from tabItem where item_group = '_Test Item Group'""", as_list=1 - ) - customers_count = frappe.db.sql( - """ select count(name) from tabCustomer where customer_group = '_Test Customer Group'""" - ) + products_count = frappe.db.count("Item", {"item_group": "_Test Item Group"}) + customers_count = frappe.db.count("Customer", {"customer_group": "_Test Customer Group"}) - self.assertEqual(len(items), products_count[0][0]) - self.assertEqual(len(customers), customers_count[0][0]) + self.assertEqual(len(items), products_count) + self.assertEqual(len(customers), customers_count) def test_disabled_pos_profile_creation(self): make_pos_profile(name="_Test POS Profile 001", disabled=1) @@ -135,8 +131,8 @@ def get_items_list(pos_profile, company): def make_pos_profile(**args): - frappe.db.sql("delete from `tabPOS Payment Method`") - frappe.db.sql("delete from `tabPOS Profile`") + frappe.db.delete("POS Payment Method") + frappe.db.delete("POS Profile") args = frappe._dict(args) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 041a3d3899e..b90e5769c11 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -97,10 +97,10 @@ class TestBOM(ERPNextTestSuite): update_cost_in_all_boms_in_test() # check if new valuation rate updated in all BOMs - for d in frappe.db.sql( - """select base_rate from `tabBOM Item` - where item_code='_Test Item 2' and docstatus=1 and parenttype='BOM'""", - as_dict=1, + for d in frappe.get_all( + "BOM Item", + filters={"item_code": "_Test Item 2", "docstatus": 1, "parenttype": "BOM"}, + fields=["base_rate"], ): self.assertEqual(d.base_rate, rm_base_rate + 10) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 34ba1696768..0a4bbcddd2a 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -5249,11 +5249,8 @@ def update_job_card(job_card, jc_qty=None, days=None): def get_secondary_item_details(bom_no): secondary_items = {} - for item in frappe.db.sql( - """select item_code, stock_qty from `tabBOM Secondary Item` - where parent = %s""", - bom_no, - as_dict=1, + for item in frappe.get_all( + "BOM Secondary Item", filters={"parent": bom_no}, fields=["item_code", "stock_qty"] ): secondary_items[item.item_code] = item.stock_qty diff --git a/erpnext/projects/doctype/project/test_project.py b/erpnext/projects/doctype/project/test_project.py index 1defd5f4733..90e8d78f60e 100644 --- a/erpnext/projects/doctype/project/test_project.py +++ b/erpnext/projects/doctype/project/test_project.py @@ -49,7 +49,7 @@ class TestProject(ERPNextTestSuite): def test_project_with_template_having_no_parent_and_depend_tasks(self): project_name = "Test Project with Template - No Parent and Dependend Tasks" - frappe.db.sql(""" delete from tabTask where project = %s """, project_name) + frappe.db.delete("Task", {"project": project_name}) frappe.delete_doc("Project", project_name) task1 = task_exists("Test Template Task with No Parent and Dependency") @@ -82,7 +82,7 @@ class TestProject(ERPNextTestSuite): if frappe.db.get_value("Project", {"project_name": project_name}, "name"): project_name = frappe.db.get_value("Project", {"project_name": project_name}, "name") - frappe.db.sql(""" delete from tabTask where project = %s """, project_name) + frappe.db.delete("Task", {"project": project_name}) frappe.delete_doc("Project", project_name) task1 = task_exists("Test Template Task Parent") @@ -137,7 +137,7 @@ class TestProject(ERPNextTestSuite): def test_project_template_having_dependent_tasks(self): project_name = "Test Project with Template - Dependent Tasks" - frappe.db.sql(""" delete from tabTask where project = %s """, project_name) + frappe.db.delete("Task", {"project": project_name}) frappe.delete_doc("Project", project_name) task1 = task_exists("Test Template Task for Dependency") @@ -252,7 +252,7 @@ class TestProject(ERPNextTestSuite): def test_project_having_no_tasks_complete(self): project_name = "Test Project - No Tasks Completion" - frappe.db.sql(""" delete from tabTask where project = %s """, project_name) + frappe.db.delete("Task", {"project": project_name}) frappe.delete_doc("Project", project_name) project = frappe.get_doc( diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 052ee574b72..c1c1e513857 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -1742,9 +1742,7 @@ class TestSalesOrder(ERPNextTestSuite): mr_dict["include_exploded_items"] = 0 mr_dict["ignore_existing_ordered_qty"] = 1 make_raw_material_request(mr_dict, so.company, so.name) - mr = frappe.db.sql( - """select name from `tabMaterial Request` ORDER BY creation DESC LIMIT 1""", as_dict=1 - )[0] + mr = frappe.get_all("Material Request", fields=["name"], order_by="creation desc", limit=1)[0] mr_doc = frappe.get_doc("Material Request", mr.get("name")) self.assertEqual(mr_doc.items[0].sales_order, so.name) diff --git a/erpnext/stock/doctype/material_request/test_material_request.py b/erpnext/stock/doctype/material_request/test_material_request.py index c6e63269c1f..faec072513e 100644 --- a/erpnext/stock/doctype/material_request/test_material_request.py +++ b/erpnext/stock/doctype/material_request/test_material_request.py @@ -746,11 +746,11 @@ class TestMaterialRequest(ERPNextTestSuite): mr = frappe.get_doc("Material Request", mr.name) mr.submit() completed_qty = mr.items[0].ordered_qty - requested_qty = frappe.db.sql( - """select indented_qty from `tabBin` where \ - item_code= %s and warehouse= %s """, - (mr.items[0].item_code, mr.items[0].warehouse), - )[0][0] + requested_qty = frappe.db.get_value( + "Bin", + {"item_code": mr.items[0].item_code, "warehouse": mr.items[0].warehouse}, + "indented_qty", + ) prod_order = raise_work_orders(mr.name, mr.company) po = frappe.get_doc("Work Order", prod_order[0]) @@ -760,11 +760,11 @@ class TestMaterialRequest(ERPNextTestSuite): mr = frappe.get_doc("Material Request", mr.name) self.assertEqual(completed_qty + po.qty, mr.items[0].ordered_qty) - new_requested_qty = frappe.db.sql( - """select indented_qty from `tabBin` where \ - item_code= %s and warehouse= %s """, - (mr.items[0].item_code, mr.items[0].warehouse), - )[0][0] + new_requested_qty = frappe.db.get_value( + "Bin", + {"item_code": mr.items[0].item_code, "warehouse": mr.items[0].warehouse}, + "indented_qty", + ) self.assertEqual(requested_qty - po.qty, new_requested_qty) @@ -773,11 +773,11 @@ class TestMaterialRequest(ERPNextTestSuite): mr = frappe.get_doc("Material Request", mr.name) self.assertEqual(completed_qty, mr.items[0].ordered_qty) - new_requested_qty = frappe.db.sql( - """select indented_qty from `tabBin` where \ - item_code= %s and warehouse= %s """, - (mr.items[0].item_code, mr.items[0].warehouse), - )[0][0] + new_requested_qty = frappe.db.get_value( + "Bin", + {"item_code": mr.items[0].item_code, "warehouse": mr.items[0].warehouse}, + "indented_qty", + ) self.assertEqual(requested_qty, new_requested_qty) def test_requested_qty_multi_uom(self): diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 5dcad431d21..3088a8cfaca 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -269,20 +269,10 @@ class TestStockEntry(ERPNextTestSuite): mr.cancel() self.assertTrue( - frappe.db.sql( - """select * from `tabStock Ledger Entry` - where voucher_type='Stock Entry' and voucher_no=%s""", - mr.name, - ) + frappe.db.exists("Stock Ledger Entry", {"voucher_type": "Stock Entry", "voucher_no": mr.name}) ) - self.assertTrue( - frappe.db.sql( - """select * from `tabGL Entry` - where voucher_type='Stock Entry' and voucher_no=%s""", - mr.name, - ) - ) + self.assertTrue(frappe.db.exists("GL Entry", {"voucher_type": "Stock Entry", "voucher_no": mr.name})) def test_material_issue_gl_entry(self): company = frappe.db.get_value("Warehouse", "Stores - TCP1", "company") @@ -361,12 +351,7 @@ class TestStockEntry(ERPNextTestSuite): if source_warehouse_account == target_warehouse_account: # no gl entry as both source and target warehouse has linked to same account. self.assertFalse( - frappe.db.sql( - """select * from `tabGL Entry` - where voucher_type='Stock Entry' and voucher_no=%s""", - mtn.name, - as_dict=1, - ) + frappe.db.exists("GL Entry", {"voucher_type": "Stock Entry", "voucher_no": mtn.name}) ) else: @@ -460,14 +445,9 @@ class TestStockEntry(ERPNextTestSuite): ], ) - gl_entries = frappe.db.sql( - """select account, debit, credit - from `tabGL Entry` where voucher_type='Stock Entry' and voucher_no=%s - order by account desc""", - repack.name, - as_dict=1, + self.assertFalse( + frappe.db.exists("GL Entry", {"voucher_type": "Stock Entry", "voucher_no": repack.name}) ) - self.assertFalse(gl_entries) def test_repack_with_additional_costs(self): company = frappe.db.get_value("Warehouse", "Stores - TCP1", "company") diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 5a36d5c213e..87519bb8de8 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -19,11 +19,10 @@ class TestWarehouse(ERPNextTestSuite): def test_warehouse_hierarchy(self): p_warehouse = frappe.get_doc("Warehouse", "_Test Warehouse Group - _TC") - child_warehouses = frappe.db.sql( - """select name, is_group, parent_warehouse from `tabWarehouse` wh - where wh.lft > %s and wh.rgt < %s""", - (p_warehouse.lft, p_warehouse.rgt), - as_dict=1, + child_warehouses = frappe.get_all( + "Warehouse", + filters={"lft": [">", p_warehouse.lft], "rgt": ["<", p_warehouse.rgt]}, + fields=["name", "is_group", "parent_warehouse"], ) for child_warehouse in child_warehouses: diff --git a/erpnext/support/doctype/issue/test_issue.py b/erpnext/support/doctype/issue/test_issue.py index 371a9449e8f..9403550482b 100644 --- a/erpnext/support/doctype/issue/test_issue.py +++ b/erpnext/support/doctype/issue/test_issue.py @@ -14,11 +14,11 @@ from erpnext.tests.utils import ERPNextTestSuite class TestSetUp(ERPNextTestSuite): def setUp(self): - frappe.db.sql("delete from `tabService Level Agreement`") - frappe.db.sql("delete from `tabService Level Priority`") - frappe.db.sql("delete from `tabSLA Fulfilled On Status`") - frappe.db.sql("delete from `tabPause SLA On Status`") - frappe.db.sql("delete from `tabService Day`") + frappe.db.delete("Service Level Agreement") + frappe.db.delete("Service Level Priority") + frappe.db.delete("SLA Fulfilled On Status") + frappe.db.delete("Pause SLA On Status") + frappe.db.delete("Service Day") frappe.db.set_single_value("Support Settings", "track_service_level_agreement", 1) create_service_level_agreements_for_issues()