From 2cebb8e872c9db7659dbe3b2e9bee04cf7a4e0d3 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Wed, 13 Mar 2024 21:58:22 +0100 Subject: [PATCH 1/3] test: enforce tmp working directory This change ensures that working director when executing a test is set to `tmp_path` fixture value. Additionally, we introduce the `set_project_context` fixture that allows for test cases to switch cwd to the required fixture project (copied to `tmp_path` by default) to ensure correct source files are used. Previously, some tests accidentally picked up incorrect pyproject.toml files as a result. --- tests/conftest.py | 36 +++++++++++++++++++ tests/console/test_application.py | 58 ++++++++++++++++++------------- tests/helpers.py | 29 ++++++++++++++++ tests/types.py | 7 ++++ 4 files changed, 105 insertions(+), 25 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5955c81dacd..fbbd76aa0c5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,6 @@ from __future__ import annotations +import contextlib import logging import os import re @@ -41,6 +42,8 @@ from tests.helpers import isolated_environment from tests.helpers import mock_clone from tests.helpers import mock_download +from tests.helpers import switch_working_directory +from tests.helpers import with_working_directory if TYPE_CHECKING: @@ -49,12 +52,14 @@ from _pytest.config import Config as PyTestConfig from _pytest.config.argparsing import Parser + from _pytest.tmpdir import TempPathFactory from pytest_mock import MockerFixture from poetry.poetry import Poetry from tests.types import FixtureCopier from tests.types import FixtureDirGetter from tests.types import ProjectFactory + from tests.types import SetProjectContext def pytest_addoption(parser: Parser) -> None: @@ -525,3 +530,34 @@ def venv_flags_default() -> dict[str, bool]: def httpretty_windows_mock_urllib3_wait_for_socket(mocker: MockerFixture) -> None: # this is a workaround for https://github.com/gabrielfalcao/HTTPretty/issues/442 mocker.patch("urllib3.util.wait.select_wait_for_socket", returns=True) + + +@pytest.fixture(autouse=True) +def tmp_working_directory(tmp_path: Path) -> Iterator[Path]: + with switch_working_directory(tmp_path): + yield tmp_path + + +@pytest.fixture(autouse=True, scope="session") +def tmp_session_working_directory(tmp_path_factory: TempPathFactory) -> Iterator[Path]: + tmp_path = tmp_path_factory.mktemp("session-working-directory") + with switch_working_directory(tmp_path): + yield tmp_path + + +@pytest.fixture +def set_project_context( + tmp_working_directory: Path, tmp_path: Path, fixture_dir: FixtureDirGetter +) -> SetProjectContext: + @contextlib.contextmanager + def project_context(project: str | Path, in_place: bool = False) -> Iterator[Path]: + if isinstance(project, str): + project = fixture_dir(project) + + with with_working_directory( + source=project, + target=tmp_path.joinpath(project.name) if not in_place else None, + ) as path: + yield path + + return project_context diff --git a/tests/console/test_application.py b/tests/console/test_application.py index 931944e227f..b7d057b687c 100644 --- a/tests/console/test_application.py +++ b/tests/console/test_application.py @@ -20,6 +20,8 @@ if TYPE_CHECKING: from pytest_mock import MockerFixture + from tests.types import SetProjectContext + class FooCommand(Command): name = "foo" @@ -85,44 +87,50 @@ def test_application_execute_plugin_command_with_plugins_disabled( @pytest.mark.parametrize("disable_cache", [True, False]) -def test_application_verify_source_cache_flag(disable_cache: bool) -> None: - app = Application() +def test_application_verify_source_cache_flag( + disable_cache: bool, set_project_context: SetProjectContext +) -> None: + with set_project_context("sample_project"): + app = Application() - tester = ApplicationTester(app) - command = "debug info" + tester = ApplicationTester(app) + command = "debug info" - if disable_cache: - command = f"{command} --no-cache" + if disable_cache: + command = f"{command} --no-cache" - assert not app._poetry + assert not app._poetry - tester.execute(command) + tester.execute(command) - assert app.poetry.pool.repositories + assert app.poetry.pool.repositories - for repo in app.poetry.pool.repositories: - assert isinstance(repo, CachedRepository) - assert repo._disable_cache == disable_cache + for repo in app.poetry.pool.repositories: + assert isinstance(repo, CachedRepository) + assert repo._disable_cache == disable_cache @pytest.mark.parametrize("disable_cache", [True, False]) def test_application_verify_cache_flag_at_install( - mocker: MockerFixture, disable_cache: bool + mocker: MockerFixture, + disable_cache: bool, + set_project_context: SetProjectContext, ) -> None: - app = Application() + with set_project_context("sample_project"): + app = Application() - tester = ApplicationTester(app) - command = "install --dry-run" + tester = ApplicationTester(app) + command = "install --dry-run" - if disable_cache: - command = f"{command} --no-cache" + if disable_cache: + command = f"{command} --no-cache" - spy = mocker.spy(Authenticator, "__init__") + spy = mocker.spy(Authenticator, "__init__") - tester.execute(command) + tester.execute(command) - assert spy.call_count == 2 - for call in spy.mock_calls: - (name, args, kwargs) = call - assert "disable_cache" in kwargs - assert disable_cache is kwargs["disable_cache"] + assert spy.call_count == 2 + for call in spy.mock_calls: + (name, args, kwargs) = call + assert "disable_cache" in kwargs + assert disable_cache is kwargs["disable_cache"] diff --git a/tests/helpers.py b/tests/helpers.py index cb15db558f6..67592a549ca 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -338,3 +338,32 @@ def redirect_request_callback( status=status_code, body=redirect_request_callback, ) + + +@contextlib.contextmanager +def switch_working_directory(path: Path, remove: bool = False) -> Iterator[Path]: + original_cwd = Path.cwd() + os.chdir(path) + + with contextlib.suppress(Exception) as exception: + yield path + + os.chdir(original_cwd) + + if remove: + shutil.rmtree(path, ignore_errors=True) + + if exception is not None: + raise exception + + +@contextlib.contextmanager +def with_working_directory(source: Path, target: Path | None = None) -> Iterator[Path]: + use_copy = target is not None + + if use_copy: + assert target is not None + shutil.copytree(source, target) + + with switch_working_directory(target or source, remove=use_copy) as path: + yield path diff --git a/tests/types.py b/tests/types.py index 4fc279abe06..e58bad84dc8 100644 --- a/tests/types.py +++ b/tests/types.py @@ -8,6 +8,7 @@ if TYPE_CHECKING: from collections.abc import Callable from pathlib import Path + from typing import ContextManager from typing import Dict from typing import Tuple @@ -79,3 +80,9 @@ def __call__(self, content: str, base_url: str | None = None) -> str: ... class RequestsSessionGet(Protocol): def __call__(self, url: str, **__: Any) -> requests.Response: ... + + +class SetProjectContext(Protocol): + def __call__( + self, project: str | Path, in_place: bool = False + ) -> ContextManager[Path]: ... From 288a4f6a08bbc7a11493d33f426c4318cf908cc6 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Wed, 13 Mar 2024 23:41:50 +0100 Subject: [PATCH 2/3] tests: convert constants to fixtures --- .../repositories/test_installed_repository.py | 161 +++++++++++------- 1 file changed, 102 insertions(+), 59 deletions(-) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index 2dbf01410b3..d5e1dee8a4b 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -5,6 +5,7 @@ from pathlib import Path from typing import TYPE_CHECKING +from typing import Iterator from typing import NamedTuple import pytest @@ -12,8 +13,9 @@ from poetry.repositories.installed_repository import InstalledRepository from poetry.utils._compat import metadata from poetry.utils.env import EnvManager -from poetry.utils.env import MockEnv as BaseMockEnv +from poetry.utils.env import MockEnv from poetry.utils.env import VirtualEnv +from tests.helpers import with_working_directory if TYPE_CHECKING: @@ -25,57 +27,84 @@ from tests.types import FixtureDirGetter from tests.types import ProjectFactory -FIXTURES_DIR = Path(__file__).parent / "fixtures" -ENV_DIR = (FIXTURES_DIR / "installed").resolve() -SITE_PURELIB = ENV_DIR / "lib" / "python3.7" / "site-packages" -SITE_PLATLIB = ENV_DIR / "lib64" / "python3.7" / "site-packages" -SRC = ENV_DIR / "src" -INSTALLED_RESULTS = [ - metadata.PathDistribution(SITE_PURELIB / "cleo-0.7.6.dist-info"), - metadata.PathDistribution(SRC / "pendulum" / "pendulum.egg-info"), - metadata.PathDistribution( - zipfile.Path( # type: ignore[arg-type] - str(SITE_PURELIB / "foo-0.1.0-py3.8.egg"), - "EGG-INFO", - ) - ), - metadata.PathDistribution(SITE_PURELIB / "standard-1.2.3.dist-info"), - metadata.PathDistribution(SITE_PURELIB / "editable-2.3.4.dist-info"), - metadata.PathDistribution(SITE_PURELIB / "editable-with-import-2.3.4.dist-info"), - metadata.PathDistribution(SITE_PLATLIB / "lib64-2.3.4.dist-info"), - metadata.PathDistribution(SITE_PLATLIB / "bender-2.0.5.dist-info"), - metadata.PathDistribution(SITE_PURELIB / "git_pep_610-1.2.3.dist-info"), - metadata.PathDistribution( - SITE_PURELIB / "git_pep_610_no_requested_version-1.2.3.dist-info" - ), - metadata.PathDistribution( - SITE_PURELIB / "git_pep_610_subdirectory-1.2.3.dist-info" - ), - metadata.PathDistribution(SITE_PURELIB / "url_pep_610-1.2.3.dist-info"), - metadata.PathDistribution(SITE_PURELIB / "file_pep_610-1.2.3.dist-info"), - metadata.PathDistribution(SITE_PURELIB / "directory_pep_610-1.2.3.dist-info"), - metadata.PathDistribution( - SITE_PURELIB / "editable_directory_pep_610-1.2.3.dist-info" - ), -] - - -class MockEnv(BaseMockEnv): - @property - def paths(self) -> dict[str, str]: - return { - "purelib": SITE_PURELIB.as_posix(), - "platlib": SITE_PLATLIB.as_posix(), - } - - @property - def sys_path(self) -> list[str]: - return [str(path) for path in [ENV_DIR, SITE_PLATLIB, SITE_PURELIB]] + +@pytest.fixture(scope="session") +def env_dir(tmp_session_working_directory: Path) -> Iterator[Path]: + source = Path(__file__).parent / "fixtures" / "installed" + target = tmp_session_working_directory / source.name + + with with_working_directory(source=source, target=target) as path: + yield path + + +@pytest.fixture(scope="session") +def site_purelib(env_dir: Path) -> Path: + return env_dir / "lib" / "python3.7" / "site-packages" + + +@pytest.fixture(scope="session") +def site_platlib(env_dir: Path) -> Path: + return env_dir / "lib64" / "python3.7" / "site-packages" + + +@pytest.fixture(scope="session") +def src_dir(env_dir: Path) -> Path: + return env_dir / "src" + + +@pytest.fixture(scope="session") +def installed_results( + site_purelib: Path, site_platlib: Path, src_dir: Path +) -> list[metadata.PathDistribution]: + return [ + metadata.PathDistribution(site_purelib / "cleo-0.7.6.dist-info"), + metadata.PathDistribution(src_dir / "pendulum" / "pendulum.egg-info"), + metadata.PathDistribution( + zipfile.Path( # type: ignore[arg-type] + str(site_purelib / "foo-0.1.0-py3.8.egg"), + "EGG-INFO", + ) + ), + metadata.PathDistribution(site_purelib / "standard-1.2.3.dist-info"), + metadata.PathDistribution(site_purelib / "editable-2.3.4.dist-info"), + metadata.PathDistribution( + site_purelib / "editable-with-import-2.3.4.dist-info" + ), + metadata.PathDistribution(site_platlib / "lib64-2.3.4.dist-info"), + metadata.PathDistribution(site_platlib / "bender-2.0.5.dist-info"), + metadata.PathDistribution(site_purelib / "git_pep_610-1.2.3.dist-info"), + metadata.PathDistribution( + site_purelib / "git_pep_610_no_requested_version-1.2.3.dist-info" + ), + metadata.PathDistribution( + site_purelib / "git_pep_610_subdirectory-1.2.3.dist-info" + ), + metadata.PathDistribution(site_purelib / "url_pep_610-1.2.3.dist-info"), + metadata.PathDistribution(site_purelib / "file_pep_610-1.2.3.dist-info"), + metadata.PathDistribution(site_purelib / "directory_pep_610-1.2.3.dist-info"), + metadata.PathDistribution( + site_purelib / "editable_directory_pep_610-1.2.3.dist-info" + ), + ] @pytest.fixture -def env() -> MockEnv: - return MockEnv(path=ENV_DIR) +def env( + env_dir: Path, site_purelib: Path, site_platlib: Path, src_dir: Path +) -> MockEnv: + class _MockEnv(MockEnv): + @property + def paths(self) -> dict[str, str]: + return { + "purelib": site_purelib.as_posix(), + "platlib": site_platlib.as_posix(), + } + + @property + def sys_path(self) -> list[str]: + return [str(path) for path in [env_dir, site_platlib, site_purelib]] + + return _MockEnv(path=env_dir) @pytest.fixture(autouse=True) @@ -94,10 +123,14 @@ class GitRepoLocalInfo(NamedTuple): @pytest.fixture -def repository(mocker: MockerFixture, env: MockEnv) -> InstalledRepository: +def repository( + mocker: MockerFixture, + env: MockEnv, + installed_results: list[metadata.PathDistribution], +) -> InstalledRepository: mocker.patch( "poetry.utils._compat.metadata.Distribution.discover", - return_value=INSTALLED_RESULTS, + return_value=installed_results, ) return InstalledRepository.load(env) @@ -112,26 +145,36 @@ def get_package_from_repository( @pytest.fixture -def poetry(project_factory: ProjectFactory, fixture_dir: FixtureDirGetter) -> Poetry: +def poetry( + project_factory: ProjectFactory, + fixture_dir: FixtureDirGetter, + installed_results: list[metadata.PathDistribution], +) -> Poetry: return project_factory("simple", source=fixture_dir("simple_project")) -def test_load_successful(repository: InstalledRepository) -> None: - assert len(repository.packages) == len(INSTALLED_RESULTS) +def test_load_successful( + repository: InstalledRepository, installed_results: list[metadata.PathDistribution] +) -> None: + assert len(repository.packages) == len(installed_results) def test_load_successful_with_invalid_distribution( - caplog: LogCaptureFixture, mocker: MockerFixture, env: MockEnv, tmp_path: Path + caplog: LogCaptureFixture, + mocker: MockerFixture, + env: MockEnv, + tmp_path: Path, + installed_results: list[metadata.PathDistribution], ) -> None: invalid_dist_info = tmp_path / "site-packages" / "invalid-0.1.0.dist-info" invalid_dist_info.mkdir(parents=True) mocker.patch( "poetry.utils._compat.metadata.Distribution.discover", - return_value=[*INSTALLED_RESULTS, metadata.PathDistribution(invalid_dist_info)], + return_value=[*installed_results, metadata.PathDistribution(invalid_dist_info)], ) repository_with_invalid_distribution = InstalledRepository.load(env) - assert len(repository_with_invalid_distribution.packages) == len(INSTALLED_RESULTS) + assert len(repository_with_invalid_distribution.packages) == len(installed_results) assert len(caplog.messages) == 1 message = caplog.messages[0] @@ -316,7 +359,7 @@ def test_load_pep_610_compliant_editable_directory_packages( def test_system_site_packages_source_type( - tmp_path: Path, mocker: MockerFixture, poetry: Poetry + tmp_path: Path, mocker: MockerFixture, poetry: Poetry, site_purelib: Path ) -> None: """ The source type of system site packages @@ -325,7 +368,7 @@ def test_system_site_packages_source_type( venv_path = tmp_path / "venv" site_path = tmp_path / "site" for dist_info in {"cleo-0.7.6.dist-info", "directory_pep_610-1.2.3.dist-info"}: - shutil.copytree(SITE_PURELIB / dist_info, site_path / dist_info) + shutil.copytree(site_purelib / dist_info, site_path / dist_info) mocker.patch("poetry.utils.env.virtual_env.VirtualEnv.sys_path", [str(site_path)]) mocker.patch("site.getsitepackages", return_value=[str(site_path)]) From fc1ca462d9202e7a8ceac15324173417fad6e46f Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Thu, 14 Mar 2024 22:05:41 +0100 Subject: [PATCH 3/3] test: fix incorrect windows editable pth file --- .../repositories/test_installed_repository.py | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index d5e1dee8a4b..24a2f12f013 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import shutil import zipfile @@ -153,6 +154,23 @@ def poetry( return project_factory("simple", source=fixture_dir("simple_project")) +@pytest.fixture(scope="session") +def editable_source_directory_path() -> str: + return Path("/path/to/editable").resolve(strict=False).as_posix() + + +@pytest.fixture(scope="session", autouse=(os.name == "nt")) +def fix_editable_path_for_windows( + site_purelib: Path, editable_source_directory_path: str +) -> None: + # we handle this as a special case since in certain scenarios (eg: on Windows GHA runners) + # the temp directory is on a different drive causing path resolutions without drive letters + # to give inconsistent results at different phases of the test suite execution; additionally + # this represents a more realistic scenario + editable_pth_file = site_purelib / "editable.pth" + editable_pth_file.write_text(editable_source_directory_path) + + def test_load_successful( repository: InstalledRepository, installed_results: list[metadata.PathDistribution] ) -> None: @@ -231,17 +249,16 @@ def test_load_platlib_package(repository: InstalledRepository) -> None: assert lib64.version.text == "2.3.4" -def test_load_editable_package(repository: InstalledRepository) -> None: +def test_load_editable_package( + repository: InstalledRepository, editable_source_directory_path: str +) -> None: # test editable package with text .pth file editable = get_package_from_repository("editable", repository) assert editable is not None assert editable.name == "editable" assert editable.version.text == "2.3.4" assert editable.source_type == "directory" - assert ( - editable.source_url - == Path("/path/to/editable").resolve(strict=False).as_posix() - ) + assert editable.source_url == editable_source_directory_path def test_load_editable_with_import_package(repository: InstalledRepository) -> None: