Skip to content

Commit

Permalink
Raise KhiopsEnvironmentError on reading failing khiops_env script
Browse files Browse the repository at this point in the history
closes #300
  • Loading branch information
popescu-v committed Dec 18, 2024
1 parent cb17bdb commit 1ae7c92
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 1 deletion.
7 changes: 6 additions & 1 deletion khiops/core/internals/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,12 @@ def _initialize_khiops_environment(self):
stderr=subprocess.PIPE,
universal_newlines=True,
) as khiops_env_process:
stdout, _ = khiops_env_process.communicate()
stdout, stderr = khiops_env_process.communicate()
if khiops_env_process.returncode != 0:
raise KhiopsEnvironmentError(
"Error initializing the environment for Khiops from the "
f"{khiops_env_path} script. Contents of stderr:\n{stderr}"
)
for line in stdout.split("\n"):
tokens = line.rstrip().split(maxsplit=1)
if len(tokens) == 2:
Expand Down
65 changes: 65 additions & 0 deletions tests/test_khiops_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@

import os
import platform
import re
import shutil
import stat
import subprocess
import tempfile
import unittest

import khiops.core as kh
Expand Down Expand Up @@ -95,6 +98,68 @@ def test_runner_has_mpiexec_on_linux(self):
else:
self.skipTest("Skipping test: MPI support is not installed")

def test_environment_error_on_bogus_khiops_env_script(self):
"""Test that error is raised on reading erroneous khiops_env script"""

with tempfile.TemporaryDirectory() as temp_dir:

# Create temporary khiops_env path
temp_khiops_env_file_path = os.path.join(temp_dir, "khiops_env")
if platform.system() == "Windows":
temp_khiops_env_file_path += ".cmd"

# Replace the khiops_env with a script that fails showing an error message
with open(
temp_khiops_env_file_path, "w", encoding="utf-8"
) as temp_khiops_env_file:
test_error_message = "Test Khiops environment error"
if platform.system() == "Windows":
temp_khiops_env_file.write("@echo off\r\n")
temp_khiops_env_file.write(f"echo {test_error_message}>&2\r\n")
temp_khiops_env_file.write("exit /b 1")
else:
temp_khiops_env_file.write("#!/bin/bash\n")
temp_khiops_env_file.write(f'>&2 echo "{test_error_message}"\n')
temp_khiops_env_file.write("exit 1")

# Make the temporary khiops_env file executable
os.chmod(
temp_khiops_env_file_path,
os.stat(temp_khiops_env_file_path).st_mode | stat.S_IEXEC,
)

# Store initial PATH
original_path_env_var = os.environ["PATH"]

# Prepend path to temporary khiops_env to PATH
os.environ["PATH"] = temp_dir + os.pathsep + original_path_env_var

# Create new KhiopsLocalRunner and capture the exception
with self.assertRaises(KhiopsEnvironmentError) as context:
_ = KhiopsLocalRunner()

# Restore initial PATH
os.environ["PATH"] = original_path_env_var

# We use the Khiops CC path to find khiops_env's
# Note: The Khiops executable directory may not be the same as for
# khiops_env
khiops_env_file_path = os.path.join(
os.path.dirname(kh.get_runner().khiops_coclustering_path), "khiops_env"
)
if platform.system() == "Windows":
khiops_env_file_path += ".cmd"

# Check that the script error message matches the expected one
expected_msg_regex = re.compile(
"Error initializing the environment for Khiops from the "
f".*{os.path.basename(khiops_env_file_path)} script. "
f"Contents of stderr:\n{test_error_message}",
re.DOTALL,
)
output_msg = str(context.exception)
self.assertRegex(output_msg, expected_msg_regex)

def test_runner_with_conda_based_environment(self):
"""Test that local runner works in non-Conda, Conda-based environments"""

Expand Down

0 comments on commit 1ae7c92

Please sign in to comment.