From 52963587516f61ffdc6f2015105458f98769e2bd Mon Sep 17 00:00:00 2001 From: Adam Richardson Date: Thu, 24 Nov 2022 11:04:13 +0000 Subject: [PATCH 1/5] Add ability for the builder factory to accept an output directory as an argument --- src/poetry/core/masonry/builder.py | 9 +++++-- .../masonry/builders/test_builder_factory.py | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/masonry/builders/test_builder_factory.py diff --git a/src/poetry/core/masonry/builder.py b/src/poetry/core/masonry/builder.py index 304288585..ccb66a36a 100644 --- a/src/poetry/core/masonry/builder.py +++ b/src/poetry/core/masonry/builder.py @@ -21,7 +21,12 @@ def __init__(self, poetry: Poetry) -> None: "wheel": WheelBuilder, } - def build(self, fmt: str, executable: str | Path | None = None) -> None: + def build( + self, + fmt: str, + output_directory: Path | None = None, + executable: str | Path | None = None + ) -> None: if fmt in self._formats: builders = [self._formats[fmt]] elif fmt == "all": @@ -30,4 +35,4 @@ def build(self, fmt: str, executable: str | Path | None = None) -> None: raise ValueError(f"Invalid format: {fmt}") for builder in builders: - builder(self._poetry, executable=executable).build() + builder(self._poetry, executable=executable).build(output_directory) diff --git a/tests/masonry/builders/test_builder_factory.py b/tests/masonry/builders/test_builder_factory.py new file mode 100644 index 000000000..0f728bd42 --- /dev/null +++ b/tests/masonry/builders/test_builder_factory.py @@ -0,0 +1,25 @@ +from pathlib import Path +import pytest +from poetry.core.masonry.builder import Builder +from poetry.core.factory import Factory + + +def project(name: str) -> Path: + return Path(__file__).parent / "fixtures" / name + + +def get_poetry(name: str): + return Factory().create_poetry(project(name)) + + +def test_builder_factory_raises_error_when_format_is_not_valid(): + with pytest.raises(ValueError, match=r"Invalid format.*"): + Builder(get_poetry("complete")).build("not_valid") + + +@pytest.mark.parametrize("format", ["sdist", "wheel", "all"]) +def test_builder_creates_new_directory(tmp_path: Path, format: str): + poetry = Factory().create_poetry(Path(__file__).parent / "fixtures" / "complete") + Builder(poetry).build(format, tmp_path) + assert all(archive.exists() for archive in tmp_path.glob( + f"{poetry.package.name.replace('-', '_')}-{poetry.package.version}*")) From f10148eded38d7f6d97f7c5dd77f4619f183ddee Mon Sep 17 00:00:00 2001 From: Adam Richardson Date: Thu, 24 Nov 2022 11:46:26 +0000 Subject: [PATCH 2/5] Use context managers to clean up directories and add test that default dist directory is created if no output is passed --- src/poetry/core/masonry/builder.py | 2 +- .../masonry/builders/test_builder_factory.py | 60 ++++++++++++++++--- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/poetry/core/masonry/builder.py b/src/poetry/core/masonry/builder.py index ccb66a36a..baade9943 100644 --- a/src/poetry/core/masonry/builder.py +++ b/src/poetry/core/masonry/builder.py @@ -25,7 +25,7 @@ def build( self, fmt: str, output_directory: Path | None = None, - executable: str | Path | None = None + executable: str | Path | None = None, ) -> None: if fmt in self._formats: builders = [self._formats[fmt]] diff --git a/tests/masonry/builders/test_builder_factory.py b/tests/masonry/builders/test_builder_factory.py index 0f728bd42..c5a7b9a50 100644 --- a/tests/masonry/builders/test_builder_factory.py +++ b/tests/masonry/builders/test_builder_factory.py @@ -1,25 +1,67 @@ +from __future__ import annotations + +import shutil + +from contextlib import contextmanager from pathlib import Path +from typing import TYPE_CHECKING +from typing import Iterator + import pytest -from poetry.core.masonry.builder import Builder + from poetry.core.factory import Factory +from poetry.core.masonry.builder import Builder + + +if TYPE_CHECKING: + from poetry.core.poetry import Poetry -def project(name: str) -> Path: +def get_project(name: str) -> Path: return Path(__file__).parent / "fixtures" / name -def get_poetry(name: str): - return Factory().create_poetry(project(name)) +@contextmanager +def get_project_context(name: str) -> Iterator[Path]: + project_directory = get_project(name) + try: + yield project_directory + finally: + shutil.rmtree(project_directory / "dist") -def test_builder_factory_raises_error_when_format_is_not_valid(): +def get_poetry(name: str) -> Poetry: + return Factory().create_poetry(get_project(name)) + + +def get_package_glob(poetry: Poetry) -> str: + return f"{poetry.package.name.replace('-', '_')}-{poetry.package.version}*" + + +def test_builder_factory_raises_error_when_format_is_not_valid() -> None: with pytest.raises(ValueError, match=r"Invalid format.*"): Builder(get_poetry("complete")).build("not_valid") @pytest.mark.parametrize("format", ["sdist", "wheel", "all"]) -def test_builder_creates_new_directory(tmp_path: Path, format: str): - poetry = Factory().create_poetry(Path(__file__).parent / "fixtures" / "complete") +def test_builder_creates_places_built_files_in_specified_directory( + tmp_path: Path, format: str +) -> None: + poetry = get_poetry("complete") Builder(poetry).build(format, tmp_path) - assert all(archive.exists() for archive in tmp_path.glob( - f"{poetry.package.name.replace('-', '_')}-{poetry.package.version}*")) + assert all(archive.exists() for archive in tmp_path.glob(get_package_glob(poetry))) + + +@pytest.mark.parametrize("format", ["sdist", "wheel", "all"]) +def test_builder_creates_packages_in_dist_directory_if_no_output_is_specified( + format: str, +) -> None: + with get_project_context("complete") as project: + poetry = Factory().create_poetry(project) + Builder(poetry).build(format, None) + package_directory = project / "dist" + assert package_directory.is_dir() + assert all( + archive.exists() + for archive in package_directory.glob(get_package_glob(poetry)) + ) From 8460aa655b7b6acd6817c721dbcae8c5eb52cbec Mon Sep 17 00:00:00 2001 From: Adam Richardson Date: Thu, 24 Nov 2022 17:55:34 +0000 Subject: [PATCH 3/5] Change order of arguments to maintain backwards compatability and make target_dir argument keyword only --- src/poetry/core/masonry/builder.py | 5 +++-- tests/masonry/builders/test_builder_factory.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/poetry/core/masonry/builder.py b/src/poetry/core/masonry/builder.py index baade9943..d31ce627d 100644 --- a/src/poetry/core/masonry/builder.py +++ b/src/poetry/core/masonry/builder.py @@ -24,8 +24,9 @@ def __init__(self, poetry: Poetry) -> None: def build( self, fmt: str, - output_directory: Path | None = None, executable: str | Path | None = None, + *, + target_dir: Path | None = None, ) -> None: if fmt in self._formats: builders = [self._formats[fmt]] @@ -35,4 +36,4 @@ def build( raise ValueError(f"Invalid format: {fmt}") for builder in builders: - builder(self._poetry, executable=executable).build(output_directory) + builder(self._poetry, executable=executable).build(target_dir) diff --git a/tests/masonry/builders/test_builder_factory.py b/tests/masonry/builders/test_builder_factory.py index c5a7b9a50..ad75fd4ea 100644 --- a/tests/masonry/builders/test_builder_factory.py +++ b/tests/masonry/builders/test_builder_factory.py @@ -48,7 +48,7 @@ def test_builder_creates_places_built_files_in_specified_directory( tmp_path: Path, format: str ) -> None: poetry = get_poetry("complete") - Builder(poetry).build(format, tmp_path) + Builder(poetry).build(format, target_dir=tmp_path) assert all(archive.exists() for archive in tmp_path.glob(get_package_glob(poetry))) @@ -58,7 +58,7 @@ def test_builder_creates_packages_in_dist_directory_if_no_output_is_specified( ) -> None: with get_project_context("complete") as project: poetry = Factory().create_poetry(project) - Builder(poetry).build(format, None) + Builder(poetry).build(format, target_dir=None) package_directory = project / "dist" assert package_directory.is_dir() assert all( From 35c054f90caa15c7de413d8b4b140f2fa4167200 Mon Sep 17 00:00:00 2001 From: Adam Richardson Date: Sat, 7 Jan 2023 14:42:07 +0000 Subject: [PATCH 4/5] Change test layout to be the same as src layout --- .../{builders/test_builder_factory.py => test_builder.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/masonry/{builders/test_builder_factory.py => test_builder.py} (96%) diff --git a/tests/masonry/builders/test_builder_factory.py b/tests/masonry/test_builder.py similarity index 96% rename from tests/masonry/builders/test_builder_factory.py rename to tests/masonry/test_builder.py index ad75fd4ea..14432f8ed 100644 --- a/tests/masonry/builders/test_builder_factory.py +++ b/tests/masonry/test_builder.py @@ -18,7 +18,7 @@ def get_project(name: str) -> Path: - return Path(__file__).parent / "fixtures" / name + return Path(__file__).parent / "builders" / "fixtures" / name @contextmanager From b8f197a413532f569c2646bbf4b26b98f8b71fe0 Mon Sep 17 00:00:00 2001 From: Adam Richardson Date: Sat, 7 Jan 2023 15:05:53 +0000 Subject: [PATCH 5/5] Amend builder factory tests to assert built packages are not empty --- tests/masonry/test_builder.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/masonry/test_builder.py b/tests/masonry/test_builder.py index 14432f8ed..4cf518ddd 100644 --- a/tests/masonry/test_builder.py +++ b/tests/masonry/test_builder.py @@ -18,7 +18,9 @@ def get_project(name: str) -> Path: - return Path(__file__).parent / "builders" / "fixtures" / name + project_directory = Path(__file__).parent / "builders" / "fixtures" / name + assert project_directory.is_dir() + return project_directory @contextmanager @@ -49,7 +51,9 @@ def test_builder_creates_places_built_files_in_specified_directory( ) -> None: poetry = get_poetry("complete") Builder(poetry).build(format, target_dir=tmp_path) - assert all(archive.exists() for archive in tmp_path.glob(get_package_glob(poetry))) + build_artifacts = tuple(tmp_path.glob(get_package_glob(poetry))) + assert len(build_artifacts) > 0 + assert all(archive.exists() for archive in build_artifacts) @pytest.mark.parametrize("format", ["sdist", "wheel", "all"]) @@ -60,8 +64,7 @@ def test_builder_creates_packages_in_dist_directory_if_no_output_is_specified( poetry = Factory().create_poetry(project) Builder(poetry).build(format, target_dir=None) package_directory = project / "dist" + build_artifacts = tuple(package_directory.glob(get_package_glob(poetry))) assert package_directory.is_dir() - assert all( - archive.exists() - for archive in package_directory.glob(get_package_glob(poetry)) - ) + assert len(build_artifacts) > 0 + assert all(archive.exists() for archive in build_artifacts)