chore: Less hacky tests, versioning (replace bom) and clearing log data (update cost)
- Remove `auto_commit_on_many_writes` in `update_cost_in_level()` as commits happen every N BOMs
- Auto commit every 50 BOMs
- test: Remove hacky `frappe.flags.in_test` returns
- test: Enqueue `now` if in tests (for update cost and replace bom)
- Replace BOM: Copy bom object to `_doc_before_save` so that version.py finds a difference between the two
- Replace BOM: Add reference to version
- Update Cost: Unset `processed_boms` if Log is completed (useless after completion)
- test: `update_cost_in_all_boms_in_test` works close to actual prod implementation (only call Cron job manually)
- Test: use `enqueue_replace_bom` so that test works closest to production behaviour
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
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 71430bd..9c9c240 100644
--- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py
+++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py
@@ -67,9 +67,6 @@
)
def on_submit(self):
- if frappe.flags.in_test:
- return
-
if self.update_type == "Replace BOM":
boms = {"current_bom": self.current_bom, "new_bom": self.new_bom}
frappe.enqueue(
@@ -77,6 +74,7 @@
doc=self,
boms=boms,
timeout=40000,
+ now=frappe.flags.in_test,
)
else:
process_boms_cost_level_wise(self)
@@ -94,7 +92,7 @@
frappe.db.auto_commit_on_many_writes = 1
boms = frappe._dict(boms or {})
- replace_bom(boms)
+ replace_bom(boms, doc.name)
doc.db_set("status", "Completed")
except Exception:
@@ -135,10 +133,6 @@
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)
@@ -161,16 +155,13 @@
)
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,
bom_list=boms_to_process,
batch_name=batch_row.name,
queue="long",
+ now=frappe.flags.in_test,
)
@@ -208,10 +199,11 @@
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)
+ # Unset processed BOMs if log is complete, it is used for next level BOMs
set_values_in_log(
log.name,
values={
- "processed_boms": json.dumps(processed_boms),
+ "processed_boms": json.dumps([] if not parent_boms else processed_boms),
"status": "Completed" if not parent_boms else "In Progress",
},
commit=True,
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 dde1e4e..af115e3 100644
--- a/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py
+++ b/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py
@@ -1,6 +1,7 @@
# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors
# For license information, please see license.txt
+import copy
import json
from collections import defaultdict
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
@@ -12,7 +13,7 @@
from frappe import _
-def replace_bom(boms: Dict) -> None:
+def replace_bom(boms: Dict, log_name: str) -> None:
"Replace current BOM with new BOM in parent BOMs."
current_bom = boms.get("current_bom")
@@ -29,13 +30,17 @@
# this is only used for versioning and we do not want
# to make separate db calls by using load_doc_before_save
# which proves to be expensive while doing bulk replace
- bom_obj._doc_before_save = bom_obj
+ bom_obj._doc_before_save = copy.deepcopy(bom_obj)
bom_obj.update_exploded_items()
bom_obj.calculate_cost()
bom_obj.update_parent_cost()
bom_obj.db_update()
- if bom_obj.meta.get("track_changes") and not bom_obj.flags.ignore_version:
- bom_obj.save_version()
+ bom_obj.flags.updater_reference = {
+ "doctype": "BOM Update Log",
+ "docname": log_name,
+ "label": _("via BOM Update Tool"),
+ }
+ bom_obj.save_version()
def update_cost_in_level(
@@ -48,8 +53,6 @@
if status == "Failed":
return
- frappe.db.auto_commit_on_many_writes = 1
-
update_cost_in_boms(bom_list=bom_list) # main updation logic
bom_batch = frappe.qb.DocType("BOM Update Batch")
@@ -62,8 +65,6 @@
except Exception:
handle_exception(doc)
finally:
- frappe.db.auto_commit_on_many_writes = 0
-
if not frappe.flags.in_test:
frappe.db.commit() # nosemgrep
@@ -121,7 +122,7 @@
bom_doc.calculate_cost(save_updates=True, update_hour_rate=True)
bom_doc.db_update()
- if (index % 100 == 0) and not frappe.flags.in_test:
+ if (index % 50 == 0) 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 d770f6c..b38fc89 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,22 +1,12 @@
# 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_log.bom_updation_utils import (
- get_next_higher_level_boms,
- set_values_in_log,
- update_cost_in_level,
+ resume_bom_cost_update_jobs,
)
from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import (
enqueue_replace_bom,
@@ -60,62 +50,22 @@
with self.assertRaises(frappe.ValidationError):
enqueue_replace_bom(boms=frappe._dict(current_bom=self.boms.new_bom, new_bom="Dummy BOM"))
- def test_bom_update_log_queueing(self):
- "Test if BOM Update Log is created and queued."
-
- log = enqueue_replace_bom(boms=self.boms)
-
- self.assertEqual(log.docstatus, 1)
- self.assertEqual(log.status, "Queued")
-
def test_bom_update_log_completion(self):
"Test if BOM Update Log handles job completion correctly."
log = enqueue_replace_bom(boms=self.boms)
-
- # Is run via background job IRL
- run_replace_bom_job(doc=log, boms=self.boms)
log.reload()
-
self.assertEqual(log.status, "Completed")
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.
+ Utility to run 'Update Cost' job in tests without Cron job 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"],
- )
- 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",
- },
- )
+ resume_bom_cost_update_jobs() # run cron job until complete
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 d1882e5..5dd557f 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
@@ -4,10 +4,10 @@
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_log.test_bom_update_log import (
update_cost_in_all_boms_in_test,
)
+from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import enqueue_replace_bom
from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom
from erpnext.stock.doctype.item.test_item import create_item
@@ -17,6 +17,9 @@
class TestBOMUpdateTool(FrappeTestCase):
"Test major functions run via BOM Update Tool."
+ def tearDown(self):
+ frappe.db.rollback()
+
def test_replace_bom(self):
current_bom = "BOM-_Test Item Home Desktop Manufactured-001"
@@ -25,16 +28,11 @@
bom_doc.insert()
boms = frappe._dict(current_bom=current_bom, new_bom=bom_doc.name)
- replace_bom(boms)
+ enqueue_replace_bom(boms=boms)
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
- boms.new_bom = current_bom
- replace_bom(boms)
-
def test_bom_cost(self):
for item in ["BOM Cost Test Item 1", "BOM Cost Test Item 2", "BOM Cost Test Item 3"]:
item_doc = create_item(item, valuation_rate=100)