Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 27 additions & 2 deletions src/openjd/adaptor_runtime/process/_logging_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import annotations

import logging
import shutil
import signal
import subprocess
import uuid
Expand All @@ -21,7 +22,11 @@


class LoggingSubprocess(object):
"""A process whose stdout/stderr lines are sent to a configurable logger"""
"""A process whose stdout/stderr lines are sent to a configurable logger

Raises:
FileNotFoundError: When the executable the adaptor is set to run cannot be found.
"""

_logger: logging.Logger
_process: subprocess.Popen
Expand Down Expand Up @@ -64,7 +69,27 @@ def __init__(
# In Windows, this is required for signal. SIGBREAK will be sent to the entire process group.
# Without this one, current process will also get the SIGBREAK and may react incorrectly.
popen_params.update(creationflags=subprocess.CREATE_NEW_PROCESS_GROUP) # type: ignore[attr-defined]
self._process = subprocess.Popen(**popen_params)

try:
self._process = subprocess.Popen(**popen_params)

except FileNotFoundError as fnf_error:
# In ManagedProcess we prepend the executable to the list of arguments before creating a LoggingSubprocess
executable = args[0]
exe_path = shutil.which(executable)

# If we didn't find the executable found by which
if exe_path is not None:
raise FileNotFoundError(
f"Could not find executable at: {exe_path} using alias {executable}\n"
f"Error:{fnf_error}"
Comment on lines +82 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own curiosity, how does a user end up in this situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If an alias gets broken this'll happen. I can't imagine this really ever gets run, but if it does we'd be happy to have it. :)

)

raise FileNotFoundError(
f"Could not find the specified command: {executable}\n"
f"Ensure the command is available from the PATH environment variable.\n"
f"Error:{fnf_error}"
)

if not self._process.stdout: # pragma: no cover
raise RuntimeError("process stdout not set")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ def test_startup_directory(self, startup_dir: str | None, caplog):
def test_startup_directory_empty_posix(self):
"""When calling LoggingSubprocess with an empty cwd, FileNotFoundError will be raised."""
args = ["pwd"]
with pytest.raises(FileNotFoundError) as excinfo:
with pytest.raises(FileNotFoundError) as exc_info:
LoggingSubprocess(args=args, startup_directory="")
assert "[Errno 2] No such file or directory: ''" in str(excinfo.value)
assert "[Errno 2] No such file or directory: ''" in str(exc_info.value)

@pytest.mark.skipif(not OSName.is_windows(), reason="Only run this test in Windows.")
def test_startup_directory_empty_windows(self):
Expand Down Expand Up @@ -151,6 +151,13 @@ def test_log_levels(self, log_level: int, caplog):

assert any(r.message == message and r.levelno == _STDERR_LEVEL for r in records)

def test_executable_not_found(self):
"""When calling LoggingSubprocess with a missing executable, FileNotFoundError will be raised"""
args = ["missing_executable"]
with pytest.raises(FileNotFoundError) as exc_info:
LoggingSubprocess(args=args)
assert "Could not find the specified command" in str(exc_info.value)


class TestIntegrationRegexHandler(object):
"""Integration tests for LoggingSubprocess"""
Expand Down
Loading