refactor: Tests cleanup and better test isolation (#452)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/django_components/__init__.py

@@ -2,6 +2,7 @@ import importlib
 import importlib.util
 import os
 from pathlib import Path
+from typing import Callable, List, Optional
 
 import django
 from django.conf import settings
@@ -14,9 +15,15 @@ if django.VERSION < (3, 2):
     default_app_config = "django_components.apps.ComponentsConfig"
 
 
-def autodiscover() -> None:
+def autodiscover(map_import_paths: Optional[Callable[[str], str]] = None) -> List[str]:
+    """
+    Search for component files and import them. Returns a list of module
+    paths of imported files.
+    """
     from django_components.app_settings import app_settings
 
+    imported_modules: List[str] = []
+
     if app_settings.AUTODISCOVER:
         # Autodetect a components.py file in each app directory
         autodiscover_modules("components")
@@ -26,15 +33,21 @@ def autodiscover() -> None:
         logger.debug(f"Autodiscover found {len(component_filepaths)} files in component directories.")
 
         for path in component_filepaths:
+            module_name = _filepath_to_python_module(path)
+            if map_import_paths:
+                module_name = map_import_paths(module_name)
+
             # This imports the file and runs it's code. So if the file defines any
             # django components, they will be registered.
-            module_name = _filepath_to_python_module(path)
             logger.debug(f'Importing module "{module_name}" (derived from path "{path}")')
             importlib.import_module(module_name)
+            imported_modules.append(module_name)
 
     for path_lib in app_settings.LIBRARIES:
         importlib.import_module(path_lib)
 
+    return imported_modules
+
 
 def _filepath_to_python_module(file_path: Path) -> str:
     """

tests/test_component.py

@@ -1,5 +1,5 @@
+import sys
 from pathlib import Path
-from textwrap import dedent
 from typing import Any, Dict, Optional
 
 from django.core.exceptions import ImproperlyConfigured
@@ -8,14 +8,48 @@ from django.test import override_settings
 
 # isort: off
 from .django_test_setup import *  # NOQA
-from .testutils import BaseTestCase
+from .testutils import BaseTestCase, autodiscover_with_cleanup
 
 # isort: on
 
 from django_components import component
 
+#########################
+# COMPONENTS
+#########################
+
+
+class ParentComponent(component.Component):
+    template_name = "parent_template.html"
+
+    def get_context_data(self):
+        return {"shadowing_variable": "NOT SHADOWED"}
+
+
+class VariableDisplay(component.Component):
+    template_name = "variable_display.html"
+
+    def get_context_data(self, shadowing_variable=None, new_variable=None):
+        context = {}
+        if shadowing_variable is not None:
+            context["shadowing_variable"] = shadowing_variable
+        if new_variable is not None:
+            context["unique_variable"] = new_variable
+        return context
+
+
+#########################
+# TESTS
+#########################
+
 
 class ComponentTest(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="parent_component", component=ParentComponent)
+        component.registry.register(name="variable_display", component=VariableDisplay)
+
     def test_empty_component(self):
         class EmptyComponent(component.Component):
             pass
@@ -41,21 +75,17 @@ class ComponentTest(BaseTestCase):
 
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
             """
                 <link href="style.css" media="all" rel="stylesheet">
                 <script src="script.js"></script>
-        """
+            """,
-            ).strip(),
         )
 
         self.assertHTMLEqual(
             comp.render(context),
-            dedent(
             """
             Variable: <strong>test</strong>
-        """
+            """,
-            ).lstrip(),
         )
 
     def test_css_only_component(self):
@@ -69,11 +99,9 @@ class ComponentTest(BaseTestCase):
 
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
             """
             <link href="style.css" media="all" rel="stylesheet">
-        """
+            """,
-            ).strip(),
         )
 
     def test_js_only_component(self):
@@ -87,11 +115,9 @@ class ComponentTest(BaseTestCase):
 
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
             """
             <script src="script.js"></script>
-        """
+            """,
-            ).strip(),
         )
 
     def test_empty_media_component(self):
@@ -123,14 +149,12 @@ class ComponentTest(BaseTestCase):
 
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
             """
             <link href="style.css" media="all" rel="stylesheet">
             <link href="style2.css" media="all" rel="stylesheet">
             <script src="script.js"></script>
             <script src="script2.js"></script>
-        """
+            """,
-            ).strip(),
         )
 
     def test_component_with_filtered_template(self):
@@ -148,12 +172,10 @@ class ComponentTest(BaseTestCase):
 
         self.assertHTMLEqual(
             comp.render(context),
-            dedent(
             """
             Var1: <strong>test1</strong>
             Var2 (uppercased): <strong>TEST2</strong>
-        """
+            """,
-            ).lstrip(),
         )
 
     def test_component_with_dynamic_template(self):
@@ -172,24 +194,31 @@ class ComponentTest(BaseTestCase):
         comp = SvgComponent("svg_component")
         self.assertHTMLEqual(
             comp.render(Context(comp.get_context_data(name="dynamic1"))),
-            dedent(
-                """\
-                <svg>Dynamic1</svg>
             """
-            ),
+            <svg>Dynamic1</svg>
+            """,
         )
         self.assertHTMLEqual(
             comp.render(Context(comp.get_context_data(name="dynamic2"))),
-            dedent(
-                """\
-                <svg>Dynamic2</svg>
             """
-            ),
+            <svg>Dynamic2</svg>
+            """,
         )
 
-    def test_component_with_relative_paths_as_subcomponent(
+    # Settings required for autodiscover to work
-        self,
+    @override_settings(
-    ):
+        BASE_DIR=Path(__file__).resolve().parent,
+        STATICFILES_DIRS=[
+            Path(__file__).resolve().parent / "components",
+        ],
+    )
+    def test_component_media_with_dict_with_relative_paths(self):
+        # Ensure that the module is executed again after import in autodiscovery
+        if "tests.components.relative_file.relative_file" in sys.modules:
+            del sys.modules["tests.components.relative_file.relative_file"]
+
+        # Fix the paths, since the "components" dir is nested
+        with autodiscover_with_cleanup(map_import_paths=lambda p: f"tests.{p}"):
             template = Template(
                 """
                 {% load component_tags %}{% component_dependencies %}
@@ -202,7 +231,6 @@ class ComponentTest(BaseTestCase):
                 """  # NOQA
             )
             rendered = template.render(Context({}))
-
             self.assertIn('<input type="text" name="variable" value="hello">', rendered, rendered)
 
     def test_component_inside_slot(self):
@@ -360,12 +388,10 @@ class InlineComponentTest(BaseTestCase):
         )
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <script src="path/to/script.js"></script>
-            """
+            """,
-            ),
         )
 
     def test_html_js_string_with_css_file(self):
@@ -383,12 +409,10 @@ class InlineComponentTest(BaseTestCase):
         )
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <script>console.log('HTML and JS only');</script>
-                """
+            """,
-            ),
         )
 
     def test_html_css_string_with_js_file(self):
@@ -406,11 +430,9 @@ class InlineComponentTest(BaseTestCase):
         )
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
-                """\
-                <style>.html-string-file { color: blue; }</style><script src="path/to/script.js"></script>
             """
-            ),
+            <style>.html-string-file { color: blue; }</style><script src="path/to/script.js"></script>
+            """,
         )
 
     def test_component_with_variable_in_html(self):
@@ -436,12 +458,10 @@ class ComponentMediaTests(BaseTestCase):
         comp = SimpleComponent("")
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <script src="path/to/script.js"></script>
-            """
+            """,
-            ),
         )
 
     def test_component_media_with_lists(self):
@@ -453,13 +473,11 @@ class ComponentMediaTests(BaseTestCase):
         comp = SimpleComponent("")
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <link href="path/to/style2.css" media="all" rel="stylesheet">
             <script src="path/to/script.js"></script>
-            """
+            """,
-            ),
         )
 
     def test_component_media_with_dict_and_list(self):
@@ -475,14 +493,12 @@ class ComponentMediaTests(BaseTestCase):
         comp = SimpleComponent("")
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <link href="path/to/style2.css" media="print" rel="stylesheet">
             <link href="path/to/style3.css" media="screen" rel="stylesheet">
             <script src="path/to/script.js"></script>
-            """
+            """,
-            ),
         )
 
     def test_component_media_with_dict_with_list_and_list(self):
@@ -494,14 +510,13 @@ class ComponentMediaTests(BaseTestCase):
         comp = SimpleComponent("")
         self.assertHTMLEqual(
             comp.render_dependencies(),
-            dedent(
+            """
-                """\
             <link href="path/to/style.css" media="all" rel="stylesheet">
             <script src="path/to/script.js"></script>
-            """
+            """,
-            ),
         )
 
+    # Settings required for autodiscover to work
     @override_settings(
         BASE_DIR=Path(__file__).resolve().parent,
         STATICFILES_DIRS=[
@@ -509,24 +524,21 @@ class ComponentMediaTests(BaseTestCase):
         ],
     )
     def test_component_media_with_dict_with_relative_paths(self):
-        from .components.relative_file.relative_file import RelativeFileComponent
+        # Fix the paths, since the "components" dir is nested
-
+        with autodiscover_with_cleanup(map_import_paths=lambda p: f"tests.{p}"):
-        comp = RelativeFileComponent("")
+            template = Template(
-
-        self.assertHTMLEqual(
-            comp.render_dependencies(),
-            dedent(
-                """\
-                <link href="relative_file/relative_file.css" media="all" rel="stylesheet">
-                <script src="relative_file/relative_file.js"></script>
                 """
-            ),
+                {% load component_tags %}{% component_dependencies %}
+                {% component name='relative_file_component' variable=variable %}
+                {% endcomponent %}
+                """  # NOQA
             )
-
+            rendered = template.render(Context({"variable": "test"}))
-        rendered = comp.render(Context(comp.get_context_data(variable="test")))
             self.assertHTMLEqual(
                 rendered,
                 """
+                <link href="relative_file/relative_file.css" media="all" rel="stylesheet">
+                <script src="relative_file/relative_file.js"></script>
                 <form method="post">
                     <input type="text" name="variable" value="test">
                     <input type="submit">

tests/test_component_as_view.py

@@ -13,8 +13,11 @@ from .testutils import BaseTestCase
 
 from django_components import component
 
+#########################
+# COMPONENTS
+#########################
+
 
-@component.register("testcomponent")
 class MockComponentRequest(component.Component):
     template = """
         <form method="post">
@@ -35,7 +38,6 @@ class MockComponentRequest(component.Component):
         return {"variable": variable}
 
 
-@component.register("testcomponent_slot")
 class MockComponentSlot(component.Component):
     template = """
         {% load component_tags %}
@@ -52,7 +54,6 @@ class MockComponentSlot(component.Component):
         return self.render_to_response({"name": "Bob"}, {"second_slot": "Nice to meet you, Bob"})
 
 
-@component.register("testcomponent_context_insecure")
 class MockInsecureComponentContext(component.Component):
     template = """
         {% load component_tags %}
@@ -65,7 +66,6 @@ class MockInsecureComponentContext(component.Component):
         return self.render_to_response({"variable": "<script>alert(1);</script>"})
 
 
-@component.register("testcomponent_slot_insecure")
 class MockInsecureComponentSlot(component.Component):
     template = """
         {% load component_tags %}
@@ -110,7 +110,19 @@ class CustomClient(Client):
         super().__init__(*args, **kwargs)
 
 
+#########################
+# TESTS
+#########################
+
+
 class TestComponentAsView(BaseTestCase):
+    @classmethod
+    def setUpClass(self):
+        component.registry.register("testcomponent", MockComponentRequest)
+        component.registry.register("testcomponent_slot", MockComponentSlot)
+        component.registry.register("testcomponent_context_insecure", MockInsecureComponentContext)
+        component.registry.register("testcomponent_slot_insecure", MockInsecureComponentSlot)
+
     def setUp(self):
         self.client = CustomClient()
 
@@ -159,7 +171,7 @@ class TestComponentAsView(BaseTestCase):
         )
 
     def test_replace_context_in_view_with_insecure_content(self):
-        response = self.client.get("/test_slot_insecure/")
+        response = self.client.get("/test_context_insecure/")
         self.assertEqual(response.status_code, 200)
         self.assertNotIn(
             b"<script>",

tests/test_context.py

@@ -8,6 +8,10 @@ from django_components import component
 from .django_test_setup import *  # NOQA
 from .testutils import BaseTestCase
 
+#########################
+# COMPONENTS
+#########################
+
 
 class SimpleComponent(component.Component):
     template_name = "simple_template.html"
@@ -69,15 +73,18 @@ class OuterContextComponent(component.Component):
         return self.outer_context.flatten()
 
 
-component.registry.register(name="parent_component", component=ParentComponent)
+#########################
-component.registry.register(name="parent_with_args", component=ParentComponentWithArgs)
+# TESTS
-component.registry.register(name="variable_display", component=VariableDisplay)
+#########################
-component.registry.register(name="incrementer", component=IncrementerComponent)
-component.registry.register(name="simple_component", component=SimpleComponent)
-component.registry.register(name="outer_context_component", component=OuterContextComponent)
 
 
 class ContextTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="variable_display", component=VariableDisplay)
+        component.registry.register(name="parent_component", component=ParentComponent)
+
     def test_nested_component_context_shadows_parent_with_unfilled_slots_and_component_tag(
         self,
     ):
@@ -199,6 +206,13 @@ class ContextTests(BaseTestCase):
 
 
 class ParentArgsTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="incrementer", component=IncrementerComponent)
+        component.registry.register(name="parent_with_args", component=ParentComponentWithArgs)
+        component.registry.register(name="variable_display", component=VariableDisplay)
+
     def test_parent_args_can_be_drawn_from_context(self):
         template = Template(
             "{% load component_tags %}{% component_dependencies %}"
@@ -242,6 +256,11 @@ class ParentArgsTests(BaseTestCase):
 
 
 class ContextCalledOnceTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="incrementer", component=IncrementerComponent)
+
     def test_one_context_call_with_simple_component(self):
         template = Template(
             "{% load component_tags %}{% component_dependencies %}"
@@ -250,8 +269,6 @@ class ContextCalledOnceTests(BaseTestCase):
         rendered = template.render(Context()).strip().replace("\n", "")
         self.assertHTMLEqual(
             rendered,
-            '<link href="relative_file/relative_file.css" media="all" rel="stylesheet">'
-            '<script src="relative_file/relative_file.js"></script>'
             '<p class="incrementer">value=1;calls=1</p>',
         )
 
@@ -303,6 +320,11 @@ class ContextCalledOnceTests(BaseTestCase):
 
 
 class ComponentsCanAccessOuterContext(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="simple_component", component=SimpleComponent)
+
     def test_simple_component_can_use_outer_context(self):
         template = Template(
             "{% load component_tags %}{% component_dependencies %}"
@@ -313,6 +335,11 @@ class ComponentsCanAccessOuterContext(BaseTestCase):
 
 
 class IsolatedContextTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="simple_component", component=SimpleComponent)
+
     def test_simple_component_can_pass_outer_context_in_args(self):
         template = Template(
             "{% load component_tags %}{% component_dependencies %}"
@@ -331,6 +358,11 @@ class IsolatedContextTests(BaseTestCase):
 
 
 class IsolatedContextSettingTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="simple_component", component=SimpleComponent)
+
     def setUp(self):
         self.patcher = patch(
             "django_components.app_settings.AppSettings.CONTEXT_BEHAVIOR",
@@ -386,6 +418,11 @@ class IsolatedContextSettingTests(BaseTestCase):
 
 
 class OuterContextPropertyTests(BaseTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register(name="outer_context_component", component=OuterContextComponent)
+
     @override_settings(
         COMPONENTS={"context_behavior": "global"},
     )

tests/testutils.py

@@ -1,3 +1,5 @@
+import contextlib
+import sys
 from typing import List
 from unittest.mock import Mock
 
@@ -5,6 +7,8 @@ from django.template import Context, Node
 from django.template.response import TemplateResponse
 from django.test import SimpleTestCase
 
+from django_components import autodiscover
+from django_components.component_registry import registry
 from django_components.middleware import ComponentDependencyMiddleware
 
 # Create middleware instance
@@ -12,9 +16,11 @@ response_stash = None
 middleware = ComponentDependencyMiddleware(get_response=lambda _: response_stash)
 
 
-# TODO: Use this class to manage component registry cleanup before/after tests.
 class BaseTestCase(SimpleTestCase):
-    pass
+    @classmethod
+    def setUpClass(self) -> None:
+        registry.clear()
+        return super().setUpClass()
 
 
 request = Mock()
@@ -52,3 +58,20 @@ def print_nodes(nodes: List[Node], indent=0) -> None:
         print(repr)
         if child_nodes:
             print_nodes(child_nodes, indent=indent + 1)
+
+
+# TODO: Make sure that this is done before/after each test automatically?
+@contextlib.contextmanager
+def autodiscover_with_cleanup(*args, **kwargs):
+    """
+    Use this in place of regular `autodiscover` in test files to ensure that
+    the autoimport does not pollute the global state.
+    """
+    imported_modules = autodiscover(*args, **kwargs)
+    try:
+        yield imported_modules
+    finally:
+        # Teardown - delete autoimported modules, so the module is executed also the
+        # next time one of the tests calls `autodiscover`.
+        for mod in imported_modules:
+            del sys.modules[mod]