Simone/local-ci-cd-system
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(vmgui): pre-flight X display, stop failing silently

Browse Source 
`vmgui` sent the child's stderr to /dev/null, so when the X display was
unreachable (the common case: sudo -u ci-runner loses the desktop user's
XAUTHORITY and xhost wasn't granted) the GUI died with "cannot open display"
and the command still printed success — "nothing opens".

Now it:
- pre-flights the display with xdpyinfo and aborts with the exact
  `xhost +SI:localuser:ci-runner` hint instead of spawning a doomed process;
- drops an inherited XAUTHORITY that this user can't read (falls back to
  host-based xhost auth);
- logs the GUI's stdio to /tmp/vmgui-*.log and grace-polls 1.5s, surfacing the
  log if the GUI exits immediately;
- errors (non-zero) when no DISPLAY is resolvable.

Runbook §4.7 updated with the "if nothing opens" guidance. 13 tests; green ≥90%.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Simone
2026-06-08 00:25:37 +02:00
parent 6f6a304bbe
commit 005662e359
3 changed files with 167 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/python/test_commands_vm.py
+72 -16
View File
@@ -502,26 +502,44 @@ def test_vm_cleanup_backend_unavailable(monkeypatch: pytest.MonkeyPatch, tmp_pat
# ── vmgui ───────────────────────────────────────────────────────────────────
def _capture_launch(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]:
"""Patch _launch_gui to record argv/env instead of spawning a process."""
class _FakeProc:
"""Minimal subprocess.Popen stand-in for vmgui tests."""
def __init__(self, pid: int = 4242, rc: int | None = None) -> None:
self.pid = pid
self._rc = rc
def poll(self) -> int | None:
return self._rc
def _capture_launch(
monkeypatch: pytest.MonkeyPatch, *, rc: int | None = None
) -> dict[str, Any]:
"""Patch _launch_gui + preflight + sleep so no real process is spawned."""
captured: dict[str, Any] = {}
def _fake(argv: list[str], env: dict[str, str]) -> int:
def _fake(argv: list[str], env: dict[str, str], log_path: Path) -> _FakeProc:
captured["argv"] = argv
captured["env"] = env
return 4242
captured["log_path"] = log_path
if rc is not None and rc != 0:
log_path.write_text("cannot open display\n")
return _FakeProc(rc=rc)
monkeypatch.setattr(vm_module, "_launch_gui", _fake)
monkeypatch.setattr(vm_module, "_x_display_reachable", lambda _env: (True, ""))
monkeypatch.setattr(vm_module.time, "sleep", lambda _s: None)
return captured
def test_vm_open_missing_vmx_errors(tmp_path: Path) -> None:
def test_vmgui_missing_vmx_errors(tmp_path: Path) -> None:
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(tmp_path / "nope.vmx")])
assert result.exit_code != 0
assert "VMX file not found" in result.output
def test_vm_open_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
cap = _capture_launch(monkeypatch)
@@ -533,7 +551,7 @@ def test_vm_open_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) ->
assert "pid 4242" in result.output
def test_vm_open_power_on_adds_x(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_power_on_adds_x(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
cap = _capture_launch(monkeypatch)
@@ -543,7 +561,7 @@ def test_vm_open_power_on_adds_x(monkeypatch: pytest.MonkeyPatch, tmp_path: Path
assert "-x" in cap["argv"]
def test_vm_open_fullscreen_adds_capital_x(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_fullscreen_adds_capital_x(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
cap = _capture_launch(monkeypatch)
@@ -555,7 +573,7 @@ def test_vm_open_fullscreen_adds_capital_x(monkeypatch: pytest.MonkeyPatch, tmp_
assert "-X" in cap["argv"] and "-n" in cap["argv"]
def test_vm_open_display_option_overrides_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_display_option_overrides_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
cap = _capture_launch(monkeypatch)
@@ -566,42 +584,80 @@ def test_vm_open_display_option_overrides_env(monkeypatch: pytest.MonkeyPatch, t
assert "display=:1" in result.output
def test_vm_open_warns_without_display(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_errors_without_display(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
_capture_launch(monkeypatch)
monkeypatch.delenv("DISPLAY", raising=False)
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(vmx)])
assert result.exit_code == 0, result.output
assert result.exit_code != 0
assert "no X display" in result.output
def test_vm_open_launch_failure_is_reported(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
def test_vmgui_preflight_failure_reports_xhost(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
_capture_launch(monkeypatch)
# Override the optimistic preflight with a failing probe.
monkeypatch.setattr(
vm_module, "_x_display_reachable", lambda _env: (False, "cannot open display :0")
)
monkeypatch.setenv("DISPLAY", ":0")
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(vmx)])
assert result.exit_code != 0
assert "xhost +SI:localuser:ci-runner" in result.output
def test_vmgui_drops_unreadable_xauthority(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
cap = _capture_launch(monkeypatch)
monkeypatch.setenv("DISPLAY", ":0")
monkeypatch.setenv("XAUTHORITY", str(tmp_path / "missing.Xauthority"))
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(vmx)])
assert result.exit_code == 0, result.output
assert "XAUTHORITY" not in cap["env"]
assert "dropping unreadable XAUTHORITY" in result.output
def test_vmgui_immediate_exit_surfaces_log(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
_capture_launch(monkeypatch, rc=1)
monkeypatch.setenv("DISPLAY", ":0")
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(vmx)])
assert result.exit_code != 0
assert "exited immediately" in result.output
assert "cannot open display" in result.output
def test_vmgui_launch_oserror_is_reported(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
vmx = tmp_path / "clone.vmx"
vmx.write_text("config")
def _boom(argv: list[str], env: dict[str, str]) -> int:
def _boom(argv: list[str], env: dict[str, str], log_path: Path) -> Any:
raise OSError("vmware not found")
monkeypatch.setattr(vm_module, "_launch_gui", _boom)
monkeypatch.setattr(vm_module, "_x_display_reachable", lambda _env: (True, ""))
monkeypatch.setattr(vm_module.time, "sleep", lambda _s: None)
monkeypatch.setenv("DISPLAY", ":0")
result = CliRunner().invoke(cli, ["vmgui", "--vmx", str(vmx)])
assert result.exit_code != 0
assert "failed to launch VMware GUI" in result.output
def test_vm_open_without_vmx_launches_bare_gui(monkeypatch: pytest.MonkeyPatch) -> None:
def test_vmgui_without_vmx_launches_bare_gui(monkeypatch: pytest.MonkeyPatch) -> None:
cap = _capture_launch(monkeypatch)
monkeypatch.setenv("DISPLAY", ":0")
result = CliRunner().invoke(cli, ["vmgui"])
assert result.exit_code == 0, result.output
# Only the binary (no vmx path, no power-on flags).
assert cap["argv"][1:] == [] or cap["argv"][1:] == ["-n"]
assert "-x" not in cap["argv"] and "-X" not in cap["argv"]
assert "pid 4242" in result.output
def test_vm_open_power_on_ignored_without_vmx(monkeypatch: pytest.MonkeyPatch) -> None:
def test_vmgui_power_on_ignored_without_vmx(monkeypatch: pytest.MonkeyPatch) -> None:
cap = _capture_launch(monkeypatch)
monkeypatch.setenv("DISPLAY", ":0")
result = CliRunner().invoke(cli, ["vmgui", "--power-on"])