Skip to content

refactor(test): move tests to inventory dimension #54508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ruthra-kumar wants to merge 1 commit into
base: develop
Choose a base branch
from
+47 −41
Open

refactor(test): move tests to inventory dimension #54508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors
# See license.txt
import copy

import frappe
from frappe.custom.doctype.custom_field.custom_field import create_custom_field
Expand Down Expand Up @@ -545,6 +546,52 @@ def test_validate_negative_stock_with_multiple_dimension(self):
dn_doc.save()
self.assertRaises(InventoryDimensionNegativeStockError, dn_doc.submit)

def test_inventory_dimension_after_save(self):
"""
The subcontracting controller resets the supplied items table on each save causing the inventory dimensions to be lost.
This test ensures that the inventory dimensions are retained on each save.
"""
from erpnext.controllers.tests.test_subcontracting_controller import set_backflush_based_on
from erpnext.subcontracting.doctype.subcontracting_receipt.test_subcontracting_receipt import (
get_rm_items,
get_subcontracting_order,
make_stock_in_entry,
make_stock_transfer_entry,
make_subcontracting_receipt,
)

inventory_dimension = create_inventory_dimension(
apply_to_all_doctypes=1,
dimension_name="Inv Site",
reference_document="Inv Site",
document_type="Inv Site",
)

inventory_dimension.reqd = 1
inventory_dimension.save()

set_backflush_based_on("BOM")

sco = get_subcontracting_order()
rm_items = get_rm_items(sco.supplied_items)
itemwise_details = make_stock_in_entry(rm_items=rm_items)
make_stock_transfer_entry(
sco_no=sco.name,
rm_items=rm_items,
itemwise_details=copy.deepcopy(itemwise_details),
)
scr = make_subcontracting_receipt(sco.name)
scr.items[0].inv_site = "Site 1"
scr.save()

scr.supplied_items[0].inv_site = "Site 1"
scr.save()

self.assertEqual(scr.supplied_items[0].inv_site, "Site 1")

inventory_dimension.reqd = 0
inventory_dimension.save()
Comment on lines +570 to +593
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard cleanup with try/finally and reset backflush_based_on.

Two pieces of global state leak if this test fails or even succeeds:

  1. set_backflush_based_on("BOM") is never reverted. Since Buying Settings.backflush_raw_materials_of_subcontract_based_on is a singleton persisted across the test run, this will silently alter behavior for any subsequent subcontracting tests in the same process.
  2. inventory_dimension.reqd = 0 is only restored on the happy path. If assertEqual on line 590 (or any prior save()) raises, reqd=1 remains on the shared "Inv Site" dimension — and other tests in this file (test_validate_negative_stock_for_inventory_dimension, test_validate_negative_stock_with_multiple_dimension) reuse the same "Inv Site" dimension via the idempotent create_inventory_dimension, so they will start seeing a mandatory field and fail with unrelated errors.

Wrap the mutation/restore in try/finally (and capture the previous backflush value):

🧹 Proposed cleanup 
-		inventory_dimension.reqd = 1
-		inventory_dimension.save()
-
-		set_backflush_based_on("BOM")
-
-		sco = get_subcontracting_order()
-		rm_items = get_rm_items(sco.supplied_items)
-		itemwise_details = make_stock_in_entry(rm_items=rm_items)
-		make_stock_transfer_entry(
-			sco_no=sco.name,
-			rm_items=rm_items,
-			itemwise_details=copy.deepcopy(itemwise_details),
-		)
-		scr = make_subcontracting_receipt(sco.name)
-		scr.items[0].inv_site = "Site 1"
-		scr.save()
-
-		scr.supplied_items[0].inv_site = "Site 1"
-		scr.save()
-
-		self.assertEqual(scr.supplied_items[0].inv_site, "Site 1")
-
-		inventory_dimension.reqd = 0
-		inventory_dimension.save()
+		previous_backflush = frappe.db.get_single_value(
+			"Buying Settings", "backflush_raw_materials_of_subcontract_based_on"
+		)
+		inventory_dimension.reqd = 1
+		inventory_dimension.save()
+		set_backflush_based_on("BOM")
+		try:
+			sco = get_subcontracting_order()
+			rm_items = get_rm_items(sco.supplied_items)
+			itemwise_details = make_stock_in_entry(rm_items=rm_items)
+			make_stock_transfer_entry(
+				sco_no=sco.name,
+				rm_items=rm_items,
+				itemwise_details=copy.deepcopy(itemwise_details),
+			)
+			scr = make_subcontracting_receipt(sco.name)
+			scr.items[0].inv_site = "Site 1"
+			scr.save()
+
+			scr.supplied_items[0].inv_site = "Site 1"
+			scr.save()
+
+			self.assertEqual(scr.supplied_items[0].inv_site, "Site 1")
+		finally:
+			inventory_dimension.reqd = 0
+			inventory_dimension.save()
+			if previous_backflush:
+				set_backflush_based_on(previous_backflush)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py` around
lines 570 - 593, Wrap the changes to global state in a try/finally: before
calling set_backflush_based_on("BOM") capture the current backflush setting (the
Buying Settings singleton) into a local variable, then call
set_backflush_based_on("BOM") and run the test steps; in the finally block
restore the captured backflush value via set_backflush_based_on(...) and also
ensure inventory_dimension.reqd is set back to its original value and
inventory_dimension.save() is called so the "Inv Site" dimension is always
reverted even if assertions or saves fail. Reference:
set_backflush_based_on(...), inventory_dimension.reqd, and
inventory_dimension.save().



def get_voucher_sl_entries(voucher_no, fields):
return frappe.get_all(
Expand Down
41 changes: 0 additions & 41 deletions erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,47 +2035,6 @@ def test_over_receipt(self):
scr.submit()
frappe.flags["args"].pop("items", None)

def test_inventory_dimensions(self):
"""
The subcontracting controller resets the supplied items table on each save causing the inventory dimensions to be lost.
This test ensures that the inventory dimensions are retained on each save.
"""
from erpnext.stock.doctype.inventory_dimension.test_inventory_dimension import (
create_inventory_dimension,
)

inventory_dimension = create_inventory_dimension(
apply_to_all_doctypes=1,
dimension_name="Inv Site",
reference_document="Inv Site",
document_type="Inv Site",
)

inventory_dimension.reqd = 1
inventory_dimension.save()

set_backflush_based_on("BOM")

sco = get_subcontracting_order()
rm_items = get_rm_items(sco.supplied_items)
itemwise_details = make_stock_in_entry(rm_items=rm_items)
make_stock_transfer_entry(
sco_no=sco.name,
rm_items=rm_items,
itemwise_details=copy.deepcopy(itemwise_details),
)
scr = make_subcontracting_receipt(sco.name)
scr.items[0].inv_site = "Site 1"
scr.save()

scr.supplied_items[0].inv_site = "Site 1"
scr.save()

self.assertEqual(scr.supplied_items[0].inv_site, "Site 1")

inventory_dimension.reqd = 0
inventory_dimension.save()


def make_return_subcontracting_receipt(**args):
args = frappe._dict(args)
Expand Down
Loading