From 6bde1bb5d2446c3ed08f566060c321664ad1d4e4 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 7 Jun 2022 14:44:00 +0530 Subject: [PATCH] test: Util to update cost in all BOMs - Utility to update cost in all BOMs without cron jobs or background jobs (run immediately) - Re-use util wherever all bom costs are to be updated - Skip explicit commits if in test - Specify company in test records (dirty data sometimes, company wh mismatch) - Skip background jobs queueing if in test --- erpnext/manufacturing/doctype/bom/test_bom.py | 6 +- .../doctype/bom/test_records.json | 1 + .../doctype/bom_update_log/bom_update_log.py | 21 +++- .../bom_update_log/bom_updation_utils.py | 10 +- .../bom_update_log/test_bom_update_log.py | 97 ++++++++++++------- .../bom_update_tool/test_bom_update_tool.py | 14 +-- 6 files changed, 97 insertions(+), 52 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 62fc0724e0..bc1bea7389 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -11,7 +11,9 @@ from frappe.utils import cstr, flt from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order from erpnext.manufacturing.doctype.bom.bom import item_query, make_variant_bom -from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost +from erpnext.manufacturing.doctype.bom_update_log.test_bom_update_log import ( + update_cost_in_all_boms_in_test, +) from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( create_stock_reconciliation, @@ -80,7 +82,7 @@ class TestBOM(FrappeTestCase): reset_item_valuation_rate(item_code="_Test Item 2", qty=200, rate=rm_rate + 10) # update cost of all BOMs based on latest valuation rate - update_cost() + update_cost_in_all_boms_in_test() # check if new valuation rate updated in all BOMs for d in frappe.db.sql( diff --git a/erpnext/manufacturing/doctype/bom/test_records.json b/erpnext/manufacturing/doctype/bom/test_records.json index 25730f9b9f..507d319b51 100644 --- a/erpnext/manufacturing/doctype/bom/test_records.json +++ b/erpnext/manufacturing/doctype/bom/test_records.json @@ -32,6 +32,7 @@ "is_active": 1, "is_default": 1, "item": "_Test Item Home Desktop Manufactured", + "company": "_Test Company", "quantity": 1.0 }, { diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py index d714b9d5fd..71430bd57e 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -1,7 +1,7 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt import json -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple, Union import frappe from frappe import _ @@ -101,12 +101,14 @@ def run_replace_bom_job( handle_exception(doc) finally: frappe.db.auto_commit_on_many_writes = 0 - frappe.db.commit() # nosemgrep + + if not frappe.flags.in_test: + frappe.db.commit() # nosemgrep def process_boms_cost_level_wise( update_doc: "BOMUpdateLog", parent_boms: List[str] = None -) -> None: +) -> Union[None, Tuple]: "Queue jobs at the start of new BOM Level in 'Update Cost' Jobs." current_boms = {} @@ -133,6 +135,10 @@ def process_boms_cost_level_wise( values = {"current_level": current_level} set_values_in_log(update_doc.name, values, commit=True) + + if frappe.flags.in_test: + return current_boms, current_level + queue_bom_cost_jobs(current_boms, update_doc, current_level) @@ -155,6 +161,10 @@ def queue_bom_cost_jobs( ) batch_row.db_insert() + if frappe.flags.in_test: + # skip background jobs in test + return boms_to_process, batch_row.name + frappe.enqueue( method="erpnext.manufacturing.doctype.bom_update_log.bom_updation_utils.update_cost_in_level", doc=update_doc, @@ -216,7 +226,10 @@ def resume_bom_cost_update_jobs(): def get_processed_current_boms( log: Dict[str, Any], bom_batches: Dict[str, Any] ) -> Tuple[List[str], Dict[str, Any]]: - "Aggregate all BOMs from BOM Update Batch rows into 'processed_boms' field and into current boms list." + """ + Aggregate all BOMs from BOM Update Batch rows into 'processed_boms' field + and into current boms list. + """ processed_boms = json.loads(log.processed_boms) if log.processed_boms else {} current_boms = [] diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py b/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py index 49e747c4bb..dde1e4ed75 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py @@ -63,7 +63,9 @@ def update_cost_in_level( handle_exception(doc) finally: frappe.db.auto_commit_on_many_writes = 0 - frappe.db.commit() # nosemgrep + + if not frappe.flags.in_test: + frappe.db.commit() # nosemgrep def get_ancestor_boms(new_bom: str, bom_list: Optional[List] = None) -> List: @@ -119,8 +121,8 @@ def update_cost_in_boms(bom_list: List[str]) -> None: bom_doc.calculate_cost(save_updates=True, update_hour_rate=True) bom_doc.db_update() - if index % 100 == 0: - frappe.db.commit() + if (index % 100 == 0) and not frappe.flags.in_test: + frappe.db.commit() # nosemgrep def get_next_higher_level_boms( @@ -210,7 +212,7 @@ def set_values_in_log(log_name: str, values: Dict[str, Any], commit: bool = Fals query = query.set(key, value) query.run() - if commit: + if commit and not frappe.flags.in_test: frappe.db.commit() # nosemgrep diff --git a/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py index 4f151334a2..d770f6c56a 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py @@ -1,14 +1,27 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +import json + import frappe from frappe.tests.utils import FrappeTestCase from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import ( BOMMissingError, + get_processed_current_boms, + process_boms_cost_level_wise, + queue_bom_cost_jobs, run_replace_bom_job, ) -from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import enqueue_replace_bom +from erpnext.manufacturing.doctype.bom_update_log.bom_updation_utils import ( + get_next_higher_level_boms, + set_values_in_log, + update_cost_in_level, +) +from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import ( + enqueue_replace_bom, + enqueue_update_cost, +) test_records = frappe.get_test_records("BOM") @@ -31,17 +44,12 @@ class TestBOMUpdateLog(FrappeTestCase): def tearDown(self): frappe.db.rollback() - if self._testMethodName == "test_bom_update_log_completion": - # clear logs and delete BOM created via setUp - frappe.db.delete("BOM Update Log") - self.new_bom_doc.cancel() - self.new_bom_doc.delete() - - # explicitly commit and restore to original state - frappe.db.commit() # nosemgrep - def test_bom_update_log_validate(self): - "Test if BOM presence is validated." + """ + 1) Test if BOM presence is validated. + 2) Test if same BOMs are validated. + 3) Test of non-existent BOM is validated. + """ with self.assertRaises(BOMMissingError): enqueue_replace_bom(boms={}) @@ -55,9 +63,7 @@ class TestBOMUpdateLog(FrappeTestCase): def test_bom_update_log_queueing(self): "Test if BOM Update Log is created and queued." - log = enqueue_replace_bom( - boms=self.boms, - ) + log = enqueue_replace_bom(boms=self.boms) self.assertEqual(log.docstatus, 1) self.assertEqual(log.status, "Queued") @@ -65,32 +71,51 @@ class TestBOMUpdateLog(FrappeTestCase): def test_bom_update_log_completion(self): "Test if BOM Update Log handles job completion correctly." - log = enqueue_replace_bom( - boms=self.boms, - ) + log = enqueue_replace_bom(boms=self.boms) - # Explicitly commits log, new bom (setUp) and replacement impact. - # Is run via background jobs IRL - run_replace_bom_job( - doc=log, - boms=self.boms, - update_type="Replace BOM", - ) + # Is run via background job IRL + run_replace_bom_job(doc=log, boms=self.boms) log.reload() self.assertEqual(log.status, "Completed") - # teardown (undo replace impact) due to commit - boms = frappe._dict( - current_bom=self.boms.new_bom, - new_bom=self.boms.current_bom, + +def update_cost_in_all_boms_in_test(): + """ + Utility to run 'Update Cost' job in tests immediately without Cron job. + Run job for all levels (manually) until fully complete. + """ + parent_boms = [] + log = enqueue_update_cost() # create BOM Update Log + + while log.status != "Completed": + level_boms, current_level = process_boms_cost_level_wise(log, parent_boms) + log.reload() + + boms, batch = queue_bom_cost_jobs( + level_boms, log, current_level + ) # adds rows in log for tracking + log.reload() + + update_cost_in_level(log, boms, batch) # business logic + log.reload() + + # current level done, get next level boms + bom_batches = frappe.db.get_all( + "BOM Update Batch", + {"parent": log.name, "level": log.current_level}, + ["name", "boms_updated", "status"], ) - log2 = enqueue_replace_bom( - boms=self.boms, + current_boms, processed_boms = get_processed_current_boms(log, bom_batches) + parent_boms = get_next_higher_level_boms(child_boms=current_boms, processed_boms=processed_boms) + + set_values_in_log( + log.name, + values={ + "processed_boms": json.dumps(processed_boms), + "status": "Completed" if not parent_boms else "In Progress", + }, ) - run_replace_bom_job( # Explicitly commits - doc=log2, - boms=boms, - update_type="Replace BOM", - ) - self.assertEqual(log2.status, "Completed") + log.reload() + + return log diff --git a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py index fae72a0f6f..d1882e56e9 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py @@ -1,11 +1,13 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt import frappe from frappe.tests.utils import FrappeTestCase from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import replace_bom -from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost +from erpnext.manufacturing.doctype.bom_update_log.test_bom_update_log import ( + update_cost_in_all_boms_in_test, +) from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom from erpnext.stock.doctype.item.test_item import create_item @@ -25,8 +27,8 @@ class TestBOMUpdateTool(FrappeTestCase): boms = frappe._dict(current_bom=current_bom, new_bom=bom_doc.name) replace_bom(boms) - self.assertFalse(frappe.db.sql("select name from `tabBOM Item` where bom_no=%s", current_bom)) - self.assertTrue(frappe.db.sql("select name from `tabBOM Item` where bom_no=%s", bom_doc.name)) + self.assertFalse(frappe.db.exists("BOM Item", {"bom_no": current_bom, "docstatus": 1})) + self.assertTrue(frappe.db.exists("BOM Item", {"bom_no": bom_doc.name, "docstatus": 1})) # reverse, as it affects other testcases boms.current_bom = bom_doc.name @@ -52,13 +54,13 @@ class TestBOMUpdateTool(FrappeTestCase): self.assertEqual(doc.total_cost, 200) frappe.db.set_value("Item", "BOM Cost Test Item 2", "valuation_rate", 200) - update_cost() + update_cost_in_all_boms_in_test() doc.load_from_db() self.assertEqual(doc.total_cost, 300) frappe.db.set_value("Item", "BOM Cost Test Item 2", "valuation_rate", 100) - update_cost() + update_cost_in_all_boms_in_test() doc.load_from_db() self.assertEqual(doc.total_cost, 200)