diff --git a/src/poetry/core/packages/directory_dependency.py b/src/poetry/core/packages/directory_dependency.py index db2c43927..3f6acd1f8 100644 --- a/src/poetry/core/packages/directory_dependency.py +++ b/src/poetry/core/packages/directory_dependency.py @@ -1,7 +1,6 @@ from __future__ import annotations import functools -import logging from typing import TYPE_CHECKING from typing import Iterable @@ -14,8 +13,6 @@ if TYPE_CHECKING: from pathlib import Path -logger = logging.getLogger(__name__) - class DirectoryDependency(PathDependency): def __init__( @@ -46,28 +43,22 @@ def __init__( def develop(self) -> bool: return self._develop - def validate(self, *, raise_error: bool) -> bool: - if not super().validate(raise_error=raise_error): - return False + def _validate(self) -> str: + message = super()._validate() + if message: + return message - message = "" if self._full_path.is_file(): - message = ( + return ( f"{self._full_path} for {self.pretty_name} is a file," " expected a directory" ) - elif not is_python_project(self._full_path): - message = ( + if not is_python_project(self._full_path): + return ( f"Directory {self._full_path} for {self.pretty_name} does not seem" " to be a Python package" ) - - if message: - if raise_error: - raise ValueError(message) - logger.warning(message) - return False - return True + return "" def _supports_poetry(self) -> bool: return PyProjectTOML(self._full_path / "pyproject.toml").is_poetry_project() diff --git a/src/poetry/core/packages/file_dependency.py b/src/poetry/core/packages/file_dependency.py index 7ad60de78..02b6b1974 100644 --- a/src/poetry/core/packages/file_dependency.py +++ b/src/poetry/core/packages/file_dependency.py @@ -2,7 +2,6 @@ import hashlib import io -import logging import warnings from typing import TYPE_CHECKING @@ -14,8 +13,6 @@ if TYPE_CHECKING: from pathlib import Path -logger = logging.getLogger(__name__) - class FileDependency(PathDependency): def __init__( @@ -37,20 +34,17 @@ def __init__( extras=extras, ) - def validate(self, *, raise_error: bool) -> bool: - if not super().validate(raise_error=raise_error): - return False + def _validate(self) -> str: + message = super()._validate() + if message: + return message if self._full_path.is_dir(): - message = ( + return ( f"{self._full_path} for {self.pretty_name} is a directory," " expected a file" ) - if raise_error: - raise ValueError(message) - logger.warning(message) - return False - return True + return "" def hash(self, hash_name: str = "sha256") -> str: warnings.warn( diff --git a/src/poetry/core/packages/path_dependency.py b/src/poetry/core/packages/path_dependency.py index 18cdf82c2..48076e8fd 100644 --- a/src/poetry/core/packages/path_dependency.py +++ b/src/poetry/core/packages/path_dependency.py @@ -45,6 +45,8 @@ def __init__( source_url=self._full_path.as_posix(), extras=extras, ) + # cache validation result to avoid unnecessary file system access + self._validation_error = self._validate() self.validate(raise_error=False) @property @@ -66,13 +68,12 @@ def is_directory(self) -> bool: return self._source_type == "directory" def validate(self, *, raise_error: bool) -> bool: - if not self._full_path.exists(): - message = f"Path {self._full_path} for {self.pretty_name} does not exist" - if raise_error: - raise ValueError(message) - logger.warning(message) - return False - return True + if not self._validation_error: + return True + if raise_error: + raise ValueError(self._validation_error) + logger.warning(self._validation_error) + return False @property def base_pep_508_name(self) -> str: @@ -86,3 +87,8 @@ def base_pep_508_name(self) -> str: requirement += f" @ {path}" return requirement + + def _validate(self) -> str: + if not self._full_path.exists(): + return f"Path {self._full_path} for {self.pretty_name} does not exist" + return "" diff --git a/tests/packages/test_directory_dependency.py b/tests/packages/test_directory_dependency.py index 23576317f..776593ab1 100644 --- a/tests/packages/test_directory_dependency.py +++ b/tests/packages/test_directory_dependency.py @@ -12,12 +12,18 @@ if TYPE_CHECKING: from _pytest.logging import LogCaptureFixture + from pytest_mock import MockerFixture + DIST_PATH = Path(__file__).parent.parent / "fixtures" / "distributions" SAMPLE_PROJECT = Path(__file__).parent.parent / "fixtures" / "sample_project" -def test_directory_dependency_does_not_exist(caplog: LogCaptureFixture) -> None: +def test_directory_dependency_does_not_exist( + caplog: LogCaptureFixture, mocker: MockerFixture +) -> None: + mock_exists = mocker.patch.object(Path, "exists") + mock_exists.return_value = False dep = DirectoryDependency("demo", DIST_PATH / "invalid") assert len(caplog.records) == 1 record = caplog.records[0] @@ -27,8 +33,14 @@ def test_directory_dependency_does_not_exist(caplog: LogCaptureFixture) -> None: with pytest.raises(ValueError, match="does not exist"): dep.validate(raise_error=True) + mock_exists.assert_called_once() + -def test_directory_dependency_is_file(caplog: LogCaptureFixture) -> None: +def test_directory_dependency_is_file( + caplog: LogCaptureFixture, mocker: MockerFixture +) -> None: + mock_is_file = mocker.patch.object(Path, "is_file") + mock_is_file.return_value = True dep = DirectoryDependency("demo", DIST_PATH / "demo-0.1.0.tar.gz") assert len(caplog.records) == 1 record = caplog.records[0] @@ -38,6 +50,8 @@ def test_directory_dependency_is_file(caplog: LogCaptureFixture) -> None: with pytest.raises(ValueError, match="is a file"): dep.validate(raise_error=True) + mock_is_file.assert_called_once() + def test_directory_dependency_is_not_a_python_project( caplog: LogCaptureFixture, diff --git a/tests/packages/test_file_dependency.py b/tests/packages/test_file_dependency.py index d5afd12df..124792258 100644 --- a/tests/packages/test_file_dependency.py +++ b/tests/packages/test_file_dependency.py @@ -21,7 +21,11 @@ TEST_FILE = "demo-0.1.0.tar.gz" -def test_file_dependency_does_not_exist(caplog: LogCaptureFixture) -> None: +def test_file_dependency_does_not_exist( + caplog: LogCaptureFixture, mocker: MockerFixture +) -> None: + mock_exists = mocker.patch.object(Path, "exists") + mock_exists.return_value = False dep = FileDependency("demo", DIST_PATH / "demo-0.2.0.tar.gz") assert len(caplog.records) == 1 record = caplog.records[0] @@ -31,8 +35,14 @@ def test_file_dependency_does_not_exist(caplog: LogCaptureFixture) -> None: with pytest.raises(ValueError, match="does not exist"): dep.validate(raise_error=True) + mock_exists.assert_called_once() + -def test_file_dependency_is_directory(caplog: LogCaptureFixture) -> None: +def test_file_dependency_is_directory( + caplog: LogCaptureFixture, mocker: MockerFixture +) -> None: + mock_is_directory = mocker.patch.object(Path, "is_dir") + mock_is_directory.return_value = True dep = FileDependency("demo", DIST_PATH) assert len(caplog.records) == 1 record = caplog.records[0] @@ -42,6 +52,8 @@ def test_file_dependency_is_directory(caplog: LogCaptureFixture) -> None: with pytest.raises(ValueError, match="is a directory"): dep.validate(raise_error=True) + mock_is_directory.assert_called_once() + def test_default_hash() -> None: with pytest.warns(DeprecationWarning):