refactor(test): move tests to inventory dimension

[ruthra-kumar]

No description provided.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change relocates a test for inventory dimension persistence behavior from the subcontracting receipt test suite to the inventory dimension test suite. The new test test_inventory_dimension_after_save validates that inv_site values in subcontracting receipt supplied items persist across multiple save() operations. The test constructs an Inventory Dimension, configures backflush behavior, creates a subcontracting receipt flow, and verifies that the dimension values remain set after consecutive saves. The corresponding test is removed from the subcontracting receipt test file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to assess relevance to the changeset.	Add a description explaining the test refactoring and why the test was moved to the inventory dimension module.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: moving a test from subcontracting_receipt to inventory_dimension module.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py`:
- Around line 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().

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: e4e70de8-48ac-42f9-92d1-b3349f43c37f

📥 Commits

Reviewing files that changed from the base of the PR and between 5518e8c and aa3f3e7.

📒 Files selected for processing (2)

• erpnext/stock/doctype/inventory_dimension/test_inventory_dimension.py
• erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py

💤 Files with no reviewable changes (1)

• erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py

[coderabbitai]

⚠️ 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().