fix: preserve docker discovery in backend preflight
Follow up on salvaged PR #940 by reusing find_docker() during the new availability check so non-PATH Docker Desktop installs still work. Add a regression test covering the resolved executable path.
This commit is contained in:
@@ -23,22 +23,22 @@ def _make_dummy_env(**kwargs):
|
||||
|
||||
|
||||
def test_ensure_docker_available_logs_and_raises_when_not_found(monkeypatch, caplog):
|
||||
"""When docker is missing from PATH, we should raise a clear error and log it."""
|
||||
"""When docker cannot be found, raise a clear error before mini-swe setup."""
|
||||
|
||||
def _raise_not_found(*args, **kwargs):
|
||||
raise FileNotFoundError("docker not found")
|
||||
|
||||
monkeypatch.setattr(docker_env, "subprocess", docker_env.subprocess)
|
||||
monkeypatch.setattr(docker_env.subprocess, "run", _raise_not_found)
|
||||
monkeypatch.setattr(docker_env, "find_docker", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
docker_env.subprocess,
|
||||
"run",
|
||||
lambda *args, **kwargs: pytest.fail("subprocess.run should not be called when docker is missing"),
|
||||
)
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
with pytest.raises(RuntimeError) as excinfo:
|
||||
_make_dummy_env()
|
||||
|
||||
# Error message should mention that docker is not in PATH
|
||||
assert "Docker executable not found in PATH" in str(excinfo.value)
|
||||
assert "Docker executable not found in PATH or known install locations" in str(excinfo.value)
|
||||
assert any(
|
||||
"Docker backend selected but 'docker' executable was not found in PATH"
|
||||
"no docker executable was found in PATH or known install locations"
|
||||
in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
@@ -48,8 +48,9 @@ def test_ensure_docker_available_logs_and_raises_on_timeout(monkeypatch, caplog)
|
||||
"""When docker version times out, surface a helpful error instead of hanging."""
|
||||
|
||||
def _raise_timeout(*args, **kwargs):
|
||||
raise subprocess.TimeoutExpired(cmd=["docker", "version"], timeout=5)
|
||||
raise subprocess.TimeoutExpired(cmd=["/custom/docker", "version"], timeout=5)
|
||||
|
||||
monkeypatch.setattr(docker_env, "find_docker", lambda: "/custom/docker")
|
||||
monkeypatch.setattr(docker_env.subprocess, "run", _raise_timeout)
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
@@ -58,7 +59,30 @@ def test_ensure_docker_available_logs_and_raises_on_timeout(monkeypatch, caplog)
|
||||
|
||||
assert "Docker daemon is not responding" in str(excinfo.value)
|
||||
assert any(
|
||||
"docker version' timed out" in record.getMessage()
|
||||
"/custom/docker version' timed out" in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
|
||||
|
||||
def test_ensure_docker_available_uses_resolved_executable(monkeypatch):
|
||||
"""When docker is found outside PATH, preflight should use that resolved path."""
|
||||
|
||||
calls = []
|
||||
|
||||
def _run(cmd, **kwargs):
|
||||
calls.append((cmd, kwargs))
|
||||
return subprocess.CompletedProcess(cmd, 0, stdout="Docker version", stderr="")
|
||||
|
||||
monkeypatch.setattr(docker_env, "find_docker", lambda: "/opt/homebrew/bin/docker")
|
||||
monkeypatch.setattr(docker_env.subprocess, "run", _run)
|
||||
|
||||
docker_env._ensure_docker_available()
|
||||
|
||||
assert calls == [
|
||||
(["/opt/homebrew/bin/docker", "version"], {
|
||||
"capture_output": True,
|
||||
"text": True,
|
||||
"timeout": 5,
|
||||
})
|
||||
]
|
||||
|
||||
|
||||
@@ -85,31 +85,43 @@ _storage_opt_ok: Optional[bool] = None # cached result across instances
|
||||
def _ensure_docker_available() -> None:
|
||||
"""Best-effort check that the docker CLI is available before use.
|
||||
|
||||
This mirrors the gateway's behaviour of surfacing a clear, actionable
|
||||
error when Docker is not installed or not in PATH, instead of failing
|
||||
later with a low-level FileNotFoundError.
|
||||
Reuses ``find_docker()`` so this preflight stays consistent with the rest of
|
||||
the Docker backend, including known non-PATH Docker Desktop locations.
|
||||
"""
|
||||
docker_exe = find_docker()
|
||||
if not docker_exe:
|
||||
logger.error(
|
||||
"Docker backend selected but no docker executable was found in PATH "
|
||||
"or known install locations. Install Docker Desktop and ensure the "
|
||||
"CLI is available."
|
||||
)
|
||||
raise RuntimeError(
|
||||
"Docker executable not found in PATH or known install locations. "
|
||||
"Install Docker and ensure the 'docker' command is available."
|
||||
)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["docker", "version"],
|
||||
[docker_exe, "version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
logger.error(
|
||||
"Docker backend selected but 'docker' executable was not found in PATH. "
|
||||
"Install Docker Desktop and ensure 'docker' is available on the command line.",
|
||||
"Docker backend selected but the resolved docker executable '%s' could "
|
||||
"not be executed.",
|
||||
docker_exe,
|
||||
exc_info=True,
|
||||
)
|
||||
raise RuntimeError(
|
||||
"Docker executable not found in PATH. Install Docker and ensure "
|
||||
"the 'docker' command is available."
|
||||
"Docker executable could not be executed. Check your Docker installation."
|
||||
)
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.error(
|
||||
"Docker backend selected but 'docker version' timed out. "
|
||||
"Docker backend selected but '%s version' timed out. "
|
||||
"The Docker daemon may not be running.",
|
||||
docker_exe,
|
||||
exc_info=True,
|
||||
)
|
||||
raise RuntimeError(
|
||||
@@ -124,8 +136,9 @@ def _ensure_docker_available() -> None:
|
||||
else:
|
||||
if result.returncode != 0:
|
||||
logger.error(
|
||||
"Docker backend selected but 'docker version' failed "
|
||||
"Docker backend selected but '%s version' failed "
|
||||
"(exit code %d, stderr=%s)",
|
||||
docker_exe,
|
||||
result.returncode,
|
||||
result.stderr.strip(),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user