From 83bc1f09e99eeacb71a2a96d001f9675cd447b69 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Wed, 9 Oct 2024 15:44:27 +0200 Subject: [PATCH] refactor: clarify test record dep management in test modules (#28060) --- .../contacts/doctype/contact/test_contact.py | 2 +- frappe/core/doctype/docshare/test_docshare.py | 2 +- .../doctype/boilerplate/test_controller._py | 7 ++ frappe/core/doctype/report/test_report.py | 2 +- .../doctype/role_profile/test_role_profile.py | 2 +- .../customize_form/test_customize_form.py | 2 +- frappe/deprecation_dumpster.py | 54 ++++++-- frappe/desk/doctype/todo/test_todo.py | 2 +- .../doctype/notification/test_notification.py | 2 +- .../doctype/token_cache/test_token_cache.py | 2 +- frappe/tests/classes/integration_test_case.py | 9 +- frappe/tests/test_db_query.py | 2 +- frappe/tests/test_email.py | 2 +- frappe/tests/test_form_load.py | 2 +- frappe/tests/test_permissions.py | 2 +- frappe/tests/utils/__init__.py | 3 + frappe/tests/utils/generators.py | 115 ++++++++++++------ .../doctype/blog_post/test_blog_post.py | 2 +- .../website/doctype/web_form/test_web_form.py | 2 +- .../test_website_route_meta.py | 2 +- 20 files changed, 153 insertions(+), 65 deletions(-) diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index bcb9cf7a03..69d7b6e440 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -5,7 +5,7 @@ from frappe.contacts.doctype.contact.contact import get_full_name from frappe.email import get_contact_list from frappe.tests import IntegrationTestCase, UnitTestCase -test_dependencies = ["Contact", "Salutation"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Contact", "Salutation"] class UnitTestContact(UnitTestCase): diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py index 48dcf87e54..c59592a7da 100644 --- a/frappe/core/doctype/docshare/test_docshare.py +++ b/frappe/core/doctype/docshare/test_docshare.py @@ -6,7 +6,7 @@ import frappe.share from frappe.automation.doctype.auto_repeat.test_auto_repeat import create_submittable_doctype from frappe.tests import IntegrationTestCase, UnitTestCase -test_dependencies = ["User"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User"] class UnitTestDocshare(UnitTestCase): diff --git a/frappe/core/doctype/doctype/boilerplate/test_controller._py b/frappe/core/doctype/doctype/boilerplate/test_controller._py index 550d26ad38..cbc58e587b 100644 --- a/frappe/core/doctype/doctype/boilerplate/test_controller._py +++ b/frappe/core/doctype/doctype/boilerplate/test_controller._py @@ -5,6 +5,13 @@ from frappe.tests import IntegrationTestCase, UnitTestCase +# On IntegrationTestCase, the doctype test records and all +# link-field test record depdendencies are recursively loaded +# Use these module variables to add/remove to/from that list +EXTRA_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] +IGNORE_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] + + class Test{classname}(UnitTestCase): """ Unit tests for {classname}. diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index c1f0f22ecf..a0898a8ea7 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -14,7 +14,7 @@ from frappe.desk.reportview import save_report as _save_report from frappe.tests import IntegrationTestCase, UnitTestCase test_records = frappe.get_test_records("Report") -test_dependencies = ["User"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User"] class UnitTestReport(UnitTestCase): diff --git a/frappe/core/doctype/role_profile/test_role_profile.py b/frappe/core/doctype/role_profile/test_role_profile.py index fe0575549f..a80b4f8e87 100644 --- a/frappe/core/doctype/role_profile/test_role_profile.py +++ b/frappe/core/doctype/role_profile/test_role_profile.py @@ -3,7 +3,7 @@ import frappe from frappe.tests import IntegrationTestCase, UnitTestCase -test_dependencies = ["Role"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Role"] class UnitTestRoleProfile(UnitTestCase): diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index 2a1e7fb4b0..90608fd26f 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -9,7 +9,7 @@ from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.tests import IntegrationTestCase, UnitTestCase from frappe.tests.utils import make_test_records_for_doctype -test_dependencies = ["Custom Field", "Property Setter"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Custom Field", "Property Setter"] class UnitTestCustomizeForm(UnitTestCase): diff --git a/frappe/deprecation_dumpster.py b/frappe/deprecation_dumpster.py index df0278289b..c8a4291e69 100644 --- a/frappe/deprecation_dumpster.py +++ b/frappe/deprecation_dumpster.py @@ -290,15 +290,6 @@ def validate_roles(self): self.populate_role_profile_roles() -@deprecated( - "frappe.tests_runner.get_dependencies", "2024-20-08", "v17", "use frappe.tests.utils.get_dependencies" -) -def test_runner_get_dependencies(doctype): - from frappe.tests.utils import get_dependencies - - return get_dependencies(doctype) - - @deprecated("frappe.tests_runner.get_modules", "2024-20-08", "v17", "use frappe.tests.utils.get_modules") def test_runner_get_modules(doctype): from frappe.tests.utils import get_modules @@ -530,3 +521,48 @@ def model_trace_traced_field_context(*args, **kwargs): from frappe.tests.classes.context_managers import trace_fields return trace_fields(*args, **kwargs) + + +@deprecated( + "frappe.tests.utils.get_dependencies", + "2024-20-09", + "v17", + "refactor to use frappe.tests.utils.get_missing_records_doctypes", +) +def tests_utils_get_dependencies(doctype): + """Get the dependencies for the specified doctype""" + import frappe + from frappe.tests.utils.generators import get_modules + + module, test_module = get_modules(doctype) + meta = frappe.get_meta(doctype) + link_fields = meta.get_link_fields() + + for df in meta.get_table_fields(): + link_fields.extend(frappe.get_meta(df.options).get_link_fields()) + + options_list = [df.options for df in link_fields] + + if hasattr(test_module, "test_dependencies"): + options_list += test_module.test_dependencies + + options_list = list(set(options_list)) + + if hasattr(test_module, "test_ignore"): + for doctype_name in test_module.test_ignore: + if doctype_name in options_list: + options_list.remove(doctype_name) + + options_list.sort() + + return options_list + + +@deprecated( + "frappe.tests_runner.get_dependencies", + "2024-20-08", + "v17", + "refactor to use frappe.tests.utils.get_missing_record_doctypes", +) +def test_runner_get_dependencies(doctype): + return tests_utils_get_dependencies(doctype) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index dab34c4c2b..f5061b25f3 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -6,7 +6,7 @@ from frappe.model.db_query import DatabaseQuery from frappe.permissions import add_permission, reset_perms from frappe.tests import IntegrationTestCase, UnitTestCase -test_dependencies = ["User"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User"] class UnitTestTodo(UnitTestCase): diff --git a/frappe/email/doctype/notification/test_notification.py b/frappe/email/doctype/notification/test_notification.py index 986a3ae437..4ec82ee23c 100644 --- a/frappe/email/doctype/notification/test_notification.py +++ b/frappe/email/doctype/notification/test_notification.py @@ -12,7 +12,7 @@ from frappe.tests import IntegrationTestCase, UnitTestCase from .notification import trigger_notifications -test_dependencies = ["User", "Notification"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User", "Notification"] @contextmanager diff --git a/frappe/integrations/doctype/token_cache/test_token_cache.py b/frappe/integrations/doctype/token_cache/test_token_cache.py index 7d918c5d85..26eb4a287d 100644 --- a/frappe/integrations/doctype/token_cache/test_token_cache.py +++ b/frappe/integrations/doctype/token_cache/test_token_cache.py @@ -3,7 +3,7 @@ import frappe from frappe.tests import IntegrationTestCase, UnitTestCase -test_dependencies = ["User", "Connected App", "Token Cache"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User", "Connected App", "Token Cache"] class UnitTestTokenCache(UnitTestCase): diff --git a/frappe/tests/classes/integration_test_case.py b/frappe/tests/classes/integration_test_case.py index 613c0f88cc..46df7c35a0 100644 --- a/frappe/tests/classes/integration_test_case.py +++ b/frappe/tests/classes/integration_test_case.py @@ -5,7 +5,7 @@ from contextlib import AbstractContextManager, contextmanager import frappe from frappe.utils import cint -from ..utils.generators import make_test_records +from ..utils.generators import get_missing_records_module_overrides, make_test_records from .unit_test_case import UnitTestCase logger = logging.Logger(__file__) @@ -47,10 +47,11 @@ class IntegrationTestCase(UnitTestCase): cls._newly_created_test_records = [] if cls.doctype and cls.doctype not in frappe.local.test_objects: cls._newly_created_test_records += make_test_records(cls.doctype) - for doctype in getattr(cls.module, "test_dependencies", []): - if doctype not in frappe.local.test_objects: + elif not cls.doctype: + to_add, to_remove = get_missing_records_module_overrides(cls.module) + to_add.difference_update(to_remove) + for doctype in to_add: cls._newly_created_test_records += make_test_records(doctype) - # flush changes done so far to avoid flake frappe.db.commit() if cls.SHOW_TRANSACTION_COMMIT_WARNINGS: diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index e40b3a3b53..e634bcba31 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -18,7 +18,7 @@ from frappe.tests import IntegrationTestCase from frappe.tests.test_query_builder import db_type_is, run_only_if from frappe.utils.testutils import add_custom_field, clear_custom_fields -test_dependencies = ["User", "Blog Post", "Blog Category", "Blogger"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["User", "Blog Post", "Blog Category", "Blogger"] @contextmanager diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index 74a4cb6ae3..f0c1a8dd33 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -16,7 +16,7 @@ from frappe.query_builder.utils import db_type_is from frappe.tests import IntegrationTestCase from frappe.tests.test_query_builder import run_only_if -test_dependencies = ["Email Account"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Email Account"] class TestEmail(IntegrationTestCase): diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index bf410e1d6e..194ef28612 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -7,7 +7,7 @@ from frappe.desk.form.load import get_docinfo, getdoc, getdoctype from frappe.tests import IntegrationTestCase from frappe.utils.file_manager import save_file -test_dependencies = ["Blog Category", "Blogger"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Blog Category", "Blogger"] class TestFormLoad(IntegrationTestCase): diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index e6084acae3..a9ed74b552 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -26,7 +26,7 @@ from frappe.tests import IntegrationTestCase from frappe.tests.utils import make_test_records_for_doctype from frappe.utils.data import now_datetime -test_dependencies = ["Blogger", "Blog Post", "User", "Contact", "Salutation"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Blogger", "Blog Post", "User", "Contact", "Salutation"] class TestPermissions(IntegrationTestCase): diff --git a/frappe/tests/utils/__init__.py b/frappe/tests/utils/__init__.py index 914fdf264f..6dde9c9fcd 100644 --- a/frappe/tests/utils/__init__.py +++ b/frappe/tests/utils/__init__.py @@ -47,3 +47,6 @@ from frappe.deprecation_dumpster import ( from frappe.deprecation_dumpster import ( tests_UnitTestCase as UnitTestCase, ) +from frappe.deprecation_dumpster import ( + tests_utils_get_dependencies as get_dependencies, +) diff --git a/frappe/tests/utils/generators.py b/frappe/tests/utils/generators.py index feb29c9796..09731a1079 100644 --- a/frappe/tests/utils/generators.py +++ b/frappe/tests/utils/generators.py @@ -14,7 +14,8 @@ datetime_like_types = (datetime.datetime, datetime.date, datetime.time, datetime __all__ = [ "get_modules", - "get_dependencies", + "get_missing_records_doctypes", + "get_missing_records_module_overrides", "make_test_records", "make_test_records_for_doctype", "make_test_objects", @@ -36,8 +37,15 @@ def get_modules(doctype): @cache -def get_dependencies(doctype): - """Get the dependencies for the specified doctype""" +def get_missing_records_doctypes(doctype): + """Get the dependencies for the specified doctype in a depth-first manner""" + # If already visited in a prior run + if doctype in frappe.local.test_objects: + return [] + else: + # Infinite recrusion guard (depth-first discovery) + frappe.local.test_objects[doctype] = [] + module, test_module = get_modules(doctype) meta = frappe.get_meta(doctype) link_fields = meta.get_link_fields() @@ -45,21 +53,73 @@ def get_dependencies(doctype): for df in meta.get_table_fields(): link_fields.extend(frappe.get_meta(df.options).get_link_fields()) - options_list = [df.options for df in link_fields] + doctype_set = {df.options for df in link_fields if df.options != "[Select]"} - if hasattr(test_module, "test_dependencies"): - options_list += test_module.test_dependencies + to_add, to_remove = get_missing_records_module_overrides(test_module) + doctype_set.update(to_add) + doctype_set.difference_update(to_remove) - options_list = list(set(options_list)) + # Recursive depth-first traversal + result = [] + for dep_doctype in doctype_set: + result.extend(get_missing_records_doctypes(dep_doctype)) - if hasattr(test_module, "test_ignore"): - for doctype_name in test_module.test_ignore: - if doctype_name in options_list: - options_list.remove(doctype_name) + result.append(doctype) + return result - options_list.sort() - return options_list +def get_missing_records_module_overrides(module) -> [set, set]: + to_add = set() + to_remove = set() + if hasattr(module, "test_dependencies"): + from frappe.deprecation_dumpster import deprecation_warning + + deprecation_warning( + "2024-10-09", + "v17", + """test_dependencies was clarified to EXTRA_TEST_RECORD_DEPENDENCIES: run: +```bash +# Find Python files +find . -name "*.py" | while read -r file; do + # Check if the file contains 'test_dependencies' at the module level + if grep -q "^test_dependencies" "$file"; then + # Replace 'test_dependencies' with 'EXTRA_TEST_RECORD_DEPENDENCIES' + sed -i 's/^test_dependencies/EXTRA_TEST_RECORD_DEPENDENCIES/' "$file" + echo "Updated $file" + fi +done +```""", + ) + to_add.update(set(module.test_dependencies)) + + if hasattr(module, "EXTRA_TEST_RECORD_DEPENDENCIES"): + to_add.update(set(module.EXTRA_TEST_RECORD_DEPENDENCIES)) + + if hasattr(module, "test_ignore"): + from frappe.deprecation_dumpster import deprecation_warning + + deprecation_warning( + "2024-10-09", + "v17", + """test_ignore was clarified to IGNORE_TEST_RECORD_DEPENDENCIES: run: +```bash +# Find Python files +find . -name "*.py" | while read -r file; do + # Check if the file contains 'test_dependencies' at the module level + if grep -q "^test_ignore" "$file"; then + # Replace 'test_ignore' with 'IGNORE_TEST_RECORD_DEPENDENCIES' + sed -i 's/^test_ignore/IGNORE_TEST_RECORD_DEPENDENCIES/' "$file" + echo "Updated $file" + fi +done +```""", + ) + to_remove.difference_update(set(module.test_ignore)) + + if hasattr(module, "IGNORE_TEST_RECORD_DEPENDENCIES"): + to_remove.difference_update(set(module.IGNORE_TEST_RECORD_DEPENDENCIES)) + + return to_add, to_remove # Test record generation @@ -70,7 +130,7 @@ def make_test_records(doctype, force=False, commit=False): def make_test_records_for_doctype(doctype, force=False, commit=False): - return list(_make_test_records_for_doctype(doctype, force, commit)) + return list(_make_test_record(doctype, force, commit)) def make_test_objects(doctype, test_records=None, reset=False, commit=False): @@ -79,31 +139,12 @@ def make_test_objects(doctype, test_records=None, reset=False, commit=False): def _make_test_records(doctype, force=False, commit=False): """Make test records for the specified doctype""" - - loadme = False - - if doctype not in frappe.local.test_objects: - loadme = True - frappe.local.test_objects[doctype] = [] # infinite recursion guard, here - - # First, create test records for dependencies - for dependency in get_dependencies(doctype): - if dependency != "[Select]" and dependency not in frappe.local.test_objects: - yield from _make_test_records(dependency, force, commit) - - # Then, create test records for the doctype itself - if loadme: - # Yield the doctype and record length - yield ( - doctype, - len( - # Create all test records - list(_make_test_records_for_doctype(doctype, force, commit)) - ), - ) + for _doctype in get_missing_records_doctypes(doctype): + # Create all test records and yield + yield (_doctype, len(list(_make_test_record(_doctype, force, commit)))) -def _make_test_records_for_doctype(doctype, force=False, commit=False): +def _make_test_record(doctype, force=False, commit=False): """Make test records for the specified doctype""" test_record_log_instance = TestRecordLog() diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index 432ed1e343..ec9698bb6d 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -13,7 +13,7 @@ from frappe.website.serve import get_response from frappe.website.utils import clear_website_cache from frappe.website.website_generator import WebsiteGenerator -test_dependencies = ["Blog Post"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Blog Post"] class UnitTestBlogPost(UnitTestCase): diff --git a/frappe/website/doctype/web_form/test_web_form.py b/frappe/website/doctype/web_form/test_web_form.py index c91ba631b3..4896ef8390 100644 --- a/frappe/website/doctype/web_form/test_web_form.py +++ b/frappe/website/doctype/web_form/test_web_form.py @@ -8,7 +8,7 @@ from frappe.utils import set_request from frappe.website.doctype.web_form.web_form import accept from frappe.website.serve import get_response_content -test_dependencies = ["Web Form"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Web Form"] class UnitTestWebForm(UnitTestCase): diff --git a/frappe/website/doctype/website_route_meta/test_website_route_meta.py b/frappe/website/doctype/website_route_meta/test_website_route_meta.py index 4d8962d34c..74c1c5ffc8 100644 --- a/frappe/website/doctype/website_route_meta/test_website_route_meta.py +++ b/frappe/website/doctype/website_route_meta/test_website_route_meta.py @@ -5,7 +5,7 @@ from frappe.tests import IntegrationTestCase, UnitTestCase from frappe.utils import set_request from frappe.website.serve import get_response -test_dependencies = ["Blog Post"] +EXTRA_TEST_RECORD_DEPENDENCIES = ["Blog Post"] class UnitTestWebsiteRouteMeta(UnitTestCase):