From f5f956c4dd4c902226b3682f924ef50929bb8a25 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sat, 12 Sep 2026 14:23:11 +0530 Subject: [PATCH] test: stop six tests from passing without running (#59025) * test: stop four tests from passing without running Three advisory-lock tests return early on MariaDB: if frappe.db.db_type != "postgres": return A bare return reports the test as passed, so the MariaDB CI job shows green for a test it never ran. skipTest reports it as skipped. test_stock_reco_with_opening_stock_with_diff_inventory returned early when the custom "Plant" DocType already existed. DocType creation is DDL and survives the test transaction, so the test ran once on a fresh site and silently did nothing on every run after that. Create the DocType only when it is missing and let the test run either way. Its closing loop also asserted inside an if/elif over the ledger rows, which verified nothing if the dimension came back unset. Compare the whole {plant: qty} mapping instead. * test: give the job card validator tests a real job card Both tests looked for a submitted Job Card left behind by another test and returned when they did not find one: jc_name = frappe.db.get_value("Job Card", {"docstatus": 1}) if not jc_name: return # skip if no job cards in test data Run in isolation they asserted nothing and still reported a pass, and they were the only coverage for validate_job_card_fg_item and validate_job_card_item. Move them to test_job_card.py, where the Work Order and BOM fixtures that produce Job Cards already live, and build the Job Card in the test. The finished-good case needs a card that carries one, so it goes through a track_semi_finished_goods BOM. Both now assert on the message text, and both fail if the validator body is removed. --- .../doctype/job_card/test_job_card.py | 92 +++++++++++++++++++ .../stock/doctype/pick_list/test_pick_list.py | 2 +- .../test_serial_and_batch_bundle.py | 2 +- .../doctype/stock_entry/test_stock_entry.py | 40 -------- .../test_stock_ledger_entry.py | 2 +- .../test_stock_reconciliation.py | 48 +++++----- 6 files changed, 117 insertions(+), 69 deletions(-) diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index 2aca30c2296..ed5d4427575 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -3251,6 +3251,98 @@ class TestJobCard(ERPNextTestSuite): self.assertEqual(s.additional_costs[2].amount, 480) self.assertEqual(s.additional_costs[3].amount, 480) + @ERPNextTestSuite.change_settings("Manufacturing Settings", {"job_card_excess_transfer": 0}) + def test_stock_entry_needs_a_job_card_item_reference(self): + create_bom_with_multiple_operations() + work_order = make_wo_with_transfer_against_jc() + job_card = frappe.db.get_value("Job Card", {"work_order": work_order.name}) + + stock_entry = frappe.new_doc("Stock Entry") + stock_entry.job_card = job_card + stock_entry.purpose = "Material Transfer for Manufacture" + stock_entry.append( + "items", + { + "item_code": "_Test Item", + "s_warehouse": "Stores - _TC", + "qty": 1, + "job_card_item": None, + }, + ) + + self.assertRaisesRegex( + frappe.ValidationError, + "job card item reference is missing", + stock_entry.validate_job_card_item, + ) + + def test_stock_entry_finished_good_must_match_the_job_card(self): + from erpnext.manufacturing.doctype.operation.test_operation import make_operation + from erpnext.stock.doctype.item.test_item import make_item + + warehouse = "Stores - _TC" + raw_material = make_item("_Test JC FG Check RM", {"is_stock_item": 1}).name + finished_good = make_item("_Test JC FG Check FG", {"is_stock_item": 1}).name + unrelated_item = make_item("_Test JC FG Check Other", {"is_stock_item": 1}).name + + operation = { + "operation": "_Test JC FG Check Op", + "workstation": "_Test Workstation A", + "finished_good": finished_good, + "finished_good_qty": 1, + "is_final_finished_good": 1, + "sequence_id": 1, + "time_in_mins": 60, + "source_warehouse": warehouse, + "fg_warehouse": warehouse, + "skip_material_transfer": 1, + } + make_workstation(operation) + make_operation(operation) + + bom = frappe.new_doc( + "BOM", + company="_Test Company", + item=finished_good, + quantity=1, + with_operations=1, + track_semi_finished_goods=1, + ) + bom.append("items", {"item_code": raw_material, "qty": 1, "operation_row_id": 1}) + bom.append("operations", operation) + bom.insert() + bom.submit() + + work_order = make_wo_order_test_record( + item=finished_good, + qty=1, + source_warehouse=warehouse, + fg_warehouse=warehouse, + bom_no=bom.name, + skip_transfer=1, + do_not_save=True, + ) + work_order.operations[0].time_in_mins = 60 + work_order.save() + work_order.submit() + + job_card = frappe.db.get_value("Job Card", {"work_order": work_order.name}) + self.assertEqual(frappe.db.get_value("Job Card", job_card, "finished_good"), finished_good) + + mismatched = frappe.new_doc("Stock Entry") + mismatched.job_card = job_card + mismatched.append("items", {"item_code": unrelated_item, "is_finished_item": 1, "qty": 1}) + self.assertRaisesRegex( + frappe.ValidationError, + f"Finished Good must be {finished_good}", + mismatched.validate_job_card_fg_item, + ) + + matching = frappe.new_doc("Stock Entry") + matching.job_card = job_card + matching.append("items", {"item_code": finished_good, "is_finished_item": 1, "qty": 1}) + matching.validate_job_card_fg_item() + def create_bom_with_multiple_operations(): "Create a BOM with multiple operations and Material Transfer against Job Card" diff --git a/erpnext/stock/doctype/pick_list/test_pick_list.py b/erpnext/stock/doctype/pick_list/test_pick_list.py index af76a4475f6..386862fcd86 100644 --- a/erpnext/stock/doctype/pick_list/test_pick_list.py +++ b/erpnext/stock/doctype/pick_list/test_pick_list.py @@ -86,7 +86,7 @@ class TestPickList(ERPNextTestSuite): def test_pick_list_allocation_takes_advisory_gate(self): if frappe.db.db_type != "postgres": - return + self.skipTest("advisory locks are a PostgreSQL feature") item = make_item(properties={"is_stock_item": 1}).name make_stock_entry(item=item, to_warehouse="_Test Warehouse - _TC", qty=5, basic_rate=100) diff --git a/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py b/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py index 8f397adb736..51dd2e8e504 100644 --- a/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py +++ b/erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py @@ -203,7 +203,7 @@ class TestSerialandBatchBundle(ERPNextTestSuite): def test_outward_batch_valuation_takes_transaction_advisory_lock(self): if frappe.db.db_type != "postgres": - return + self.skipTest("advisory locks are a PostgreSQL feature") from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index c158a2d5b3b..673ef381249 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -4473,46 +4473,6 @@ class TestStockEntryCoverage(ERPNextTestSuite): se.work_order = "WO-SAME-001" se.validate_source_stock_entry() # must not raise - # ── validate_job_card_fg_item ────────────────────────────────────────────── - - def test_validate_job_card_fg_item_throws_when_fg_item_mismatches(self): - wrong_fg = make_item("_JC Wrong FG Item", {"is_stock_item": 1}).name - - jc_name = frappe.db.get_value("Job Card", {"docstatus": 1, "finished_good": ("!=", "")}) - if not jc_name: - return # skip if no suitable job card in test data - - jc = frappe.db.get_value("Job Card", jc_name, ["finished_good"], as_dict=1) - if jc.finished_good == wrong_fg: - return # skip if the wrong_fg happens to match - - se = frappe.new_doc("Stock Entry") - se.job_card = jc_name - se.append("items", {"item_code": wrong_fg, "is_finished_item": 1, "qty": 1}) - self.assertRaises(frappe.ValidationError, se.validate_job_card_fg_item) - - # ── validate_job_card_item ───────────────────────────────────────────────── - - @ERPNextTestSuite.change_settings("Manufacturing Settings", {"job_card_excess_transfer": 0}) - def test_validate_job_card_item_throws_when_job_card_item_ref_missing(self): - jc_name = frappe.db.get_value("Job Card", {"docstatus": 1}) - if not jc_name: - return # skip if no job cards in test data - - se = frappe.new_doc("Stock Entry") - se.job_card = jc_name - se.purpose = "Material Transfer for Manufacture" - se.append( - "items", - { - "item_code": "_Test Item", - "s_warehouse": "_Test Warehouse - _TC", - "qty": 1, - "job_card_item": None, - }, - ) - self.assertRaises(frappe.ValidationError, se.validate_job_card_item) - # ── get_available_materials ──────────────────────────────────────────────── def test_get_available_materials_tracks_transferred_qty(self): diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 5dc26d8cd08..aa5c305c505 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -36,7 +36,7 @@ class TestStockLedgerEntry(ERPNextTestSuite, StockTestMixin): def test_stock_write_takes_sle_advisory_gate(self): if frappe.db.db_type != "postgres": - return + self.skipTest("advisory locks are a PostgreSQL feature") item = make_item(properties={"is_stock_item": 1}).name diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 47ce73872a7..fa8c9e67d6b 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -2046,27 +2046,24 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): create_inventory_dimension, ) - if frappe.db.exists("DocType", "Plant"): - return - - doctype = frappe.get_doc( - { - "doctype": "DocType", - "name": "Plant", - "module": "Stock", - "custom": 1, - "fields": [ - { - "fieldname": "plant_name", - "fieldtype": "Data", - "label": "Plant Name", - "reqd": 1, - } - ], - "autoname": "field:plant_name", - } - ) - doctype.insert(ignore_permissions=True) + if not frappe.db.exists("DocType", "Plant"): + frappe.get_doc( + { + "doctype": "DocType", + "name": "Plant", + "module": "Stock", + "custom": 1, + "fields": [ + { + "fieldname": "plant_name", + "fieldtype": "Data", + "label": "Plant Name", + "reqd": 1, + } + ], + "autoname": "field:plant_name", + } + ).insert(ignore_permissions=True) create_inventory_dimension(dimension_name="ID-Plant", reference_document="Plant") plant_a = frappe.get_doc( @@ -2130,11 +2127,10 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): {"voucher_type": "Stock Reconciliation", "voucher_no": sr.name, "is_cancelled": 0}, ["item_code", "id_plant", "actual_qty", "valuation_rate"], ) - for s in sle: - if s.id_plant == plant_a.name: - self.assertEqual(s.actual_qty, 5) - elif s.id_plant == plant_b.name: - self.assertEqual(s.actual_qty, 3) + self.assertEqual( + {row.id_plant: row.actual_qty for row in sle}, + {plant_a.name: 5, plant_b.name: 3}, + ) def test_serial_no_status_with_backdated_stock_reco(self): from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note