test(stock): cover reading number formats end to end

Set the number format on the session user rather than on System Settings: the
code reads the user default, which shadows the global one, so these tests never
exercised the path they were written for. Restoring it in a finally also keeps
a failed assertion from leaving the whole suite in another locale.

Add a table test over every format in NUMBER_FORMAT_MAP, covering the grouped
values and the three formats parse_float used to read as 0, and restore the
formula-based coverage for non-numeric readings.

erpnext/stock/doctype/quality_inspection/test_quality_inspection.py

@@ -1,8 +1,11 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors
 # See license.txt
 
+from contextlib import contextmanager
+
 import frappe
 from frappe.utils import nowdate
+from frappe.utils.number_format import NumberFormat
 
 from erpnext.controllers.stock_controller import (
 	QualityInspectionNotSubmittedError,
@@ -12,10 +15,29 @@ from erpnext.controllers.stock_controller import (
 )
 from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
 from erpnext.stock.doctype.item.test_item import create_item
+from erpnext.stock.doctype.quality_inspection.quality_inspection import (
+	get_reading_separators,
+	parse_reading,
+)
 from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
 from erpnext.tests.utils import ERPNextTestSuite
 
 
+@contextmanager
+def user_number_format(number_format):
+	"""Temporarily set the session user's own number format."""
+	user = frappe.session.user
+	previous = frappe.db.get_value("DefaultValue", {"parent": user, "defkey": "number_format"}, "defvalue")
+	frappe.defaults.set_user_default("number_format", number_format)
+	try:
+		yield
+	finally:
+		if previous:
+			frappe.defaults.set_user_default("number_format", previous)
+		else:
+			frappe.defaults.clear_user_default("number_format")
+
+
 class TestQualityInspection(ERPNextTestSuite):
 	def setUp(self):
 		super().setUp()
@@ -255,7 +277,6 @@ class TestQualityInspection(ERPNextTestSuite):
 		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
 		create_quality_inspection_parameter("Density")
 
-		# text in a numeric reading was read as 0, silently skewing the mean
 		readings = [
 			{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "random text"}
 		]
@@ -267,74 +288,193 @@ class TestQualityInspection(ERPNextTestSuite):
 
 		dn.delete()
 
-	@ERPNextTestSuite.change_settings("System Settings", {"number_format": "#.###,##"})
+	def test_non_numeric_reading_in_formula_based_criteria(self):
+		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
+		create_quality_inspection_parameter("Density")
+
+		readings = [
+			{
+				"specification": "Density",
+				"formula_based_criteria": 1,
+				"acceptance_formula": "mean < 0.9",
+				"reading_1": "0.5",
+				"reading_2": "0.7",
+				"reading_3": "random text",
+			}
+		]
+		qa = create_quality_inspection(
+			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+		)
+
+		self.assertRaises(frappe.ValidationError, qa.save)
+
+		dn.delete()
+
+	def test_manual_inspection_reading_is_not_number_checked(self):
+		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
+		create_quality_inspection_parameter("Density")
+
+		readings = [
+			{
+				"specification": "Density",
+				"manual_inspection": 1,
+				"status": "Accepted",
+				"min_value": 1.15,
+				"max_value": 1.20,
+				"reading_1": "1.15 g/cm3",
+			}
+		]
+		qa = create_quality_inspection(
+			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+		)
+		qa.save()
+
+		self.assertEqual(qa.readings[0].status, "Accepted")
+
+		qa.delete()
+		dn.delete()
+
 	def test_reading_in_comma_decimal_number_format(self):
 		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
 		create_quality_inspection_parameter("Density")
 
 		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
-		qa = create_quality_inspection(
+		with user_number_format("#.###,##"):
-			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			qa = create_quality_inspection(
-		)
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
-		qa.save()
+			)
+			qa.save()
 
-		# 1,15 is 1.15 in this format, which is within the acceptance range
+			self.assertEqual(qa.readings[0].status, "Accepted")
-		self.assertEqual(qa.readings[0].status, "Accepted")
+			self.assertEqual(qa.status, "Accepted")
-		self.assertEqual(qa.status, "Accepted")
 
 		qa.delete()
 		dn.delete()
 
-	@ERPNextTestSuite.change_settings("System Settings", {"number_format": "# ###,##"})
 	def test_reading_in_space_grouped_number_format(self):
 		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
 		create_quality_inspection_parameter("Density")
 
-		# this format is comma decimal but space grouped, so the separators were not
-		# swapped at all and 1,15 was read as 115 and rejected
 		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
-		qa = create_quality_inspection(
+		with user_number_format("# ###,##"):
-			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			qa = create_quality_inspection(
-		)
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
-		qa.save()
+			)
+			qa.save()
 
-		self.assertEqual(qa.readings[0].status, "Accepted")
+			self.assertEqual(qa.readings[0].status, "Accepted")
-		self.assertEqual(qa.status, "Accepted")
+			self.assertEqual(qa.status, "Accepted")
 
 		qa.delete()
 		dn.delete()
 
-	@ERPNextTestSuite.change_settings("System Settings", {"number_format": "#.###,##"})
 	def test_reading_in_wrong_decimal_number_format(self):
 		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
 		create_quality_inspection_parameter("Density")
 
-		# a dot is the group separator in this format, so 1.15 is not a valid number.
-		# it must be refused, not silently read as 115 and rejected.
 		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1.15"}]
-		qa = create_quality_inspection(
+		with user_number_format("#.###,##"):
-			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			qa = create_quality_inspection(
-		)
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			)
 
-		self.assertRaises(frappe.ValidationError, qa.save)
+			self.assertRaises(frappe.ValidationError, qa.save)
 
 		dn.delete()
 
-	@ERPNextTestSuite.change_settings("System Settings", {"number_format": "#,###.##"})
 	def test_reading_with_comma_in_dot_decimal_number_format(self):
 		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
 		create_quality_inspection_parameter("Density")
 
-		# the reported bug: 1,15 was read as 115 here and silently rejected
 		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
-		qa = create_quality_inspection(
+		with user_number_format("#,###.##"):
-			reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			qa = create_quality_inspection(
-		)
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			)
 
-		self.assertRaises(frappe.ValidationError, qa.save)
+			self.assertRaises(frappe.ValidationError, qa.save)
 
 		dn.delete()
 
+	@ERPNextTestSuite.change_settings("System Settings", {"number_format": "#,###.##"})
+	def test_reading_number_format_prefers_the_user_over_the_system(self):
+		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
+		create_quality_inspection_parameter("Density")
+
+		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
+		with user_number_format("#.###,##"):
+			qa = create_quality_inspection(
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			)
+			qa.save()
+
+			self.assertEqual(qa.readings[0].status, "Accepted")
+
+		qa.delete()
+		dn.delete()
+
+	def test_stored_reading_stays_submittable_in_another_number_format(self):
+		dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
+		create_quality_inspection_parameter("Density")
+
+		readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
+		with user_number_format("#.###,##"):
+			qa = create_quality_inspection(
+				reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
+			)
+			qa.save()
+
+		with user_number_format("#,###.##"):
+			qa.reload()
+			qa.submit()
+
+		self.assertEqual(qa.docstatus, 1)
+
+		qa.cancel()
+		qa.delete()
+		dn.delete()
+
+	def test_parse_reading_in_every_number_format(self):
+		accepted = [
+			("#,###.##", "1.15", 1.15),
+			("#,###.##", "1,234.56", 1234.56),
+			("#,##,###.##", "12,34,567.89", 1234567.89),
+			("#,###.###", "1,234.567", 1234.567),
+			("#.###,##", "1,15", 1.15),
+			("#.###,##", "1.234,56", 1234.56),
+			("# ###,##", "1,15", 1.15),
+			("# ###,##", "1.15", 1.15),
+			("# ###,##", "1 234,56", 1234.56),
+			("# ###.##", "1 234.56", 1234.56),
+			("#'###.##", "1'234.56", 1234.56),
+			("#, ###.##", "1, 234.56", 1234.56),
+			("#.########", "1.15", 1.15),
+			("#,###", "1.5", 1.5),
+			("#,###", "1,500", 1500.0),
+			("#.###", "1.5", 1.5),
+			("#.###", "1.500", 1.5),
+			("#,###.##", "-1,234.56", -1234.56),
+		]
+		refused = [
+			("#,###.##", "1,15"),
+			("#.###,##", "1.15"),
+			("#,###.##", "--1.15"),
+			("#,###.##", "1²"),
+			("#,###.##", "nan"),
+			("#,###.##", "random text"),
+			("#,###", "1,50"),
+		]
+
+		for number_format, value, expected in accepted:
+			decimal_str, comma_str = get_reading_separators(NumberFormat.from_string(number_format))
+			with self.subTest(number_format=number_format, value=value):
+				self.assertEqual(parse_reading(value, decimal_str, comma_str), expected)
+
+		for number_format, value in refused:
+			decimal_str, comma_str = get_reading_separators(NumberFormat.from_string(number_format))
+			with self.subTest(number_format=number_format, value=value):
+				self.assertIsNone(parse_reading(value, decimal_str, comma_str))
+
 	def test_delete_quality_inspection_linked_with_stock_entry(self):
 		item_code = create_item("_Test Cicuular Dependecy Item with QA").name