Merge pull request #16649 from ankush/testing_overhaul

test: better test utils

frappe/model/base_document.py

@@ -350,7 +350,7 @@ class BaseDocument(object):
 
 	@property
 	def docstatus(self):
-		return DocStatus(self.get("docstatus"))
+		return DocStatus(cint(self.get("docstatus")))
 
 	@docstatus.setter
 	def docstatus(self, value):

frappe/parallel_test_runner.py

@@ -46,7 +46,7 @@ class ParallelTestRunner:
 
 		if hasattr(test_module, "global_test_dependencies"):
 			for doctype in test_module.global_test_dependencies:
-				make_test_records(doctype)
+				make_test_records(doctype, commit=True)
 
 		elapsed = time.time() - start_time
 		elapsed = click.style(f" ({elapsed:.03}s)", fg="red")
@@ -76,7 +76,7 @@ class ParallelTestRunner:
 	def create_test_dependency_records(self, module, path, filename):
 		if hasattr(module, "test_dependencies"):
 			for doctype in module.test_dependencies:
-				make_test_records(doctype)
+				make_test_records(doctype, commit=True)
 
 		if os.path.basename(os.path.dirname(path)) == "doctype":
 			# test_data_migration_connector.py > data_migration_connector.json
@@ -86,7 +86,7 @@ class ParallelTestRunner:
 				with open(test_record_file_path, "r") as f:
 					doc = json.loads(f.read())
 					doctype = doc["name"]
-					make_test_records(doctype)
+					make_test_records(doctype, commit=True)
 
 	def get_module(self, path, filename):
 		app_path = frappe.get_pymodule_path(self.app)

frappe/test_runner.py

@@ -226,7 +226,7 @@ def run_tests_for_doctype(
 		if force:
 			for name in frappe.db.sql_list("select name from `tab%s`" % doctype):
 				frappe.delete_doc(doctype, name, force=True)
-		make_test_records(doctype, verbose=verbose, force=force)
+		make_test_records(doctype, verbose=verbose, force=force, commit=True)
 		modules.append(importlib.import_module(test_module))
 
 	return _run_unittest(
@@ -245,7 +245,7 @@ def run_tests_for_module(
 	module = importlib.import_module(module)
 	if hasattr(module, "test_dependencies"):
 		for doctype in module.test_dependencies:
-			make_test_records(doctype, verbose=verbose)
+			make_test_records(doctype, verbose=verbose, commit=True)
 
 	frappe.db.commit()
 	return _run_unittest(
@@ -330,7 +330,7 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False):
 
 	if hasattr(module, "test_dependencies"):
 		for doctype in module.test_dependencies:
-			make_test_records(doctype, verbose=verbose)
+			make_test_records(doctype, verbose=verbose, commit=True)
 
 	is_ui_test = True if hasattr(module, "TestDriver") else False
 
@@ -346,12 +346,12 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False):
 			with open(txt_file, "r") as f:
 				doc = json.loads(f.read())
 			doctype = doc["name"]
-			make_test_records(doctype, verbose)
+			make_test_records(doctype, verbose, commit=True)
 
 	test_suite.addTest(unittest.TestLoader().loadTestsFromModule(module))
 
 
-def make_test_records(doctype, verbose=0, force=False):
+def make_test_records(doctype, verbose=0, force=False, commit=False):
 	if not frappe.db:
 		frappe.connect()
 
@@ -364,8 +364,8 @@ def make_test_records(doctype, verbose=0, force=False):
 
 		if options not in frappe.local.test_objects:
 			frappe.local.test_objects[options] = []
-			make_test_records(options, verbose, force)
-			make_test_records_for_doctype(options, verbose, force)
+			make_test_records(options, verbose, force, commit=commit)
+			make_test_records_for_doctype(options, verbose, force, commit=commit)
 
 
 def get_modules(doctype):
@@ -405,7 +405,7 @@ def get_dependencies(doctype):
 	return options_list
 
 
-def make_test_records_for_doctype(doctype, verbose=0, force=False):
+def make_test_records_for_doctype(doctype, verbose=0, force=False, commit=False):
 	if not force and doctype in get_test_record_log():
 		return
 
@@ -420,17 +420,19 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False):
 	elif hasattr(test_module, "test_records"):
 		if doctype in frappe.local.test_objects:
 			frappe.local.test_objects[doctype] += make_test_objects(
-				doctype, test_module.test_records, verbose, force
+				doctype, test_module.test_records, verbose, force, commit=commit
 			)
 		else:
 			frappe.local.test_objects[doctype] = make_test_objects(
-				doctype, test_module.test_records, verbose, force
+				doctype, test_module.test_records, verbose, force, commit=commit
 			)
 
 	else:
 		test_records = frappe.get_test_records(doctype)
 		if test_records:
-			frappe.local.test_objects[doctype] += make_test_objects(doctype, test_records, verbose, force)
+			frappe.local.test_objects[doctype] += make_test_objects(
+				doctype, test_records, verbose, force, commit=commit
+			)
 
 		elif verbose:
 			print_mandatory_fields(doctype)
@@ -438,7 +440,7 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False):
 	add_to_test_record_log(doctype)
 
 
-def make_test_objects(doctype, test_records=None, verbose=None, reset=False):
+def make_test_objects(doctype, test_records=None, verbose=None, reset=False, commit=False):
 	"""Make test objects from given list of `test_records` or from `test_records.json`"""
 	records = []
 
@@ -495,7 +497,8 @@ def make_test_objects(doctype, test_records=None, verbose=None, reset=False):
 
 		records.append(d.name)
 
-		frappe.db.commit()
+		if commit:
+			frappe.db.commit()
 	return records
 
 

frappe/tests/test_permissions.py

@@ -24,25 +24,26 @@ test_dependencies = ["Blogger", "Blog Post", "User", "Contact", "Salutation"]
 
 
 class TestPermissions(FrappeTestCase):
+	@classmethod
+	def setUpClass(cls):
+		super().setUpClass()
+		frappe.clear_cache(doctype="Blog Post")
+		user = frappe.get_doc("User", "test1@example.com")
+		user.add_roles("Website Manager")
+		user.add_roles("System Manager")
+
+		user = frappe.get_doc("User", "test2@example.com")
+		user.add_roles("Blogger")
+
+		user = frappe.get_doc("User", "test3@example.com")
+		user.add_roles("Sales User")
+
+		user = frappe.get_doc("User", "testperm@example.com")
+		user.add_roles("Website Manager")
+
 	def setUp(self):
 		frappe.clear_cache(doctype="Blog Post")
 
-		if not frappe.flags.permission_user_setup_done:
-			user = frappe.get_doc("User", "test1@example.com")
-			user.add_roles("Website Manager")
-			user.add_roles("System Manager")
-
-			user = frappe.get_doc("User", "test2@example.com")
-			user.add_roles("Blogger")
-
-			user = frappe.get_doc("User", "test3@example.com")
-			user.add_roles("Sales User")
-
-			user = frappe.get_doc("User", "testperm@example.com")
-			user.add_roles("Website Manager")
-
-			frappe.flags.permission_user_setup_done = True
-
 		reset("Blogger")
 		reset("Blog Post")
 

frappe/tests/test_test_utils.py

@@ -0,0 +1,34 @@
+import frappe
+from frappe.tests.utils import FrappeTestCase, change_settings
+
+
+class TestTestUtils(FrappeTestCase):
+	SHOW_TRANSACTION_COMMIT_WARNINGS = True
+
+	def test_document_assertions(self):
+
+		currency = frappe.new_doc("Currency")
+		currency.currency_name = "STONKS"
+		currency.smallest_currency_fraction_value = 0.420_001
+		currency.save()
+
+		self.assertDocumentEqual(currency.as_dict(), currency)
+
+	def test_thread_locals(self):
+		frappe.flags.temp_flag_to_be_discarded = True
+
+	def test_temp_setting_changes(self):
+		current_setting = frappe.get_system_settings("logout_on_password_reset")
+
+		with change_settings("System Settings", {"logout_on_password_reset": int(not current_setting)}):
+			updated_settings = frappe.get_system_settings("logout_on_password_reset")
+			self.assertNotEqual(current_setting, updated_settings)
+
+		restored_settings = frappe.get_system_settings("logout_on_password_reset")
+		self.assertEqual(current_setting, restored_settings)
+
+
+def tearDownModule():
+	"""assertions for ensuring tests didn't leave state behind"""
+	assert "temp_flag_to_be_discarded" not in frappe.flags
+	assert not frappe.db.exists("Currency", "STONKS")

frappe/tests/utils.py

@@ -1,23 +1,87 @@
 import copy
+import datetime
 import signal
 import unittest
 from contextlib import contextmanager
 
 import frappe
+from frappe.model.base_document import BaseDocument
+from frappe.utils import cint
+
+datetime_like_types = (datetime.datetime, datetime.date, datetime.time, datetime.timedelta)
 
 
 class FrappeTestCase(unittest.TestCase):
 	"""Base test class for Frappe tests."""
 
-	@classmethod
-	def setUpClass(cls) -> None:
-		frappe.db.commit()
-		return super().setUpClass()
+	SHOW_TRANSACTION_COMMIT_WARNINGS = False
 
 	@classmethod
-	def tearDownClass(cls) -> None:
-		frappe.db.rollback()
-		return super().tearDownClass()
+	def setUpClass(cls) -> None:
+		# flush changes done so far to avoid flake
+		frappe.db.commit()
+		frappe.db.begin()
+		if cls.SHOW_TRANSACTION_COMMIT_WARNINGS:
+			frappe.db.add_before_commit(_commit_watcher)
+
+		# enqueue teardown actions (executed in LIFO order)
+		cls.addClassCleanup(_restore_thread_locals, copy.deepcopy(frappe.local.flags))
+		cls.addClassCleanup(_rollback_db)
+
+		return super().setUpClass()
+
+	# --- Frappe Framework specific assertions
+	def assertDocumentEqual(self, expected, actual):
+		"""Compare a (partial) expected document with actual Document."""
+
+		if isinstance(expected, BaseDocument):
+			expected = expected.as_dict()
+
+		for field, value in expected.items():
+			if isinstance(value, list):
+				actual_child_docs = actual.get(field)
+				self.assertEqual(len(value), len(actual_child_docs), msg=f"{field} length should be same")
+				for exp_child, actual_child in zip(value, actual_child_docs):
+					self.assertDocumentEqual(exp_child, actual_child)
+			else:
+				self._compare_field(value, actual.get(field), actual, field)
+
+	def _compare_field(self, expected, actual, doc, field):
+		msg = f"{field} should be same."
+
+		if isinstance(expected, float):
+			precision = doc.precision(field)
+			self.assertAlmostEqual(expected, actual, f"{field} should be same to {precision} digits")
+		elif isinstance(expected, (bool, int)):
+			self.assertEqual(expected, cint(actual), msg=msg)
+		elif isinstance(expected, datetime_like_types):
+			self.assertEqual(str(expected), str(actual), msg=msg)
+		else:
+			self.assertEqual(expected, actual, msg=msg)
+
+
+def _commit_watcher():
+	import traceback
+
+	print("Warning:, transaction committed during tests.")
+	traceback.print_stack(limit=5)
+
+
+def _rollback_db():
+	frappe.local.before_commit = []
+	frappe.local.rollback_observers = []
+	frappe.db.value_cache = {}
+	frappe.db.rollback()
+
+
+def _restore_thread_locals(flags):
+	frappe.local.flags = flags
+	frappe.local.error_log = []
+	frappe.local.message_log = []
+	frappe.local.debug_log = []
+	frappe.local.realtime_log = []
+	frappe.local.conf = frappe._dict(frappe.get_site_config())
+	frappe.local.cache = {}
 
 
 @contextmanager

frappe/website/doctype/blog_post/test_blog_post.py

@@ -1,12 +1,12 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # License: MIT. See LICENSE
 import re
-import unittest
 
 from bs4 import BeautifulSoup
 
 import frappe
 from frappe.custom.doctype.customize_form.customize_form import reset_customization
+from frappe.tests.utils import FrappeTestCase
 from frappe.utils import random_string, set_request
 from frappe.website.doctype.blog_post.blog_post import get_blog_list
 from frappe.website.serve import get_response
@@ -16,7 +16,7 @@ from frappe.website.website_generator import WebsiteGenerator
 test_dependencies = ["Blog Post"]
 
 
-class TestBlogPost(unittest.TestCase):
+class TestBlogPost(FrappeTestCase):
 	def setUp(self):
 		reset_customization("Blog Post")
 
@@ -61,7 +61,7 @@ class TestBlogPost(unittest.TestCase):
 		category_page_link = list(soup.find_all("a", href=re.compile(blog.blog_category)))[0]
 		category_page_url = category_page_link["href"]
 
-		cached_value = frappe.db.value_cache[("DocType", "Blog Post", "name")]
+		cached_value = frappe.db.value_cache.get(("DocType", "Blog Post", "name"))
 		frappe.db.value_cache[("DocType", "Blog Post", "name")] = (("Blog Post",),)
 
 		# Visit the category page (by following the link found in above stage)