Skip to content

Commit

Permalink
Use shell=True for linux to fix conde feedstock runner issue
Browse files Browse the repository at this point in the history
The issue with conda feedstock runners is they use some variable for
abspath, which doesn't get correctly expanded by `os.path.expandvar`.

This requires using `shell=True` for linux as well.
  • Loading branch information
IvoDD committed Oct 18, 2024
1 parent 67553e7 commit 0d1305a
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions python/tests/compat/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ def is_running_on_windows():

def run_shell_command(command : List[Union[str, os.PathLike]], cwd : Optional[os.PathLike] = None) -> subprocess.CompletedProcess:
logger.info(f"Executing command: {command}")
# shell=True is required for running the correct python executable on Windows
result = subprocess.run(command, cwd=cwd, capture_output=True, shell=is_running_on_windows())
result = None
if is_running_on_windows():
# shell=True is required for running the correct python executable on Windows
result = subprocess.run(command, cwd=cwd, capture_output=True, shell=True)
else:
# On linux we need shell=True for conda feedstock runners (because otherwise they fail to expand path variables)
# But to correctly work with shell=True we need a single command string.
command_string = ' '.join(command)
result = subprocess.run(command_string, cwd=cwd, capture_output=True, shell=True, stdin=subprocess.DEVNULL)
if result.returncode != 0:
logger.warning(f"Command failed, stdout: {str(result.stdout)}, stderr: {str(result.stderr)}")
return result
Expand Down Expand Up @@ -146,10 +153,7 @@ def assert_read(self, sym : str, df) -> None:
def old_venv(request):
version = request.param
path = os.path.join("venvs", version)
# We need to expandvars because on some systems (e.g. Conda Feedstock) abspath uses env variables.
requirements_file = os.path.expandvars(
os.path.abspath(os.path.join("tests", "compat", f"requirements-{version}.txt"))
)
requirements_file = os.path.abspath(os.path.join("tests", "compat", f"requirements-{version}.txt"))
with Venv(path, requirements_file, version) as old_venv:
yield old_venv

Expand Down

0 comments on commit 0d1305a

Please sign in to comment.