From bc8aa1c894c0cf020fe9340591514106bedfddb8 Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Sun, 25 Feb 2024 16:20:49 +0100 Subject: [PATCH] Use cleanup hook to reload django views - reload only the django view modules instead of all modules - change the module cleanup mode default to always delete modules (avoiding problems with reloading other modules) - closes #932 --- CHANGES.md | 1 + docs/usage.rst | 28 +++++++++------- pyfakefs/fake_filesystem_unittest.py | 28 +++++----------- pyfakefs/patched_packages.py | 49 ++++++++++++++++++++++++++-- 4 files changed, 74 insertions(+), 32 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a73628b3..7925a1d1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ The released versions correspond to PyPI releases. ### Fixes * Fixed a specific problem on reloading a pandas-related module (see [#947](../../issues/947)), added possibility for unload hooks for specific modules +* Use this also to reload django views (see [#932](../../issues/932)) ## [Version 5.3.5](https://pypi.python.org/pypi/pyfakefs/5.3.5) (2024-01-30) Fixes a regression. diff --git a/docs/usage.rst b/docs/usage.rst index 55c84d3a..5332541b 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -675,24 +675,30 @@ use_dynamic_patch ~~~~~~~~~~~~~~~~~ If ``True`` (the default), dynamic patching after setup is used (for example for modules loaded locally inside of functions). -Can be switched off if it causes unwanted side effects, which happened at least in -once instance while testing a django project. +Can be switched off if it causes unwanted side effects, though that would mean that +dynamically loaded modules are no longer patched, if they use file system functions. module_cleanup_mode ~~~~~~~~~~~~~~~~~~~ This is a setting that works around a potential problem with the cleanup of -dynamically loaded modules (e.g. modules loaded after the test has started), -known to occur with `django` applications. -The setting is subject to change or removal in future versions, provided a better -solution for the problem is found. - -The setting defines how the dynamically loaded modules are cleaned up after the test -to ensure that no patched modules can be used after the test has finished. -The default (ModuleCleanupMode.AUTO) currently depends on the availability of the `django` module, -DELETE will delete all dynamically loaded modules and RELOAD will reload them. +dynamically loaded modules (e.g. modules loaded after the test has started). +As the original problem related to `django` has now been resolved in another way (see below), +the setting may not be needed anymore, and is subject to removal in a future version. + +The setting defines how the dynamically loaded modules are cleaned up after the test. +Any dynamically loaded module is cleaned up after the test to ensure that no patched modules +can be used after that. +The default (`ModuleCleanupMode.AUTO`) is currently the same as `ModuleCleanupMode.DELETE`. +`DELETE` will delete all dynamically loaded modules (so that they are reloaded the next time +they are needed), while `RELOAD` will reload them immediately. Under some rare conditions, changing this setting may help to avoid problems related to incorrect test cleanup. +Problems with unloading dynamically loaded modules may also be solved more +specifically by registering a handler for specific modules (using `Patcher.register_cleanup_handler`), +that will be called during the cleanup process. This is used internally +to handle known problems with the `django` and `pandas` packages. + .. _convenience_methods: Using convenience methods diff --git a/pyfakefs/fake_filesystem_unittest.py b/pyfakefs/fake_filesystem_unittest.py index a05c34c7..4af0f620 100644 --- a/pyfakefs/fake_filesystem_unittest.py +++ b/pyfakefs/fake_filesystem_unittest.py @@ -619,7 +619,7 @@ def __init__( self.use_cache = use_cache self.use_dynamic_patch = use_dynamic_patch self.module_cleanup_mode = module_cleanup_mode - self.cleanup_handlers: Dict[str, Callable[[], bool]] = {} + self.cleanup_handlers: Dict[str, Callable[[str], bool]] = {} if use_known_patches: from pyfakefs.patched_packages import ( @@ -683,7 +683,7 @@ def clear_cache(self) -> None: """Clear the module cache (convenience instance method).""" self.__class__.clear_fs_cache() - def register_cleanup_handler(self, name: str, handler: Callable[[], bool]): + def register_cleanup_handler(self, name: str, handler: Callable[[str], bool]): """Register a handler for cleaning up a module after it had been loaded by the dynamic patcher. This allows to handle modules that cannot be reloaded without unwanted side effects. @@ -965,9 +965,7 @@ def start_patching(self) -> None: self.patch_functions() self.patch_defaults() - self._dyn_patcher = DynamicPatcher( - self, cleanup_handlers=self.cleanup_handlers - ) + self._dyn_patcher = DynamicPatcher(self) sys.meta_path.insert(0, self._dyn_patcher) for module in self.modules_to_reload: if sys.modules.get(module.__name__) is module: @@ -1127,16 +1125,12 @@ class DynamicPatcher(MetaPathFinder, Loader): Implements the protocol needed for import hooks. """ - def __init__( - self, - patcher: Patcher, - cleanup_handlers: Optional[Dict[str, Callable[[], bool]]] = None, - ) -> None: + def __init__(self, patcher: Patcher) -> None: self._patcher = patcher self.sysmodules = {} self.modules = self._patcher.fake_modules self._loaded_module_names: Set[str] = set() - self.cleanup_handlers = cleanup_handlers or {} + self.cleanup_handlers = patcher.cleanup_handlers # remove all modules that have to be patched from `sys.modules`, # otherwise the find_... methods will not be called @@ -1159,17 +1153,13 @@ def cleanup(self, cleanup_mode: ModuleCleanupMode) -> None: ] # Delete all modules loaded during the test, ensuring that # they are reloaded after the test. - # If cleanup_mode is set to RELOAD, or it is AUTO and django is imported, - # reload the modules instead - this is a workaround related to some internal - # module caching by django, that will likely change in the future. + # If cleanup_mode is set to RELOAD, reload the modules instead. + # This is probably not needed anymore with the cleanup handlers in place. if cleanup_mode == ModuleCleanupMode.AUTO: - if "django" in sys.modules: - cleanup_mode = ModuleCleanupMode.RELOAD - else: - cleanup_mode = ModuleCleanupMode.DELETE + cleanup_mode = ModuleCleanupMode.DELETE for name in self._loaded_module_names: if name in sys.modules and name not in reloaded_module_names: - if name in self.cleanup_handlers and self.cleanup_handlers[name](): + if name in self.cleanup_handlers and self.cleanup_handlers[name](name): continue if cleanup_mode == ModuleCleanupMode.RELOAD: try: diff --git a/pyfakefs/patched_packages.py b/pyfakefs/patched_packages.py index 9ae3ed87..9f343e0c 100644 --- a/pyfakefs/patched_packages.py +++ b/pyfakefs/patched_packages.py @@ -15,6 +15,7 @@ with pyfakefs. """ import sys +from importlib import reload try: import pandas as pd @@ -33,9 +34,16 @@ except ImportError: xlrd = None + try: - from django.core.files import locks + import django + + try: + from django.core.files import locks + except ImportError: + locks = None except ImportError: + django = None locks = None # From pandas v 1.2 onwards the python fs functions are used even when the engine @@ -63,12 +71,21 @@ def get_classes_to_patch(): return classes_to_patch +def reload_handler(name): + if name in sys.modules: + reload(sys.modules[name]) + return True + + def get_cleanup_handlers(): handlers = {} if pd is not None: handlers["pandas.core.arrays.arrow.extension_types"] = ( handle_extension_type_cleanup ) + if django is not None: + for module_name in django_view_modules(): + handlers[module_name] = lambda name=module_name: reload_handler(name) return handlers @@ -151,7 +168,7 @@ def __getattr__(self, name): if pd is not None: - def handle_extension_type_cleanup(): + def handle_extension_type_cleanup(_name): # the module registers two extension types on load # on reload it raises if the extensions have not been unregistered before try: @@ -186,3 +203,31 @@ def unlock(f): def __getattr__(self, name): return getattr(self._locks_module, name) + + +if django is not None: + + def get_all_view_modules(urlpatterns, modules=None): + if modules is None: + modules = set() + for pattern in urlpatterns: + if hasattr(pattern, "url_patterns"): + get_all_view_modules(pattern.url_patterns, modules=modules) + else: + if hasattr(pattern.callback, "cls"): + view = pattern.callback.cls + elif hasattr(pattern.callback, "view_class"): + view = pattern.callback.view_class + else: + view = pattern.callback + modules.add(view.__module__) + return modules + + def django_view_modules(): + try: + all_urlpatterns = __import__( + django.conf.settings.ROOT_URLCONF + ).urls.urlpatterns + return get_all_view_modules(all_urlpatterns) + except Exception: + return set()