gh-130486: Fix test_venv fails from within venv #130487
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
FFY00
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @FFY00 for commit 4123be1 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130487%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
Failing buildbot tests appear unconnected to this change. |
|
@FFY00 shall we wait for your review? |
There was a problem hiding this comment.
@vsajip thanks for the ping!
@hrimov, the PR looks good, thank you for spending the time working on it! I just have one thing to point out about the correctness of the code, and a nitpick, which you can fix if you have time.
The buildbot failures indeed seem unrelated — the refleak configs are known to fail from time to time — so, it appears this PR doesn't introduce issues on exoteric platforms like Android and iOS, or in different test environments, as far as our testing infrastructure is concerned, at least, which is what I would look out for in a change like this.
Approving the PR, but look into the small correctness issue before merging 👍
@hrimov thank you again!
Lib/test/test_venv.py
Outdated
| builder = venv.EnvBuilder() | ||
| bin_path = 'bin' | ||
| python_exe = os.path.split(sys.executable)[1] | ||
| expected_exe = os.path.basename(sys.executable) |
There was a problem hiding this comment.
| expected_exe = os.path.basename(sys.executable) | |
| expected_exe = os.path.basename(sys._base_executable) |
Lib/test/test_venv.py
Outdated
| # Now check the venv created from the non-installed python has | ||
| # correct zip path in pythonpath. | ||
| cmd = [self.envpy(), '-S', '-c', 'import sys; print(sys.path)'] | ||
| cmd = [os.path.join(self.env_dir, self.bindir, python_exe), '-S', '-c', 'import sys; print(sys.path)'] |
There was a problem hiding this comment.
nitpick: I would construct the full path in variable, which makes it easier to parse the code.
Updated, thanks! |
4 participants
The changes relax the executable name checks while still verifying that venv correctly handles pip installation and upgrades