From 8e305f262fbba1e4a1555efdaee982877d235012 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 31 Jul 2023 04:05:45 -0400 Subject: [PATCH 1/6] add test for the *existing* `install --dry-run` functionality --- tests/functional/test_install.py | 67 +++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index eabddfe58fa..56efe2a5cfc 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -7,7 +7,7 @@ import textwrap from os.path import curdir, join, pardir from pathlib import Path -from typing import Dict, List, Tuple +from typing import Callable, Dict, Iterable, List, Optional, Tuple import pytest @@ -20,6 +20,7 @@ PipTestEnvironment, ResolverVariant, TestData, + TestPipResult, _create_svn_repo, _create_test_package, create_basic_wheel_for_package, @@ -2371,14 +2372,68 @@ def test_install_logs_pip_version_in_debug( assert_re_match(pattern, result.stdout) -def test_install_dry_run(script: PipTestEnvironment, data: TestData) -> None: - """Test that pip install --dry-run logs what it would install.""" - result = script.pip( - "install", "--dry-run", "--find-links", data.find_links, "simple" - ) +@pytest.fixture +def install_find_links( + script: PipTestEnvironment, + data: TestData, +) -> Callable[[Iterable[str], bool, Optional[Path]], TestPipResult]: + def install( + args: Iterable[str], dry_run: bool, target_dir: Optional[Path] + ) -> TestPipResult: + return script.pip( + "install", + *( + ( + "--target", + str(target_dir), + ) + if target_dir is not None + else () + ), + *(("--dry-run",) if dry_run else ()), + "--no-index", + "--find-links", + data.find_links, + *args, + ) + + return install + + +@pytest.mark.parametrize( + "with_target_dir", + (True, False), +) +def test_install_dry_run_nothing_installed( + script: PipTestEnvironment, + tmpdir: Path, + install_find_links: Callable[[Iterable[str], bool, Optional[Path]], TestPipResult], + with_target_dir: bool, +) -> None: + """Test that pip install --dry-run logs what it would install, but doesn't actually + install anything.""" + if with_target_dir: + install_dir = tmpdir / "fake-install" + install_dir.mkdir() + else: + install_dir = None + + result = install_find_links(["simple"], True, install_dir) assert "Would install simple-3.0" in result.stdout assert "Successfully installed" not in result.stdout + script.assert_not_installed("simple") + if with_target_dir: + assert not os.listdir(install_dir) + + # Ensure that the same install command would normally have worked if not for + # --dry-run. + install_find_links(["simple"], False, install_dir) + if with_target_dir: + assert os.listdir(install_dir) + else: + script.assert_installed(simple="3.0") + @pytest.mark.skipif( sys.version_info < (3, 11), From e86da734c7a1bd909ff8c911a276f8f2e6df9348 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 31 Jul 2023 05:41:42 -0400 Subject: [PATCH 2/6] add test for hash mismatch --- tests/functional/test_fast_deps.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/functional/test_fast_deps.py b/tests/functional/test_fast_deps.py index 0109db825b7..b76b833b938 100644 --- a/tests/functional/test_fast_deps.py +++ b/tests/functional/test_fast_deps.py @@ -2,12 +2,14 @@ import json import os import pathlib +import re from os.path import basename from typing import Iterable from pip._vendor.packaging.utils import canonicalize_name from pytest import mark +from pip._internal.utils.misc import hash_file from tests.lib import PipTestEnvironment, TestData, TestPipResult @@ -101,3 +103,31 @@ def test_hash_mismatch(script: PipTestEnvironment, tmp_path: pathlib.Path) -> No expect_error=True, ) assert "DO NOT MATCH THE HASHES" in result.stderr + + +@mark.network +def test_hash_mismatch_existing_download( + script: PipTestEnvironment, tmp_path: pathlib.Path +) -> None: + reqs = tmp_path / "requirements.txt" + reqs.write_text("idna==2.10") + dl_dir = tmp_path / "downloads" + dl_dir.mkdir() + idna_wheel = dl_dir / "idna-2.10-py2.py3-none-any.whl" + idna_wheel.write_text("asdf") + result = script.pip( + "download", + "--use-feature=fast-deps", + "-r", + str(reqs), + "-d", + str(dl_dir), + allow_stderr_warning=True, + ) + assert re.search( + r"WARNING: Previously-downloaded file.*has bad hash", result.stderr + ) + assert ( + hash_file(str(idna_wheel))[0].hexdigest() + == "b97d804b1e9b523befed77c48dacec60e6dcb0b5391d57af6a65a312a90648c0" + ) From 67ff36b838f543037169828be95f86f903d609c6 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 31 Jul 2023 04:07:55 -0400 Subject: [PATCH 3/6] move directory metadata test out of req install tests --- tests/unit/metadata/test_metadata.py | 14 ++++++++++++++ tests/unit/test_req.py | 17 ----------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/tests/unit/metadata/test_metadata.py b/tests/unit/metadata/test_metadata.py index f77178fb9c1..47093fb54d1 100644 --- a/tests/unit/metadata/test_metadata.py +++ b/tests/unit/metadata/test_metadata.py @@ -129,3 +129,17 @@ def test_dist_found_in_zip(tmp_path: Path) -> None: dist = get_environment([location]).get_distribution("pkg") assert dist is not None and dist.location is not None assert Path(dist.location) == Path(location) + + +@pytest.mark.parametrize( + "path", + ( + "/path/to/foo.egg-info".replace("/", os.path.sep), + # Tests issue fixed by https://github.com/pypa/pip/pull/2530 + "/path/to/foo.egg-info/".replace("/", os.path.sep), + ), +) +def test_trailing_slash_directory_metadata(path: str) -> None: + dist = get_directory_distribution(path) + assert dist.raw_name == dist.canonical_name == "foo" + assert dist.location == "/path/to".replace("/", os.path.sep) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 545828f8eea..2d1fa269490 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -23,7 +23,6 @@ PreviousBuildDirError, ) from pip._internal.index.package_finder import PackageFinder -from pip._internal.metadata import select_backend from pip._internal.models.direct_url import ArchiveInfo, DirectUrl, DirInfo, VcsInfo from pip._internal.models.link import Link from pip._internal.network.session import PipSession @@ -600,22 +599,6 @@ def test_url_preserved_editable_req(self) -> None: assert req.link is not None assert req.link.url == url - @pytest.mark.parametrize( - "path", - ( - "/path/to/foo.egg-info".replace("/", os.path.sep), - # Tests issue fixed by https://github.com/pypa/pip/pull/2530 - "/path/to/foo.egg-info/".replace("/", os.path.sep), - ), - ) - def test_get_dist(self, path: str) -> None: - req = install_req_from_line("foo") - req.metadata_directory = path - dist = req.get_dist() - assert isinstance(dist, select_backend().Distribution) - assert dist.raw_name == dist.canonical_name == "foo" - assert dist.location == "/path/to".replace("/", os.path.sep) - def test_markers(self) -> None: for line in ( # recommended syntax From 20b54de4dfb3567e088ca058819de43df90364d9 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue, 1 Aug 2023 13:34:50 -0400 Subject: [PATCH 4/6] add news --- news/12183.trivial.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/12183.trivial.rst diff --git a/news/12183.trivial.rst b/news/12183.trivial.rst new file mode 100644 index 00000000000..c22e854c9a5 --- /dev/null +++ b/news/12183.trivial.rst @@ -0,0 +1 @@ +Add test cases for some behaviors of ``install --dry-run`` and ``--use-feature=fast-deps``. From e27af2c3c9a336d6379b24f92cd98cb14c1bd090 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sat, 12 Aug 2023 10:46:22 -0400 Subject: [PATCH 5/6] add notes on hash mismatch testing --- tests/functional/test_fast_deps.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_fast_deps.py b/tests/functional/test_fast_deps.py index b76b833b938..9e529c0891e 100644 --- a/tests/functional/test_fast_deps.py +++ b/tests/functional/test_fast_deps.py @@ -106,9 +106,12 @@ def test_hash_mismatch(script: PipTestEnvironment, tmp_path: pathlib.Path) -> No @mark.network -def test_hash_mismatch_existing_download( +def test_hash_mismatch_existing_download_for_metadata_only_wheel( script: PipTestEnvironment, tmp_path: pathlib.Path ) -> None: + """Metadata-only wheels from PEP 658 or fast-deps check for hash matching in + a separate code path than when the wheel is downloaded all at once. Make sure we + still check for hash mismatches.""" reqs = tmp_path / "requirements.txt" reqs.write_text("idna==2.10") dl_dir = tmp_path / "downloads" @@ -117,6 +120,7 @@ def test_hash_mismatch_existing_download( idna_wheel.write_text("asdf") result = script.pip( "download", + # Ensure that we have a metadata-only dist for idna. "--use-feature=fast-deps", "-r", str(reqs), @@ -127,6 +131,7 @@ def test_hash_mismatch_existing_download( assert re.search( r"WARNING: Previously-downloaded file.*has bad hash", result.stderr ) + # This is the correct hash for idna==2.10. assert ( hash_file(str(idna_wheel))[0].hexdigest() == "b97d804b1e9b523befed77c48dacec60e6dcb0b5391d57af6a65a312a90648c0" From 8704c7a5dbd62367b01f39317b6213578683f4f6 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sat, 12 Aug 2023 10:51:33 -0400 Subject: [PATCH 6/6] remove unnecessary fixture --- tests/functional/test_install.py | 54 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 56efe2a5cfc..5e8a82fb345 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -7,7 +7,7 @@ import textwrap from os.path import curdir, join, pardir from pathlib import Path -from typing import Callable, Dict, Iterable, List, Optional, Tuple +from typing import Dict, Iterable, List, Optional, Tuple import pytest @@ -2372,32 +2372,30 @@ def test_install_logs_pip_version_in_debug( assert_re_match(pattern, result.stdout) -@pytest.fixture def install_find_links( script: PipTestEnvironment, data: TestData, -) -> Callable[[Iterable[str], bool, Optional[Path]], TestPipResult]: - def install( - args: Iterable[str], dry_run: bool, target_dir: Optional[Path] - ) -> TestPipResult: - return script.pip( - "install", - *( - ( - "--target", - str(target_dir), - ) - if target_dir is not None - else () - ), - *(("--dry-run",) if dry_run else ()), - "--no-index", - "--find-links", - data.find_links, - *args, - ) - - return install + args: Iterable[str], + *, + dry_run: bool, + target_dir: Optional[Path], +) -> TestPipResult: + return script.pip( + "install", + *( + ( + "--target", + str(target_dir), + ) + if target_dir is not None + else () + ), + *(("--dry-run",) if dry_run else ()), + "--no-index", + "--find-links", + data.find_links, + *args, + ) @pytest.mark.parametrize( @@ -2406,8 +2404,8 @@ def install( ) def test_install_dry_run_nothing_installed( script: PipTestEnvironment, + data: TestData, tmpdir: Path, - install_find_links: Callable[[Iterable[str], bool, Optional[Path]], TestPipResult], with_target_dir: bool, ) -> None: """Test that pip install --dry-run logs what it would install, but doesn't actually @@ -2418,7 +2416,9 @@ def test_install_dry_run_nothing_installed( else: install_dir = None - result = install_find_links(["simple"], True, install_dir) + result = install_find_links( + script, data, ["simple"], dry_run=True, target_dir=install_dir + ) assert "Would install simple-3.0" in result.stdout assert "Successfully installed" not in result.stdout @@ -2428,7 +2428,7 @@ def test_install_dry_run_nothing_installed( # Ensure that the same install command would normally have worked if not for # --dry-run. - install_find_links(["simple"], False, install_dir) + install_find_links(script, data, ["simple"], dry_run=False, target_dir=install_dir) if with_target_dir: assert os.listdir(install_dir) else: