Similar variable names referring to different things in PipTestEnvironment

[deveshks]

Environment

• pip version: 19.2.3
• Python version: 3.7.7
• OS: OSX 10.15.3

Description
While trying to write some unit tests while referring to

pip/tests/functional/test_uninstall.py

Line 528 in 8c95de1

def test_uninstall_editable_and_pip_install(script, data):

I came across two member variables from the script fixture, one is script.site_packages_path which points to site-packages which lies inside virtualenv created by pytest while running test cases and the other is script.site_packages, which points to site-packages inside virtualenv which is being used as a development environment for pip

For example:

script.site_packages => /Users/xyz/pip/venv/lib/python3.7/site-packages
script.site_packages_path => /private/var/folders/xg/blp845_s0xn093dyrtgy936h0000gp/T/pytest-of-xyz/pytest-1/test_uninstall_editable_and_pi0/workspace/venv/lib/python3.7/site-packages

Expected behavior

The two variable names should be cleaned up so that they can refer to their actual meanings. The current names are similar sounding but have pretty different uses

How to Reproduce

Run any test case within test_uninstall.py, enabling console print output by -s e.g.
tox -e py tests/functional/test_uninstall.py::test_uninstall_editable_and_pip_install -s, and add print statements within a function to see the values

[gutsytechster]

We can rename script.site_packages_path to script.test_site_packages(which I can think of now). So, can I say that renaming either of them to something suitable, would resolve this issue?

Of course, there could be better renaming options than I suggested.

[deveshks]

I think we can say script.site_packages -> script.venv_site_packages or script.dev_site_packages, as well, apart from your suggestion.

[gutsytechster]

Yes, that can also be used. But to me, when someone says site_packages wrt python, it means the path to site_package of venv python or system-level python(when no venv is being used).

[uranusjr]

Is test_venv_site_packages too verbose? 😆

[gutsytechster]

On a side note, I see that PipTestEnvironment defines only site_packages_path and I don't see any site_packages definition within it.

What I don't get is how site_packages is accessible via script fixture?

@uranusjr, I think verbose is better than being in confusion. I am in favour of what you suggested. But which variable are you suggesting to rename exactly?

[uranusjr]

I don't see any site_packages definition within it.

There’s a setattr() in PipTestEnvironment.__init__() that sets site_packages from site_packages_path. Judging from the implementation, it and a bunch other attributes are simply a relative path equivalent of their *_path counterparts.

I think the issue here is probably that we should name those better instead, e.g. site_packages_path to site_packages_abspath, and site_packages to site_packages_relpath.

[cjc7373]

Like @uranusjr said, in PipTestEnvironment.__init__() line 494, we set site_packages as a relative path of site_packages_path.
But what @deveshks posted is strange:

script.site_packages => /Users/xyz/pip/venv/lib/python3.7/site-packages
script.site_packages_path => /private/var/folders/xg/blp845_s0xn093dyrtgy936h0000gp/T/pytest-of-xyz/pytest-1/test_uninstall_editable_and_pi0/workspace/venv/lib/python3.7/site-packages

This site_packages doesn't seem like a relative path.