fix: explicit PYTHONPATH for isolated test subprocesses (#593)#594
fix: explicit PYTHONPATH for isolated test subprocesses (#593)#594planetf1 wants to merge 1 commit intogenerative-computing:mainfrom
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
planetf1
changed the title
planetf1
force-pushed
the
fix/subprocess-pythonpath
branch
from
e71015c to
7fadb7d
Compare
|
|
||
| repo_root = str(pathlib.Path(__file__).parent.parent.parent.resolve()) | ||
| env = os.environ.copy() | ||
| env["PYTHONPATH"] = f"{repo_root}{os.pathsep}{env.get('PYTHONPATH', '')}" |
There was a problem hiding this comment.
One concern: Will this cause issues if someone has a PYTHONPATH already set? I'm guessing it would only cause issues if someone has some weird setup. To hedge against that, should we set the variable with PYTHONPATH=env.get('PYTHONPATH', "")<; or :>{repo_root}{os.pathsep}.
At the very least, I don't think it makes sense adding PYTHONPATH to the repo root if already exists. I think that will jumble the path.
There was a problem hiding this comment.
I think you can set it to have multiple paths (that's what I was trying to do with my quick example); you set it to the existing path if it exists and then add a link to the base repo as well
2 participants
Explicit PYTHONPATH for isolated test subprocesses
Type of PR
Description
Fixes #593
Fixes an issue where tests spawned in isolated subprocess environments (such as via
uv run pytest) fail withModuleNotFoundError. The parent pytest process dynamically injects the package path intosys.path, but this memory state is not naturally inherited by childsubprocess.Popencalls. This change explicitly resolves the repository root and injects it into thePYTHONPATHenvironment variable for both the heavy GPU test runner and the examples runner.Verified locally, and confirmed working for both HuggingFace and vLLM test suites on LSF HPC nodes.
Testing