On Windows, os.access(path, os.X_OK) always returns True for any existing file (NTFS has no execute bit; executability is governed by file extension, not mode bits). So the test_non_executable_engine test — which chmods the engine to 0o644 and expects 'fail' — could never pass on Windows. Fixed doctor.py to use a platform-aware check: on Windows, any existing file is executable; on POSIX, honor the mode bits via os.access(X_OK). Fixed the test to assert the correct per-platform expectation: 'pass' on Windows (chmod is a no-op for executability), 'fail' on POSIX. Co-authored-by: woolcoxm <13604288+woolcoxm@users.noreply.github.com>
This commit is contained in:
+11
-1
@@ -2,6 +2,7 @@
|
||||
"""Read-only installation diagnostics for colibri."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import json
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
@@ -63,7 +64,16 @@ def run_doctor(model, ram_gb=0, context=4096, gpu_indices=None, vram_gb=0, *,
|
||||
checks.append(_check("storage.persistence", "skip", "persistence requires a model directory"))
|
||||
|
||||
engine = Path(engine_path)
|
||||
if engine.is_file() and os.access(engine, os.X_OK):
|
||||
# On Windows, os.access(X_OK) always returns True for any existing file
|
||||
# (NTFS has no execute bit; executability is governed by file extension).
|
||||
# So a chmod(0o644) "non-executable" scenario can't be detected via X_OK
|
||||
# on Windows. Use a platform-aware check: on POSIX, honor the mode bits;
|
||||
# on Windows, any existing file is treated as executable. (#141)
|
||||
if sys.platform == "win32":
|
||||
engine_ok = engine.is_file()
|
||||
else:
|
||||
engine_ok = engine.is_file() and os.access(engine, os.X_OK)
|
||||
if engine_ok:
|
||||
checks.append(_check("engine.binary", "pass", "engine executable is ready", path=str(engine)))
|
||||
elif engine.is_file():
|
||||
checks.append(_check("engine.binary", "fail", "engine exists but is not executable", path=str(engine)))
|
||||
|
||||
@@ -104,7 +104,14 @@ class DoctorTest(unittest.TestCase):
|
||||
report = self.report(ram_gb=40)
|
||||
checks = self.checks_by_id(report)
|
||||
|
||||
self.assertEqual(checks["engine.binary"]["status"], "fail")
|
||||
# On Windows chmod(0o644) does not remove executability (NTFS has no
|
||||
# execute bit; os.access(X_OK) is always True for existing files), so
|
||||
# the engine.binary check stays "pass" there. The excessive-RAM check
|
||||
# (memory.ram) is platform-independent and must still fail. (#141)
|
||||
if sys.platform == "win32":
|
||||
self.assertEqual(checks["engine.binary"]["status"], "pass")
|
||||
else:
|
||||
self.assertEqual(checks["engine.binary"]["status"], "fail")
|
||||
self.assertEqual(checks["memory.ram"]["status"], "fail")
|
||||
self.assertEqual(report["status"], "error")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user