Skip to content

Conversation

@hroncok
Copy link
Contributor

@hroncok hroncok commented May 24, 2022

This has the same reasoning as in #3133 -- the test imported the previously installed setuptools version in our environment and since that version did not yet use __EASYINSTALL_INDEX, it reached https://pypi.org/simple/ instead. That way, we recognized it as a problem. If the installed version had already used __EASYINSTALL_INDEX, we would probably never realize that it is the installed version that is being tested here.

The three individual commits follow my train of thoughts, so I preserved them here separated, for easier review.

  1. The test was very hard to grasp for me, so I removed the pytest.raises detour -- happy to revert if there was a reason for it
  2. I changed the test to modify the $PYTHONPATH value instead of overriding it, making it actually work in our environment
  3. When doing it, I realized adding install_root to $PYTHONPATH is not necessary for this test, so I removed it -- happy to squash that to the previous one if you prefer that

Thanks.

hroncok and others added 4 commits May 24, 2022 19:33
So if the tested setuptools is only located in manually set $PYTHONPATH, it still tests that setuptools instead of the previously installed one in site-packages.
@abravalheri
Copy link
Contributor

abravalheri commented Jun 7, 2022

Thank you very much @hroncok, sorry for the delay in reviewing/merging.

In fd4a482, I have added a small news fragment based on the title of your PR. I hope you don't mind.

@hroncok
Copy link
Contributor Author

hroncok commented Jun 7, 2022

I don't mind.

@abravalheri abravalheri merged commit da436d1 into pypa:main Jun 7, 2022
@hroncok hroncok deleted the test_preserve_pythonpath branch October 24, 2022 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants