fix: set paths for component media as posix paths even on windows (#799)

* fix: set paths for component media as posix paths even on windows

* refactor: try running django server in windows

* refactor: handle Windows paths in component media

* refactor: temp test only windows

* refactor: try to fix tests

* refactor: more test fixes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* refactor: remove extraneous import

* refactor: more fixes

* refactor: fix path conversion for windows

* refactor: revert windows env in CI

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Juro Oravec 2024-12-02 19:03:41 +01:00 committed by GitHub
parent 230ceee537
commit 025562ac0c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 110 additions and 82 deletions

View file

@ -226,7 +226,8 @@ def _normalize_media_filepath(filepath: Any) -> Union[str, SafeData]:
return filepath return filepath
if isinstance(filepath, (Path, os.PathLike)) or hasattr(filepath, "__fspath__"): if isinstance(filepath, (Path, os.PathLike)) or hasattr(filepath, "__fspath__"):
filepath = filepath.__fspath__() # In case of Windows OS, convert to forward slashes
filepath = Path(filepath.__fspath__()).as_posix()
if isinstance(filepath, bytes): if isinstance(filepath, bytes):
filepath = filepath.decode("utf-8") filepath = filepath.decode("utf-8")
@ -293,19 +294,21 @@ def _resolve_component_relative_files(attrs: MutableMapping) -> None:
# If not, don't modify anything. # If not, don't modify anything.
def resolve_file(filepath: Union[str, SafeData]) -> Union[str, SafeData]: def resolve_file(filepath: Union[str, SafeData]) -> Union[str, SafeData]:
if isinstance(filepath, str): if isinstance(filepath, str):
maybe_resolved_filepath = os.path.join(comp_dir_abs, filepath) filepath_abs = os.path.join(comp_dir_abs, filepath)
component_import_filepath = os.path.join(comp_dir_rel, filepath) # NOTE: The paths to resources need to use POSIX (forward slashes) for Django to wor
# See https://github.com/EmilStenstrom/django-components/issues/796
filepath_rel_to_comp_dir = Path(os.path.join(comp_dir_rel, filepath)).as_posix()
if os.path.isfile(maybe_resolved_filepath): if os.path.isfile(filepath_abs):
# NOTE: It's important to use `repr`, so we don't trigger __str__ on SafeStrings # NOTE: It's important to use `repr`, so we don't trigger __str__ on SafeStrings
logger.debug( logger.debug(
f"Interpreting template '{repr(filepath)}' of component '{module_name}'" f"Interpreting template '{repr(filepath)}' of component '{module_name}'"
" relatively to component file" " relatively to component file"
) )
return component_import_filepath return filepath_rel_to_comp_dir
return filepath
# If resolved absolute path does NOT exist or filepath is NOT a string, then return as is
logger.debug( logger.debug(
f"Interpreting template '{repr(filepath)}' of component '{module_name}'" f"Interpreting template '{repr(filepath)}' of component '{module_name}'"
" relatively to components directory" " relatively to components directory"

View file

@ -1,6 +1,6 @@
import glob import glob
import os import os
from pathlib import Path from pathlib import Path, PurePosixPath, PureWindowsPath
from typing import List, NamedTuple, Optional, Set, Union from typing import List, NamedTuple, Optional, Set, Union
from django.apps import apps from django.apps import apps
@ -211,13 +211,11 @@ def _filepath_to_python_module(
- Then the path relative to project root is `app/components/mycomp.py` - Then the path relative to project root is `app/components/mycomp.py`
- Which we then turn into python import path `app.components.mycomp` - Which we then turn into python import path `app.components.mycomp`
""" """
rel_path = os.path.relpath(file_path, start=root_fs_path) path_cls = PureWindowsPath if os.name == "nt" else PurePosixPath
rel_path_without_suffix = str(Path(rel_path).with_suffix(""))
# NOTE: `Path` normalizes paths to use `/` as separator, while `os.path` rel_path = path_cls(file_path).relative_to(path_cls(root_fs_path))
# uses `os.path.sep`. rel_path_parts = rel_path.with_suffix("").parts
sep = os.path.sep if os.path.sep in rel_path_without_suffix else "/" module_name = ".".join(rel_path_parts)
module_name = rel_path_without_suffix.replace(sep, ".")
# Combine with the base module path # Combine with the base module path
full_module_name = f"{root_module_path}.{module_name}" if root_module_path else module_name full_module_name = f"{root_module_path}.{module_name}" if root_module_path else module_name

View file

@ -1,5 +1,6 @@
import functools import functools
import subprocess import subprocess
import sys
import time import time
from pathlib import Path from pathlib import Path
@ -39,12 +40,13 @@ def run_django_dev_server():
"""Fixture to run Django development server in the background.""" """Fixture to run Django development server in the background."""
# Get the path where testserver is defined, so the command doesn't depend # Get the path where testserver is defined, so the command doesn't depend
# on user's current working directory. # on user's current working directory.
testserver_dir = (Path(__file__).parent / "testserver").absolute() testserver_dir = (Path(__file__).parent / "testserver").resolve()
# Start the Django dev server in the background # Start the Django dev server in the background
print("Starting Django dev server...") print("Starting Django dev server...")
proc = subprocess.Popen( proc = subprocess.Popen(
["python", "manage.py", "runserver", f"127.0.0.1:{TEST_SERVER_PORT}", "--noreload"], # NOTE: Use `sys.executable` so this works both for Unix and Windows OS
[sys.executable, "manage.py", "runserver", f"127.0.0.1:{TEST_SERVER_PORT}", "--noreload"],
cwd=testserver_dir, cwd=testserver_dir,
) )

View file

@ -58,6 +58,9 @@ def do_collect():
post_process=True, post_process=True,
) )
collected = cmd.collect() collected = cmd.collect()
# Convert collected paths from string to Path, so we can run tests on both Unix and Windows
collected = {key: [Path(item) for item in items] for key, items in collected.items()}
return collected return collected
@ -86,10 +89,10 @@ class StaticFilesFinderTests(SimpleTestCase):
collected = do_collect() collected = do_collect()
# Check that the component files are NOT loaded when our finder is NOT added # Check that the component files are NOT loaded when our finder is NOT added
self.assertNotIn("staticfiles/staticfiles.css", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.css"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.js", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.js"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.html", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.html"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.py", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.py"), collected["modified"])
self.assertListEqual(collected["unmodified"], []) self.assertListEqual(collected["unmodified"], [])
self.assertListEqual(collected["post_processed"], []) self.assertListEqual(collected["post_processed"], [])
@ -109,10 +112,10 @@ class StaticFilesFinderTests(SimpleTestCase):
collected = do_collect() collected = do_collect()
# Check that our staticfiles_finder finds the files and OMITS .py and .html files # Check that our staticfiles_finder finds the files and OMITS .py and .html files
self.assertIn("staticfiles/staticfiles.css", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.css"), collected["modified"])
self.assertIn("staticfiles/staticfiles.js", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.js"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.html", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.html"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.py", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.py"), collected["modified"])
self.assertListEqual(collected["unmodified"], []) self.assertListEqual(collected["unmodified"], [])
self.assertListEqual(collected["post_processed"], []) self.assertListEqual(collected["post_processed"], [])
@ -138,10 +141,10 @@ class StaticFilesFinderTests(SimpleTestCase):
collected = do_collect() collected = do_collect()
# Check that our staticfiles_finder finds the files and OMITS .py and .html files # Check that our staticfiles_finder finds the files and OMITS .py and .html files
self.assertNotIn("staticfiles/staticfiles.css", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.css"), collected["modified"])
self.assertIn("staticfiles/staticfiles.js", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.js"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.html", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.html"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.py", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.py"), collected["modified"])
self.assertListEqual(collected["unmodified"], []) self.assertListEqual(collected["unmodified"], [])
self.assertListEqual(collected["post_processed"], []) self.assertListEqual(collected["post_processed"], [])
@ -169,10 +172,10 @@ class StaticFilesFinderTests(SimpleTestCase):
collected = do_collect() collected = do_collect()
# Check that our staticfiles_finder finds the files and OMITS .py and .html files # Check that our staticfiles_finder finds the files and OMITS .py and .html files
self.assertIn("staticfiles/staticfiles.css", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.css"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.js", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.js"), collected["modified"])
self.assertIn("staticfiles/staticfiles.html", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.html"), collected["modified"])
self.assertIn("staticfiles/staticfiles.py", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.py"), collected["modified"])
self.assertListEqual(collected["unmodified"], []) self.assertListEqual(collected["unmodified"], [])
self.assertListEqual(collected["post_processed"], []) self.assertListEqual(collected["post_processed"], [])
@ -201,10 +204,10 @@ class StaticFilesFinderTests(SimpleTestCase):
collected = do_collect() collected = do_collect()
# Check that our staticfiles_finder finds the files and OMITS .py and .html files # Check that our staticfiles_finder finds the files and OMITS .py and .html files
self.assertIn("staticfiles/staticfiles.css", collected["modified"]) self.assertIn(Path("staticfiles/staticfiles.css"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.js", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.js"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.html", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.html"), collected["modified"])
self.assertNotIn("staticfiles/staticfiles.py", collected["modified"]) self.assertNotIn(Path("staticfiles/staticfiles.py"), collected["modified"])
self.assertListEqual(collected["unmodified"], []) self.assertListEqual(collected["unmodified"], [])
self.assertListEqual(collected["post_processed"], []) self.assertListEqual(collected["post_processed"], [])

View file

@ -1,5 +1,4 @@
import os import os
import re
from pathlib import Path from pathlib import Path
from unittest.mock import MagicMock, patch from unittest.mock import MagicMock, patch
@ -35,8 +34,10 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 2) self.assertEqual(len(apps_dirs), 2)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/django_components\/components$"))
self.assertRegex(str(apps_dirs[1]), re.compile(r"\/tests\/test_app\/components$")) # NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-2:], ("django_components", "components"))
self.assertTupleEqual(apps_dirs[1].parts[-3:], ("tests", "test_app", "components"))
@override_settings( @override_settings(
BASE_DIR=Path(__file__).parent.resolve() / "test_structures" / "test_structure_1", # noqa BASE_DIR=Path(__file__).parent.resolve() / "test_structures" / "test_structure_1", # noqa
@ -49,8 +50,10 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 2) self.assertEqual(len(apps_dirs), 2)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/django_components\/components$"))
self.assertRegex(str(apps_dirs[1]), re.compile(r"\/tests\/test_app\/components$")) # NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-2:], ("django_components", "components"))
self.assertTupleEqual(apps_dirs[1].parts[-3:], ("tests", "test_app", "components"))
expected = [ expected = [
Path(__file__).parent.resolve() / "test_structures" / "test_structure_1" / "components", Path(__file__).parent.resolve() / "test_structures" / "test_structure_1" / "components",
@ -76,8 +79,10 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 2) self.assertEqual(len(apps_dirs), 2)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/django_components\/components$"))
self.assertRegex(str(apps_dirs[1]), re.compile(r"\/tests\/test_app\/components$")) # NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-2:], ("django_components", "components"))
self.assertTupleEqual(apps_dirs[1].parts[-3:], ("tests", "test_app", "components"))
self.assertEqual( self.assertEqual(
own_dirs, own_dirs,
@ -104,8 +109,10 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 2) self.assertEqual(len(apps_dirs), 2)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/django_components\/components$"))
self.assertRegex(str(apps_dirs[1]), re.compile(r"\/tests\/test_app\/components$")) # NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-2:], ("django_components", "components"))
self.assertTupleEqual(apps_dirs[1].parts[-3:], ("tests", "test_app", "components"))
@override_settings( @override_settings(
BASE_DIR=Path(__file__).parent.resolve(), BASE_DIR=Path(__file__).parent.resolve(),
@ -141,7 +148,9 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 1) self.assertEqual(len(apps_dirs), 1)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/tests\/test_app\/custom_comps_dir$"))
# NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-3:], ("tests", "test_app", "custom_comps_dir"))
self.assertEqual( self.assertEqual(
own_dirs, own_dirs,
@ -204,8 +213,10 @@ class ComponentDirsTest(BaseTestCase):
# Apps with a `components` dir # Apps with a `components` dir
self.assertEqual(len(apps_dirs), 2) self.assertEqual(len(apps_dirs), 2)
self.assertRegex(str(apps_dirs[0]), re.compile(r"\/django_components\/components$"))
self.assertRegex(str(apps_dirs[1]), re.compile(r"\/tests\/test_app_nested\/app\/components$")) # NOTE: Compare parts so that the test works on Windows too
self.assertTupleEqual(apps_dirs[0].parts[-2:], ("django_components", "components"))
self.assertTupleEqual(apps_dirs[1].parts[-4:], ("tests", "test_app_nested", "app", "components"))
self.assertEqual( self.assertEqual(
own_dirs, own_dirs,
@ -225,7 +236,7 @@ class ComponentFilesTest(BaseTestCase):
files = sorted(get_component_files(".py")) files = sorted(get_component_files(".py"))
dot_paths = [f.dot_path for f in files] dot_paths = [f.dot_path for f in files]
file_paths = [str(f.filepath) for f in files] file_paths = [f.filepath for f in files]
self.assertEqual( self.assertEqual(
dot_paths, dot_paths,
@ -243,20 +254,20 @@ class ComponentFilesTest(BaseTestCase):
], ],
) )
self.assertEqual( # NOTE: Compare parts so that the test works on Windows too
[ self.assertTupleEqual(file_paths[0].parts[-3:], ("tests", "components", "__init__.py"))
file_paths[0].endswith("tests/components/__init__.py"), self.assertTupleEqual(file_paths[1].parts[-4:], ("tests", "components", "multi_file", "multi_file.py"))
file_paths[1].endswith("tests/components/multi_file/multi_file.py"), self.assertTupleEqual(file_paths[2].parts[-4:], ("tests", "components", "relative_file", "relative_file.py"))
file_paths[2].endswith("tests/components/relative_file/relative_file.py"), self.assertTupleEqual(
file_paths[3].endswith("tests/components/relative_file_pathobj/relative_file_pathobj.py"), file_paths[3].parts[-4:], ("tests", "components", "relative_file_pathobj", "relative_file_pathobj.py")
file_paths[4].endswith("tests/components/single_file.py"), )
file_paths[5].endswith("tests/components/staticfiles/staticfiles.py"), self.assertTupleEqual(file_paths[4].parts[-3:], ("tests", "components", "single_file.py"))
file_paths[6].endswith("tests/components/urls.py"), self.assertTupleEqual(file_paths[5].parts[-4:], ("tests", "components", "staticfiles", "staticfiles.py"))
file_paths[7].endswith("django_components/components/__init__.py"), self.assertTupleEqual(file_paths[6].parts[-3:], ("tests", "components", "urls.py"))
file_paths[8].endswith("django_components/components/dynamic.py"), self.assertTupleEqual(file_paths[7].parts[-3:], ("django_components", "components", "__init__.py"))
file_paths[9].endswith("tests/test_app/components/app_lvl_comp/app_lvl_comp.py"), self.assertTupleEqual(file_paths[8].parts[-3:], ("django_components", "components", "dynamic.py"))
], self.assertTupleEqual(
[True for _ in range(len(file_paths))], file_paths[9].parts[-5:], ("tests", "test_app", "components", "app_lvl_comp", "app_lvl_comp.py")
) )
@override_settings( @override_settings(
@ -266,9 +277,7 @@ class ComponentFilesTest(BaseTestCase):
files = sorted(get_component_files(".js")) files = sorted(get_component_files(".js"))
dot_paths = [f.dot_path for f in files] dot_paths = [f.dot_path for f in files]
file_paths = [str(f.filepath) for f in files] file_paths = [f.filepath for f in files]
print(file_paths)
self.assertEqual( self.assertEqual(
dot_paths, dot_paths,
@ -280,14 +289,14 @@ class ComponentFilesTest(BaseTestCase):
], ],
) )
self.assertEqual( # NOTE: Compare parts so that the test works on Windows too
[ self.assertTupleEqual(file_paths[0].parts[-4:], ("tests", "components", "relative_file", "relative_file.js"))
file_paths[0].endswith("tests/components/relative_file/relative_file.js"), self.assertTupleEqual(
file_paths[1].endswith("tests/components/relative_file_pathobj/relative_file_pathobj.js"), file_paths[1].parts[-4:], ("tests", "components", "relative_file_pathobj", "relative_file_pathobj.js")
file_paths[2].endswith("tests/components/staticfiles/staticfiles.js"), )
file_paths[3].endswith("tests/test_app/components/app_lvl_comp/app_lvl_comp.js"), self.assertTupleEqual(file_paths[2].parts[-4:], ("tests", "components", "staticfiles", "staticfiles.js"))
], self.assertTupleEqual(
[True for _ in range(len(file_paths))], file_paths[3].parts[-5:], ("tests", "test_app", "components", "app_lvl_comp", "app_lvl_comp.js")
) )
@ -307,31 +316,44 @@ class TestFilepathToPythonModule(BaseTestCase):
"tests.components.relative_file.relative_file", "tests.components.relative_file.relative_file",
) )
def test_handles_nonlinux_paths(self): def test_handles_separators_based_on_os_name(self):
base_path = str(settings.BASE_DIR).replace("/", "//") base_path = str(settings.BASE_DIR)
with patch("os.path.sep", new="//"): with patch("os.name", new="posix"):
the_path = os.path.join(base_path, "tests.py") the_path = base_path + "/" + "tests.py"
self.assertEqual( self.assertEqual(
_filepath_to_python_module(the_path, base_path, None), _filepath_to_python_module(the_path, base_path, None),
"tests", "tests",
) )
the_path = os.path.join(base_path, "tests//components//relative_file//relative_file.py") the_path = base_path + "/" + "tests/components/relative_file/relative_file.py"
self.assertEqual( self.assertEqual(
_filepath_to_python_module(the_path, base_path, None), _filepath_to_python_module(the_path, base_path, None),
"tests.components.relative_file.relative_file", "tests.components.relative_file.relative_file",
) )
base_path = str(settings.BASE_DIR).replace("//", "\\") base_path = str(settings.BASE_DIR).replace("/", "\\")
with patch("os.path.sep", new="\\"): with patch("os.name", new="nt"):
the_path = os.path.join(base_path, "tests.py") the_path = base_path + "\\" + "tests.py"
self.assertEqual( self.assertEqual(
_filepath_to_python_module(the_path, base_path, None), _filepath_to_python_module(the_path, base_path, None),
"tests", "tests",
) )
the_path = os.path.join(base_path, "tests\\components\\relative_file\\relative_file.py") the_path = base_path + "\\" + "tests\\components\\relative_file\\relative_file.py"
self.assertEqual(
_filepath_to_python_module(the_path, base_path, None),
"tests.components.relative_file.relative_file",
)
# NOTE: Windows should handle also POSIX separator
the_path = base_path + "/" + "tests.py"
self.assertEqual(
_filepath_to_python_module(the_path, base_path, None),
"tests",
)
the_path = base_path + "/" + "tests/components/relative_file/relative_file.py"
self.assertEqual( self.assertEqual(
_filepath_to_python_module(the_path, base_path, None), _filepath_to_python_module(the_path, base_path, None),
"tests.components.relative_file.relative_file", "tests.components.relative_file.relative_file",