fix(hopper): preserve an explicit display number zero in the X11 helper (#5214)
`start_xvfb` treated `--display-number 0` as omitted because it used the option's truthiness, so an explicit `:0` was replaced by a random display and its collision check ran against that random display instead. Select the random display only when the option is absent, and add a fixture-backed regression test covering explicit zero, an occupied explicit zero, explicit one, and omission. Fixes #5189 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b5442df02d
commit
c2fe0c6818
3 files changed
+114
-1
No files matched your search
@@ -580,7 +580,11 @@ def start_xvfb(
|
||||
authority = session / "Xauthority"
|
||||
collision: AdapterFailure | None = None
|
||||
for _ in range(3):
|
||||
display_number = args.display_number or (100 + secrets.randbelow(900))
|
||||
display_number = (
|
||||
args.display_number
|
||||
if args.display_number is not None
|
||||
else 100 + secrets.randbelow(900)
|
||||
)
|
||||
display_name = f":{display_number}"
|
||||
if Path(f"/tmp/.X11-unix/X{display_number}").exists() or Path(
|
||||
f"/tmp/.X{display_number}-lock"
|
||||
|
||||
@@ -34,6 +34,9 @@ const cleanupTimeoutFixturePath = fileURLToPath(
|
||||
const ownerLifetimeFixturePath = fileURLToPath(
|
||||
new URL("../../../fixtures/hopperOwnerLifetime.py", import.meta.url),
|
||||
);
|
||||
const displaySelectionFixturePath = fileURLToPath(
|
||||
new URL("../../../fixtures/hopperDisplaySelection.py", import.meta.url),
|
||||
);
|
||||
|
||||
type DiagnosticOverrides = Partial<
|
||||
Omit<HopperStartupDiagnostic, "failure_code" | "status">
|
||||
@@ -140,6 +143,43 @@ describe("Hopper process cleanup", () => {
|
||||
);
|
||||
});
|
||||
|
||||
describe("Hopper demo explicit display number", () => {
|
||||
it.skipIf(process.platform !== "linux")(
|
||||
"honors an explicit display zero instead of choosing a random display",
|
||||
() => {
|
||||
const result = spawnSync(
|
||||
"/usr/bin/python3",
|
||||
[displaySelectionFixturePath, helperPath],
|
||||
{ encoding: "utf8", timeout: 5_000 },
|
||||
);
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
const explicit = (display: string) => ({
|
||||
authorized: [`:${display}`],
|
||||
checked: [`/tmp/.X11-unix/X${display}`, `/tmp/.X${display}-lock`],
|
||||
random_calls: 0,
|
||||
reason: "xauth_failed",
|
||||
});
|
||||
expect(JSON.parse(result.stdout)).toEqual({
|
||||
explicit_zero: explicit("0"),
|
||||
explicit_zero_random_occupied: explicit("0"),
|
||||
explicit_zero_occupied: {
|
||||
authorized: [],
|
||||
checked: ["/tmp/.X11-unix/X0"],
|
||||
random_calls: 0,
|
||||
reason: "address_collision",
|
||||
},
|
||||
explicit_one: explicit("1"),
|
||||
omitted: {
|
||||
authorized: [":123"],
|
||||
checked: ["/tmp/.X11-unix/X123", "/tmp/.X123-lock"],
|
||||
random_calls: 1,
|
||||
reason: "xauth_failed",
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("Linux private display selection", () => {
|
||||
it("prefers a successful direct Xvfb probe", async () => {
|
||||
const calls: LinuxPrivateDisplayRunnableStrategy[] = [];
|
||||
|
||||
+69
@@ -0,0 +1,69 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Exercise explicit and random display selection with simulated X11 paths."""
|
||||
|
||||
import argparse
|
||||
import importlib.util
|
||||
import json
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import types
|
||||
|
||||
|
||||
spec = importlib.util.spec_from_file_location("hopper_demo_x11", sys.argv[1])
|
||||
assert spec is not None and spec.loader is not None
|
||||
helper = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(helper)
|
||||
|
||||
|
||||
def select(requested, occupied):
|
||||
checked = []
|
||||
authorized = []
|
||||
random_calls = []
|
||||
|
||||
class FakePath:
|
||||
def __init__(self, value):
|
||||
self.value = value
|
||||
|
||||
def exists(self):
|
||||
checked.append(self.value)
|
||||
return self.value in occupied
|
||||
|
||||
def randbelow(_bound):
|
||||
random_calls.append(_bound)
|
||||
return 23
|
||||
|
||||
def authorize_display(_authority, display_name, _xauth):
|
||||
authorized.append(display_name)
|
||||
# Stop before Xvfb so the fixture only observes selection.
|
||||
raise helper.AdapterFailure(
|
||||
helper.PRIVATE_DISPLAY_UNAVAILABLE,
|
||||
"xauth_failed",
|
||||
"fixture stop",
|
||||
)
|
||||
|
||||
helper.Path = FakePath
|
||||
helper.secrets = types.SimpleNamespace(randbelow=randbelow)
|
||||
helper.authorize_display = authorize_display
|
||||
try:
|
||||
helper.start_xvfb(
|
||||
Path("session"),
|
||||
argparse.Namespace(display_number=requested, xauth="xauth", xvfb="Xvfb"),
|
||||
)
|
||||
reason = None
|
||||
except helper.AdapterFailure as failure:
|
||||
reason = failure.reason
|
||||
return {
|
||||
"authorized": authorized,
|
||||
"checked": checked,
|
||||
"random_calls": len(random_calls),
|
||||
"reason": reason,
|
||||
}
|
||||
|
||||
|
||||
print(json.dumps({
|
||||
"explicit_zero": select(0, set()),
|
||||
"explicit_zero_random_occupied": select(0, {"/tmp/.X11-unix/X123"}),
|
||||
"explicit_zero_occupied": select(0, {"/tmp/.X11-unix/X0"}),
|
||||
"explicit_one": select(1, set()),
|
||||
"omitted": select(None, set()),
|
||||
}))
|
||||
Reference in new issue
Block a user