Skip to content

Use installer instead of setuptools in test suite #11291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
# Parser will be available from the public API in pytest >= 7.0.0:
# https://github.com/pytest-dev/pytest/commit/538b5c24999e9ebb4fab43faabc8bcc28737bcdf
from _pytest.config.argparsing import Parser
from setuptools.wheel import Wheel
from installer import install
from installer.destinations import SchemeDictionaryDestination
from installer.sources import WheelFile

from pip._internal.cli.main import main as pip_entry_point
from pip._internal.locations import _USE_SYSCONFIG
Expand Down Expand Up @@ -364,10 +366,29 @@ def _common_wheel_editable_install(
wheel_candidates = list(common_wheels.glob(f"{package}-*.whl"))
assert len(wheel_candidates) == 1, wheel_candidates
install_dir = tmpdir_factory.mktemp(package) / "install"
Wheel(wheel_candidates[0]).install_as_egg(install_dir)
(install_dir / "EGG-INFO").rename(install_dir / f"{package}.egg-info")
assert compileall.compile_dir(str(install_dir), quiet=1)
return install_dir
lib_install_dir = install_dir / "lib"
bin_install_dir = install_dir / "bin"
with WheelFile.open(wheel_candidates[0]) as source:
install(
source,
SchemeDictionaryDestination(
{
"purelib": os.fspath(lib_install_dir),
"platlib": os.fspath(lib_install_dir),
"scripts": os.fspath(bin_install_dir),
},
interpreter=sys.executable,
Copy link
Member

@pradyunsg pradyunsg Jul 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean that the executables/scripts from these installs will run with the same environment as the test runner (i.e. pytest), which is likely not what we want.

This should really be using the Python executable from the virtualenv. I think the right thing to do here would be to change up the virtualenv fixture, with the object coming from that getting a new .install_from_wheel(path) method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these scripts are never used. I'll remove the bin dir and see if the tests pass.
Then we can do a refactoring in followup, after #11288.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, let's do that!

I might end up picking up the refactoring, as part of https://github.com/pradyunsg/pip/tree/improve-test-isolation. :)

script_kind="posix",
),
additional_metadata={},
)
# The scripts are not necessary for our use cases, and they would be installed with
# the wrong interpreter, so remove them.
# TODO consider a refactoring by adding a install_from_wheel(path) method
# to the virtualenv fixture.
if bin_install_dir.exists():
shutil.rmtree(bin_install_dir)
return lib_install_dir


@pytest.fixture(scope="session")
Expand All @@ -389,13 +410,12 @@ def coverage_install(
return _common_wheel_editable_install(tmpdir_factory, common_wheels, "coverage")


def install_egg_link(
venv: VirtualEnvironment, project_name: str, egg_info_dir: Path
def install_pth_link(
venv: VirtualEnvironment, project_name: str, lib_dir: Path
) -> None:
with open(venv.site / "easy-install.pth", "a") as fp:
fp.write(str(egg_info_dir.resolve()) + "\n")
with open(venv.site / (project_name + ".egg-link"), "w") as fp:
fp.write(str(egg_info_dir) + "\n.")
venv.site.joinpath(f"_pip_testsuite_{project_name}.pth").write_text(
str(lib_dir.resolve()), encoding="utf-8"
)


@pytest.fixture(scope="session")
Expand All @@ -418,7 +438,7 @@ def virtualenv_template(
venv = VirtualEnvironment(tmpdir.joinpath("venv_orig"), venv_type=venv_type)

# Install setuptools and pip.
install_egg_link(venv, "setuptools", setuptools_install)
install_pth_link(venv, "setuptools", setuptools_install)
pip_editable = tmpdir_factory.mktemp("pip") / "pip"
shutil.copytree(pip_src, pip_editable, symlinks=True)
# noxfile.py is Python 3 only
Expand All @@ -433,7 +453,7 @@ def virtualenv_template(

# Install coverage and pth file for executing it in any spawned processes
# in this virtual environment.
install_egg_link(venv, "coverage", coverage_install)
install_pth_link(venv, "coverage", coverage_install)
# zz prefix ensures the file is after easy-install.pth.
with open(venv.site / "zz-coverage-helper.pth", "a") as f:
f.write("import coverage; coverage.process_startup()")
Expand Down Expand Up @@ -481,7 +501,7 @@ def virtualenv(

@pytest.fixture
def with_wheel(virtualenv: VirtualEnvironment, wheel_install: Path) -> None:
install_egg_link(virtualenv, "wheel", wheel_install)
install_pth_link(virtualenv, "wheel", wheel_install)


class ScriptFactory(Protocol):
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_local_columns_flag(simple_script: PipTestEnvironment) -> None:
assert "Package" in result.stdout
assert "Version" in result.stdout
assert "simple (1.0)" not in result.stdout
assert "simple 1.0" in result.stdout, str(result)
assert "simple 1.0" in result.stdout, str(result)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test changes because pip list --local does not show coverage and setuptools while it did before.
Therefore the column is smaller.

I think this is correct because coverage and setuptools are outside of the virtualenv, according to our implementation of BaseDistribution.local.



def test_multiple_exclude_and_normalization(
Expand Down
1 change: 1 addition & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
cryptography
freezegun
installer
pytest
pytest-cov
pytest-rerunfailures
Expand Down