Refactor redirect tests to use reload_redirects_with_settings instead of override_settings

Replaced @override_settings with a new @reload_redirects_with_settings decorator in test_redirects.py. This change ensures that redirect patterns are reloaded correctly during tests, addressing issues with Django's redirect loading behavior that were affecting redirects for Firefox.com. See #16410

Author: localjo

bedrock/firefox/tests/test_redirects.py

@@ -5,11 +5,12 @@
 from unittest.mock import patch
 
 from django.conf import settings
-from django.test import RequestFactory, override_settings
+from django.test import RequestFactory
 
 import pytest
 
 from bedrock.firefox.redirects import mobile_app, validate_param_value
+from tests.utils import reload_redirects_with_settings
 
 
 @pytest.mark.parametrize(
@@ -93,7 +94,7 @@ def test_mobile_app():
 EXPECTED_REDIRECT_QS = "?redirect_source=mozilla-org"
 
 
-@override_settings(ENABLE_FIREFOX_COM_REDIRECTS=True)
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "path,expected_location,expected_status,follow_redirects",
@@ -467,7 +468,7 @@ def test_springfield_redirect_patterns(
         assert response.headers["Location"] == expected_location
 
 
-@override_settings(ENABLE_FIREFOX_COM_REDIRECTS=True)
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "path,expected_location,expected_status,follow_redirects",
@@ -504,7 +505,7 @@ def test_springfield_redirects_carry_over_querystrings_and_add_redirect_source(
         assert response.headers["Location"] == expected_location
 
 
-@override_settings(ENABLE_FIREFOX_COM_REDIRECTS=True)
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "path",
@@ -528,6 +529,7 @@ def test_mobile_app_redirector_does_not_go_to_springfield(client):
     assert resp.headers["Location"] == "https://apps.apple.com/app/apple-store/id989804926"
 
 
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "path",
@@ -541,7 +543,6 @@ def test_mobile_app_redirector_does_not_go_to_springfield(client):
         "/firefox/releases/",
     ),
 )
-@override_settings(ENABLE_FIREFOX_COM_REDIRECTS=True)
 def test_releasenotes_generic_urls_not_rediected_to_springfield(client, path):
     resp = client.get(path)
     assert resp.status_code == 302
@@ -561,7 +562,7 @@ def test_releasenotes_generic_urls_not_rediected_to_springfield(client, path):
         ("/en-US/firefox/installer-help/", f"{settings.FXC_BASE_URL}/en-US/download/installer-help/{EXPECTED_REDIRECT_QS}"),
     ),
 )
-@override_settings(ENABLE_FIREFOX_COM_REDIRECTS=True)
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 def test_subsequent_redirects_do_not_carry_querystrings_from_earlier_requests(
     client,
     path,
@@ -584,6 +585,7 @@ def test_subsequent_redirects_do_not_carry_querystrings_from_earlier_requests(
         ("/firefox/browsers/incognito-browser/", f"{settings.FXC_BASE_URL}/more/incognito-browser/{EXPECTED_REDIRECT_QS}"),
     ),
 )
+@reload_redirects_with_settings("bedrock.firefox.redirects", ENABLE_FIREFOX_COM_REDIRECTS=True)
 def test_offsite_redirects_still_work_when_locale_not_in_source_path(
     client,
     path,

tests/__init__.py

@@ -0,0 +1,3 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at https://mozilla.org/MPL/2.0/.

tests/utils.py

@@ -0,0 +1,99 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+import functools
+import importlib
+from collections.abc import Callable
+from typing import Any
+
+from django.test import override_settings
+
+
+def reload_redirects_with_settings(module_path: str, **settings_kwargs):
+    """
+    Generic decorator that enables dynamic redirect reloading for testing.
+
+    This decorator:
+    1. Overrides specified settings using the same pattern as @override_settings
+    2. Reloads the specified redirects module to pick up new patterns
+    3. Patches the redirects middleware to use the reloaded patterns
+    4. Restores original state after the test
+
+    This is needed because Django loads redirects once at startup and doesn't
+    reload them when @override_settings is used.
+
+    Args:
+        module_path: The module path to reload (e.g., 'bedrock.firefox.redirects')
+        **settings_kwargs: Settings to override (e.g., ENABLE_FIREFOX_COM_REDIRECTS=True)
+
+    Usage:
+        @reload_redirects_with_settings('bedrock.firefox.redirects', ENABLE_FIREFOX_COM_REDIRECTS=True)
+        def test_my_redirect():
+            # Your test code here
+            pass
+    """
+
+    def decorator(func: Callable) -> Callable:
+        @functools.wraps(func)
+        def wrapper(*args: Any, **kwargs: Any) -> Any:
+            with override_settings(**settings_kwargs):
+                original_patterns, original_resolver = _patch_redirects_middleware(module_path)
+                try:
+                    return func(*args, **kwargs)
+                finally:
+                    _restore_redirects_middleware(module_path, original_patterns, original_resolver)
+
+        return wrapper
+
+    return decorator
+
+
+def _patch_redirects_middleware(module_path: str) -> tuple[Any, Any]:
+    """
+    Patch the redirects middleware to use reloaded patterns from the specified module.
+
+    Args:
+        module_path: The module path to reload
+
+    Returns:
+        Tuple of (original_patterns, original_resolver) for restoration
+    """
+    import bedrock.redirects.middleware
+
+    # Import and reload the specified module
+    module = importlib.import_module(module_path)
+    importlib.reload(module)  # type: ignore
+
+    # Store original state
+    original_patterns = module.redirectpatterns  # type: ignore
+    original_resolver = bedrock.redirects.middleware.get_resolver
+
+    # Patch the resolver to use the reloaded patterns
+    def patched_get_resolver(patterns=None):
+        if patterns is None:
+            patterns = module.redirectpatterns  # type: ignore
+        return original_resolver(patterns)
+
+    bedrock.redirects.middleware.get_resolver = patched_get_resolver
+
+    return original_patterns, original_resolver
+
+
+def _restore_redirects_middleware(module_path: str, original_patterns: Any, original_resolver: Any) -> None:
+    """
+    Restore the original redirects middleware state.
+
+    Args:
+        module_path: The module path that was reloaded
+        original_patterns: The original redirectpatterns to restore
+        original_resolver: The original get_resolver function to restore
+    """
+    import bedrock.redirects.middleware
+
+    # Restore original patterns
+    module = importlib.import_module(module_path)
+    module.redirectpatterns = original_patterns  # type: ignore
+
+    # Restore original resolver
+    bedrock.redirects.middleware.get_resolver = original_resolver