Skip to content

test: Stabilize CLI runner subprocess path #6560

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.

Sign up for GitHub

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
franciscojavierarceo merged 1 commit into from
Jun 26, 2026
Merged

test: Stabilize CLI runner subprocess path #6560

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions sdk/python/tests/utils/cli_repo_creator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
that can push the cumulative test time past the pytest global timeout budget.
"""

import os
import random
import string
import subprocess
Expand Down Expand Up @@ -53,6 +54,15 @@ class CliRunner:
modules from the feature repo, and it is hard to clean up that state otherwise.
"""

def _subprocess_env(self) -> dict[str, str]:
env = os.environ.copy()
parent_paths = [path for path in sys.path if path]
existing_pythonpath = env.get("PYTHONPATH")
if existing_pythonpath:
parent_paths.append(existing_pythonpath)

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve child PYTHONPATH precedence

In workflows where PYTHONPATH intentionally contains an empty/current-directory entry for the CLI cwd (for example, .github/workflows/unit_tests.yml exports sdk/python:${PYTHONPATH}:$SITE_PACKAGES), appending the raw existing value after the parent sys.path moves parent site-packages and repo-root paths ahead of the temporary feature repo. Since cli_check_repo() only appends the feature repo path later, imports for repo-local providers or feature definition modules can resolve to an installed/root module with the same name, so CliRunner no longer exercises the isolated repo. Keep the original PYTHONPATH entries first, or add only parent-only paths after the child cwd entry.

Useful? React with 👍 / 👎.

env["PYTHONPATH"] = os.pathsep.join(parent_paths)
return env

def run(self, args: List[str], cwd: Path) -> subprocess.CompletedProcess:
# Apply a conservative timeout to prevent CI hangs from Dask atexit-handler
# stalls or other subprocess blockages.
Expand All @@ -64,6 +74,7 @@ def run(self, args: List[str], cwd: Path) -> subprocess.CompletedProcess:
cwd=cwd,
capture_output=True,
timeout=timeout,
env=self._subprocess_env(),
)
except subprocess.TimeoutExpired:
return subprocess.CompletedProcess(
Expand All @@ -87,6 +98,7 @@ def run_with_output(self, args: List[str], cwd: Path) -> Tuple[int, bytes]:
cwd=cwd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
env=self._subprocess_env(),
)
try:
stdout, _ = proc.communicate(timeout=timeout)
Expand Down
Loading