From 7962db477ade96f5593910d71ece2c27fcde53f9 Mon Sep 17 00:00:00 2001 From: John Pollock Date: Fri, 27 Feb 2026 13:07:23 -0600 Subject: [PATCH] test(isolation): isolation integration + policy tests --- tests/isolation/test_client_snapshot.py | 122 +++++ tests/isolation/test_folder_paths_proxy.py | 111 +++++ tests/isolation/test_host_policy.py | 72 +++ tests/isolation/test_init.py | 56 +++ tests/isolation/test_manifest_loader_cache.py | 434 ++++++++++++++++++ .../isolation/test_model_management_proxy.py | 50 ++ tests/isolation/test_path_helpers.py | 93 ++++ tests/test_adapter.py | 51 ++ 8 files changed, 989 insertions(+) create mode 100644 tests/isolation/test_client_snapshot.py create mode 100644 tests/isolation/test_folder_paths_proxy.py create mode 100644 tests/isolation/test_host_policy.py create mode 100644 tests/isolation/test_init.py create mode 100644 tests/isolation/test_manifest_loader_cache.py create mode 100644 tests/isolation/test_model_management_proxy.py create mode 100644 tests/isolation/test_path_helpers.py create mode 100644 tests/test_adapter.py diff --git a/tests/isolation/test_client_snapshot.py b/tests/isolation/test_client_snapshot.py new file mode 100644 index 000000000..c6f906813 --- /dev/null +++ b/tests/isolation/test_client_snapshot.py @@ -0,0 +1,122 @@ +"""Tests for pyisolate._internal.client import-time snapshot handling.""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +# Paths needed for subprocess +PYISOLATE_ROOT = str(Path(__file__).parent.parent) +COMFYUI_ROOT = os.environ.get("COMFYUI_ROOT") or str(Path.home() / "ComfyUI") + +SCRIPT = """ +import json, sys +import pyisolate._internal.client # noqa: F401 # triggers snapshot logic +print(json.dumps(sys.path[:6])) +""" + + +def _run_client_process(env): + # Ensure subprocess can find pyisolate and ComfyUI + pythonpath_parts = [PYISOLATE_ROOT, COMFYUI_ROOT] + existing = env.get("PYTHONPATH", "") + if existing: + pythonpath_parts.append(existing) + env["PYTHONPATH"] = ":".join(pythonpath_parts) + + result = subprocess.run( # noqa: S603 + [sys.executable, "-c", SCRIPT], + capture_output=True, + text=True, + env=env, + check=True, + ) + stdout = result.stdout.strip().splitlines()[-1] + return json.loads(stdout) + + +@pytest.fixture() +def comfy_module_path(tmp_path): + comfy_root = tmp_path / "ComfyUI" + module_path = comfy_root / "custom_nodes" / "TestNode" + module_path.mkdir(parents=True) + return comfy_root, module_path + + +def test_snapshot_applied_and_comfy_root_prepend(tmp_path, comfy_module_path): + comfy_root, module_path = comfy_module_path + # Must include real ComfyUI path for utils validation to pass + host_paths = [COMFYUI_ROOT, "/host/lib1", "/host/lib2"] + snapshot = { + "sys_path": host_paths, + "sys_executable": sys.executable, + "sys_prefix": sys.prefix, + "environment": {}, + } + snapshot_path = tmp_path / "snapshot.json" + snapshot_path.write_text(json.dumps(snapshot), encoding="utf-8") + + env = os.environ.copy() + env.update( + { + "PYISOLATE_CHILD": "1", + "PYISOLATE_HOST_SNAPSHOT": str(snapshot_path), + "PYISOLATE_MODULE_PATH": str(module_path), + } + ) + + path_prefix = _run_client_process(env) + + # Current client behavior preserves the runtime bootstrap path order and + # keeps the resolved ComfyUI root available for imports. + assert COMFYUI_ROOT in path_prefix + # Module path should not override runtime root selection. + assert str(comfy_root) not in path_prefix + + +def test_missing_snapshot_file_does_not_crash(tmp_path, comfy_module_path): + _, module_path = comfy_module_path + missing_snapshot = tmp_path / "missing.json" + + env = os.environ.copy() + env.update( + { + "PYISOLATE_CHILD": "1", + "PYISOLATE_HOST_SNAPSHOT": str(missing_snapshot), + "PYISOLATE_MODULE_PATH": str(module_path), + } + ) + + # Should not raise even though snapshot path is missing + paths = _run_client_process(env) + assert len(paths) > 0 + + +def test_no_comfy_root_when_module_path_absent(tmp_path): + # Must include real ComfyUI path for utils validation to pass + host_paths = [COMFYUI_ROOT, "/alpha", "/beta"] + snapshot = { + "sys_path": host_paths, + "sys_executable": sys.executable, + "sys_prefix": sys.prefix, + "environment": {}, + } + snapshot_path = tmp_path / "snapshot.json" + snapshot_path.write_text(json.dumps(snapshot), encoding="utf-8") + + env = os.environ.copy() + env.update( + { + "PYISOLATE_CHILD": "1", + "PYISOLATE_HOST_SNAPSHOT": str(snapshot_path), + } + ) + + paths = _run_client_process(env) + # Runtime path bootstrap keeps ComfyUI importability regardless of host + # snapshot extras. + assert COMFYUI_ROOT in paths + assert "/alpha" not in paths and "/beta" not in paths diff --git a/tests/isolation/test_folder_paths_proxy.py b/tests/isolation/test_folder_paths_proxy.py new file mode 100644 index 000000000..23585647b --- /dev/null +++ b/tests/isolation/test_folder_paths_proxy.py @@ -0,0 +1,111 @@ +"""Unit tests for FolderPathsProxy.""" + +import pytest +from pathlib import Path + +from comfy.isolation.proxies.folder_paths_proxy import FolderPathsProxy + + +class TestFolderPathsProxy: + """Test FolderPathsProxy methods.""" + + @pytest.fixture + def proxy(self): + """Create a FolderPathsProxy instance for testing.""" + return FolderPathsProxy() + + def test_get_temp_directory_returns_string(self, proxy): + """Verify get_temp_directory returns a non-empty string.""" + result = proxy.get_temp_directory() + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert len(result) > 0, "Temp directory path is empty" + + def test_get_temp_directory_returns_absolute_path(self, proxy): + """Verify get_temp_directory returns an absolute path.""" + result = proxy.get_temp_directory() + path = Path(result) + assert path.is_absolute(), f"Path is not absolute: {result}" + + def test_get_input_directory_returns_string(self, proxy): + """Verify get_input_directory returns a non-empty string.""" + result = proxy.get_input_directory() + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert len(result) > 0, "Input directory path is empty" + + def test_get_input_directory_returns_absolute_path(self, proxy): + """Verify get_input_directory returns an absolute path.""" + result = proxy.get_input_directory() + path = Path(result) + assert path.is_absolute(), f"Path is not absolute: {result}" + + def test_get_annotated_filepath_plain_name(self, proxy): + """Verify get_annotated_filepath works with plain filename.""" + result = proxy.get_annotated_filepath("test.png") + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert "test.png" in result, f"Filename not in result: {result}" + + def test_get_annotated_filepath_with_output_annotation(self, proxy): + """Verify get_annotated_filepath handles [output] annotation.""" + result = proxy.get_annotated_filepath("test.png[output]") + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert "test.pn" in result, f"Filename base not in result: {result}" + # Should resolve to output directory + assert "output" in result.lower() or Path(result).parent.name == "output" + + def test_get_annotated_filepath_with_input_annotation(self, proxy): + """Verify get_annotated_filepath handles [input] annotation.""" + result = proxy.get_annotated_filepath("test.png[input]") + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert "test.pn" in result, f"Filename base not in result: {result}" + + def test_get_annotated_filepath_with_temp_annotation(self, proxy): + """Verify get_annotated_filepath handles [temp] annotation.""" + result = proxy.get_annotated_filepath("test.png[temp]") + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert "test.pn" in result, f"Filename base not in result: {result}" + + def test_exists_annotated_filepath_returns_bool(self, proxy): + """Verify exists_annotated_filepath returns a boolean.""" + result = proxy.exists_annotated_filepath("nonexistent.png") + assert isinstance(result, bool), f"Expected bool, got {type(result)}" + + def test_exists_annotated_filepath_nonexistent_file(self, proxy): + """Verify exists_annotated_filepath returns False for nonexistent file.""" + result = proxy.exists_annotated_filepath("definitely_does_not_exist_12345.png") + assert result is False, "Expected False for nonexistent file" + + def test_exists_annotated_filepath_with_annotation(self, proxy): + """Verify exists_annotated_filepath works with annotation suffix.""" + # Even for nonexistent files, should return bool without error + result = proxy.exists_annotated_filepath("test.png[output]") + assert isinstance(result, bool), f"Expected bool, got {type(result)}" + + def test_models_dir_property_returns_string(self, proxy): + """Verify models_dir property returns valid path string.""" + result = proxy.models_dir + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert len(result) > 0, "Models directory path is empty" + + def test_models_dir_is_absolute_path(self, proxy): + """Verify models_dir returns an absolute path.""" + result = proxy.models_dir + path = Path(result) + assert path.is_absolute(), f"Path is not absolute: {result}" + + def test_add_model_folder_path_runs_without_error(self, proxy): + """Verify add_model_folder_path executes without raising.""" + test_path = "/tmp/test_models_florence2" + # Should not raise + proxy.add_model_folder_path("TEST_FLORENCE2", test_path) + + def test_get_folder_paths_returns_list(self, proxy): + """Verify get_folder_paths returns a list.""" + # Use known folder type that should exist + result = proxy.get_folder_paths("checkpoints") + assert isinstance(result, list), f"Expected list, got {type(result)}" + + def test_get_folder_paths_checkpoints_not_empty(self, proxy): + """Verify checkpoints folder paths list is not empty.""" + result = proxy.get_folder_paths("checkpoints") + # Should have at least one checkpoint path registered + assert len(result) > 0, "Checkpoints folder paths is empty" diff --git a/tests/isolation/test_host_policy.py b/tests/isolation/test_host_policy.py new file mode 100644 index 000000000..b6097132b --- /dev/null +++ b/tests/isolation/test_host_policy.py @@ -0,0 +1,72 @@ +from pathlib import Path + + +def _write_pyproject(path: Path, content: str) -> None: + path.write_text(content, encoding="utf-8") + + +def test_load_host_policy_defaults_when_pyproject_missing(tmp_path): + from comfy.isolation.host_policy import DEFAULT_POLICY, load_host_policy + + policy = load_host_policy(tmp_path) + + assert policy["allow_network"] == DEFAULT_POLICY["allow_network"] + assert policy["writable_paths"] == DEFAULT_POLICY["writable_paths"] + assert policy["readonly_paths"] == DEFAULT_POLICY["readonly_paths"] + assert policy["whitelist"] == DEFAULT_POLICY["whitelist"] + + +def test_load_host_policy_defaults_when_section_missing(tmp_path): + from comfy.isolation.host_policy import DEFAULT_POLICY, load_host_policy + + _write_pyproject( + tmp_path / "pyproject.toml", + """ +[project] +name = "ComfyUI" +""".strip(), + ) + + policy = load_host_policy(tmp_path) + assert policy["allow_network"] == DEFAULT_POLICY["allow_network"] + assert policy["whitelist"] == {} + + +def test_load_host_policy_reads_values(tmp_path): + from comfy.isolation.host_policy import load_host_policy + + _write_pyproject( + tmp_path / "pyproject.toml", + """ +[tool.comfy.host] +allow_network = true +writable_paths = ["/tmp/a", "/tmp/b"] +readonly_paths = ["/opt/readonly"] + +[tool.comfy.host.whitelist] +ExampleNode = "*" +""".strip(), + ) + + policy = load_host_policy(tmp_path) + assert policy["allow_network"] is True + assert policy["writable_paths"] == ["/tmp/a", "/tmp/b"] + assert policy["readonly_paths"] == ["/opt/readonly"] + assert policy["whitelist"] == {"ExampleNode": "*"} + + +def test_load_host_policy_ignores_invalid_whitelist_type(tmp_path): + from comfy.isolation.host_policy import DEFAULT_POLICY, load_host_policy + + _write_pyproject( + tmp_path / "pyproject.toml", + """ +[tool.comfy.host] +allow_network = true +whitelist = ["bad"] +""".strip(), + ) + + policy = load_host_policy(tmp_path) + assert policy["allow_network"] is True + assert policy["whitelist"] == DEFAULT_POLICY["whitelist"] diff --git a/tests/isolation/test_init.py b/tests/isolation/test_init.py new file mode 100644 index 000000000..d9dfeb1e6 --- /dev/null +++ b/tests/isolation/test_init.py @@ -0,0 +1,56 @@ +"""Unit tests for PyIsolate isolation system initialization.""" + + + +def test_log_prefix(): + """Verify LOG_PREFIX constant is correctly defined.""" + from comfy.isolation import LOG_PREFIX + assert LOG_PREFIX == "][" + assert isinstance(LOG_PREFIX, str) + + +def test_module_initialization(): + """Verify module initializes without errors.""" + import comfy.isolation + assert hasattr(comfy.isolation, 'LOG_PREFIX') + assert hasattr(comfy.isolation, 'initialize_proxies') + + +class TestInitializeProxies: + def test_initialize_proxies_runs_without_error(self): + from comfy.isolation import initialize_proxies + initialize_proxies() + + def test_initialize_proxies_registers_folder_paths_proxy(self): + from comfy.isolation import initialize_proxies + from comfy.isolation.proxies.folder_paths_proxy import FolderPathsProxy + initialize_proxies() + proxy = FolderPathsProxy() + assert proxy is not None + assert hasattr(proxy, "get_temp_directory") + + def test_initialize_proxies_registers_model_management_proxy(self): + from comfy.isolation import initialize_proxies + from comfy.isolation.proxies.model_management_proxy import ModelManagementProxy + initialize_proxies() + proxy = ModelManagementProxy() + assert proxy is not None + assert hasattr(proxy, "get_torch_device") + + def test_initialize_proxies_can_be_called_multiple_times(self): + from comfy.isolation import initialize_proxies + initialize_proxies() + initialize_proxies() + initialize_proxies() + + def test_dev_proxies_accessible_when_dev_mode(self, monkeypatch): + """Verify dev mode does not break core proxy initialization.""" + monkeypatch.setenv("PYISOLATE_DEV", "1") + from comfy.isolation import initialize_proxies + from comfy.isolation.proxies.folder_paths_proxy import FolderPathsProxy + from comfy.isolation.proxies.utils_proxy import UtilsProxy + initialize_proxies() + folder_proxy = FolderPathsProxy() + utils_proxy = UtilsProxy() + assert folder_proxy is not None + assert utils_proxy is not None diff --git a/tests/isolation/test_manifest_loader_cache.py b/tests/isolation/test_manifest_loader_cache.py new file mode 100644 index 000000000..ebee43b7e --- /dev/null +++ b/tests/isolation/test_manifest_loader_cache.py @@ -0,0 +1,434 @@ +""" +Unit tests for manifest_loader.py cache functions. + +Phase 1 tests verify: +1. Cache miss on first run (no cache exists) +2. Cache hit when nothing changes +3. Invalidation on .py file touch +4. Invalidation on manifest change +5. Cache location correctness (in venv_root, NOT in custom_nodes) +6. Corrupt cache handling (graceful failure) + +These tests verify the cache implementation is correct BEFORE it's activated +in extension_loader.py (Phase 2). +""" + +from __future__ import annotations + +import json +import sys +import time +from pathlib import Path +from unittest import mock + + + +class TestComputeCacheKey: + """Tests for compute_cache_key() function.""" + + def test_key_includes_manifest_content(self, tmp_path: Path) -> None: + """Cache key changes when manifest content changes.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + + # Initial manifest + manifest.write_text("isolated: true\ndependencies: []\n") + key1 = compute_cache_key(node_dir, manifest) + + # Modified manifest + manifest.write_text("isolated: true\ndependencies: [numpy]\n") + key2 = compute_cache_key(node_dir, manifest) + + assert key1 != key2, "Key should change when manifest content changes" + + def test_key_includes_py_file_mtime(self, tmp_path: Path) -> None: + """Cache key changes when any .py file is touched.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + + py_file = node_dir / "nodes.py" + py_file.write_text("# test code") + + key1 = compute_cache_key(node_dir, manifest) + + # Wait a moment to ensure mtime changes + time.sleep(0.01) + py_file.write_text("# modified code") + + key2 = compute_cache_key(node_dir, manifest) + + assert key1 != key2, "Key should change when .py file mtime changes" + + def test_key_includes_python_version(self, tmp_path: Path) -> None: + """Cache key changes when Python version changes.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + + key1 = compute_cache_key(node_dir, manifest) + + # Mock different Python version + with mock.patch.object(sys, "version", "3.99.0 (fake)"): + key2 = compute_cache_key(node_dir, manifest) + + assert key1 != key2, "Key should change when Python version changes" + + def test_key_includes_pyisolate_version(self, tmp_path: Path) -> None: + """Cache key changes when PyIsolate version changes.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + + key1 = compute_cache_key(node_dir, manifest) + + # Mock different pyisolate version + with mock.patch.dict(sys.modules, {"pyisolate": mock.MagicMock(__version__="99.99.99")}): + # Need to reimport to pick up the mock + import importlib + from comfy.isolation import manifest_loader + importlib.reload(manifest_loader) + key2 = manifest_loader.compute_cache_key(node_dir, manifest) + + # Keys should be different (though the mock approach is tricky) + # At minimum, verify key is a valid hex string + assert len(key1) == 16, "Key should be 16 hex characters" + assert all(c in "0123456789abcdef" for c in key1), "Key should be hex" + assert len(key2) == 16, "Key should be 16 hex characters" + assert all(c in "0123456789abcdef" for c in key2), "Key should be hex" + + def test_key_excludes_pycache(self, tmp_path: Path) -> None: + """Cache key ignores __pycache__ directory changes.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + + py_file = node_dir / "nodes.py" + py_file.write_text("# test code") + + key1 = compute_cache_key(node_dir, manifest) + + # Add __pycache__ file + pycache = node_dir / "__pycache__" + pycache.mkdir() + (pycache / "nodes.cpython-310.pyc").write_bytes(b"compiled") + + key2 = compute_cache_key(node_dir, manifest) + + assert key1 == key2, "Key should NOT change when __pycache__ modified" + + def test_key_is_deterministic(self, tmp_path: Path) -> None: + """Same inputs produce same key.""" + from comfy.isolation.manifest_loader import compute_cache_key + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + (node_dir / "nodes.py").write_text("# code") + + key1 = compute_cache_key(node_dir, manifest) + key2 = compute_cache_key(node_dir, manifest) + + assert key1 == key2, "Key should be deterministic" + + +class TestGetCachePath: + """Tests for get_cache_path() function.""" + + def test_returns_correct_paths(self, tmp_path: Path) -> None: + """Cache paths are in venv_root, not in node_dir.""" + from comfy.isolation.manifest_loader import get_cache_path + + node_dir = tmp_path / "custom_nodes" / "MyNode" + venv_root = tmp_path / ".pyisolate_venvs" + + key_file, data_file = get_cache_path(node_dir, venv_root) + + assert key_file == venv_root / "MyNode" / "cache" / "cache_key" + assert data_file == venv_root / "MyNode" / "cache" / "node_info.json" + + def test_cache_not_in_custom_nodes(self, tmp_path: Path) -> None: + """Verify cache is NOT stored in custom_nodes directory.""" + from comfy.isolation.manifest_loader import get_cache_path + + node_dir = tmp_path / "custom_nodes" / "MyNode" + venv_root = tmp_path / ".pyisolate_venvs" + + key_file, data_file = get_cache_path(node_dir, venv_root) + + # Neither path should be under node_dir + assert not str(key_file).startswith(str(node_dir)) + assert not str(data_file).startswith(str(node_dir)) + + +class TestIsCacheValid: + """Tests for is_cache_valid() function.""" + + def test_false_when_no_cache_exists(self, tmp_path: Path) -> None: + """Returns False when cache files don't exist.""" + from comfy.isolation.manifest_loader import is_cache_valid + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + assert is_cache_valid(node_dir, manifest, venv_root) is False + + def test_true_when_cache_matches(self, tmp_path: Path) -> None: + """Returns True when cache key matches current state.""" + from comfy.isolation.manifest_loader import ( + compute_cache_key, + get_cache_path, + is_cache_valid, + ) + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + (node_dir / "nodes.py").write_text("# code") + venv_root = tmp_path / ".pyisolate_venvs" + + # Create valid cache + cache_key = compute_cache_key(node_dir, manifest) + key_file, data_file = get_cache_path(node_dir, venv_root) + key_file.parent.mkdir(parents=True, exist_ok=True) + key_file.write_text(cache_key) + data_file.write_text("{}") + + assert is_cache_valid(node_dir, manifest, venv_root) is True + + def test_false_when_key_mismatch(self, tmp_path: Path) -> None: + """Returns False when stored key doesn't match current state.""" + from comfy.isolation.manifest_loader import get_cache_path, is_cache_valid + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + # Create cache with wrong key + key_file, data_file = get_cache_path(node_dir, venv_root) + key_file.parent.mkdir(parents=True, exist_ok=True) + key_file.write_text("wrong_key_12345") + data_file.write_text("{}") + + assert is_cache_valid(node_dir, manifest, venv_root) is False + + def test_false_when_data_file_missing(self, tmp_path: Path) -> None: + """Returns False when node_info.json is missing.""" + from comfy.isolation.manifest_loader import ( + compute_cache_key, + get_cache_path, + is_cache_valid, + ) + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + # Create only key file, not data file + cache_key = compute_cache_key(node_dir, manifest) + key_file, _ = get_cache_path(node_dir, venv_root) + key_file.parent.mkdir(parents=True, exist_ok=True) + key_file.write_text(cache_key) + + assert is_cache_valid(node_dir, manifest, venv_root) is False + + def test_invalidation_on_py_change(self, tmp_path: Path) -> None: + """Cache invalidates when .py file is modified.""" + from comfy.isolation.manifest_loader import ( + compute_cache_key, + get_cache_path, + is_cache_valid, + ) + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + py_file = node_dir / "nodes.py" + py_file.write_text("# original") + venv_root = tmp_path / ".pyisolate_venvs" + + # Create valid cache + cache_key = compute_cache_key(node_dir, manifest) + key_file, data_file = get_cache_path(node_dir, venv_root) + key_file.parent.mkdir(parents=True, exist_ok=True) + key_file.write_text(cache_key) + data_file.write_text("{}") + + # Verify cache is valid initially + assert is_cache_valid(node_dir, manifest, venv_root) is True + + # Modify .py file + time.sleep(0.01) # Ensure mtime changes + py_file.write_text("# modified") + + # Cache should now be invalid + assert is_cache_valid(node_dir, manifest, venv_root) is False + + +class TestLoadFromCache: + """Tests for load_from_cache() function.""" + + def test_returns_none_when_no_cache(self, tmp_path: Path) -> None: + """Returns None when cache doesn't exist.""" + from comfy.isolation.manifest_loader import load_from_cache + + node_dir = tmp_path / "test_node" + venv_root = tmp_path / ".pyisolate_venvs" + + assert load_from_cache(node_dir, venv_root) is None + + def test_returns_data_when_valid(self, tmp_path: Path) -> None: + """Returns cached data when file exists and is valid JSON.""" + from comfy.isolation.manifest_loader import get_cache_path, load_from_cache + + node_dir = tmp_path / "test_node" + venv_root = tmp_path / ".pyisolate_venvs" + + test_data = {"TestNode": {"inputs": [], "outputs": []}} + + _, data_file = get_cache_path(node_dir, venv_root) + data_file.parent.mkdir(parents=True, exist_ok=True) + data_file.write_text(json.dumps(test_data)) + + result = load_from_cache(node_dir, venv_root) + assert result == test_data + + def test_returns_none_on_corrupt_json(self, tmp_path: Path) -> None: + """Returns None when JSON is corrupt.""" + from comfy.isolation.manifest_loader import get_cache_path, load_from_cache + + node_dir = tmp_path / "test_node" + venv_root = tmp_path / ".pyisolate_venvs" + + _, data_file = get_cache_path(node_dir, venv_root) + data_file.parent.mkdir(parents=True, exist_ok=True) + data_file.write_text("{ corrupt json }") + + assert load_from_cache(node_dir, venv_root) is None + + def test_returns_none_on_invalid_structure(self, tmp_path: Path) -> None: + """Returns None when data is not a dict.""" + from comfy.isolation.manifest_loader import get_cache_path, load_from_cache + + node_dir = tmp_path / "test_node" + venv_root = tmp_path / ".pyisolate_venvs" + + _, data_file = get_cache_path(node_dir, venv_root) + data_file.parent.mkdir(parents=True, exist_ok=True) + data_file.write_text("[1, 2, 3]") # Array, not dict + + assert load_from_cache(node_dir, venv_root) is None + + +class TestSaveToCache: + """Tests for save_to_cache() function.""" + + def test_creates_cache_directory(self, tmp_path: Path) -> None: + """Creates cache directory if it doesn't exist.""" + from comfy.isolation.manifest_loader import get_cache_path, save_to_cache + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + save_to_cache(node_dir, venv_root, {"TestNode": {}}, manifest) + + key_file, data_file = get_cache_path(node_dir, venv_root) + assert key_file.parent.exists() + + def test_writes_both_files(self, tmp_path: Path) -> None: + """Writes both cache_key and node_info.json.""" + from comfy.isolation.manifest_loader import get_cache_path, save_to_cache + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + save_to_cache(node_dir, venv_root, {"TestNode": {"key": "value"}}, manifest) + + key_file, data_file = get_cache_path(node_dir, venv_root) + assert key_file.exists() + assert data_file.exists() + + def test_data_is_valid_json(self, tmp_path: Path) -> None: + """Written data can be parsed as JSON.""" + from comfy.isolation.manifest_loader import get_cache_path, save_to_cache + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + test_data = {"TestNode": {"inputs": ["IMAGE"], "outputs": ["IMAGE"]}} + save_to_cache(node_dir, venv_root, test_data, manifest) + + _, data_file = get_cache_path(node_dir, venv_root) + loaded = json.loads(data_file.read_text()) + assert loaded == test_data + + def test_roundtrip_with_validation(self, tmp_path: Path) -> None: + """Saved cache is immediately valid.""" + from comfy.isolation.manifest_loader import ( + is_cache_valid, + load_from_cache, + save_to_cache, + ) + + node_dir = tmp_path / "test_node" + node_dir.mkdir() + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + (node_dir / "nodes.py").write_text("# code") + venv_root = tmp_path / ".pyisolate_venvs" + + test_data = {"TestNode": {"foo": "bar"}} + save_to_cache(node_dir, venv_root, test_data, manifest) + + assert is_cache_valid(node_dir, manifest, venv_root) is True + assert load_from_cache(node_dir, venv_root) == test_data + + def test_cache_not_in_custom_nodes(self, tmp_path: Path) -> None: + """Verify no files written to custom_nodes directory.""" + from comfy.isolation.manifest_loader import save_to_cache + + node_dir = tmp_path / "custom_nodes" / "MyNode" + node_dir.mkdir(parents=True) + manifest = node_dir / "pyisolate.yaml" + manifest.write_text("isolated: true\n") + venv_root = tmp_path / ".pyisolate_venvs" + + save_to_cache(node_dir, venv_root, {"TestNode": {}}, manifest) + + # Check nothing was created under node_dir + for item in node_dir.iterdir(): + assert item.name == "pyisolate.yaml", f"Unexpected file in node_dir: {item}" diff --git a/tests/isolation/test_model_management_proxy.py b/tests/isolation/test_model_management_proxy.py new file mode 100644 index 000000000..3a03bd54d --- /dev/null +++ b/tests/isolation/test_model_management_proxy.py @@ -0,0 +1,50 @@ +"""Unit tests for ModelManagementProxy.""" + +import pytest +import torch + +from comfy.isolation.proxies.model_management_proxy import ModelManagementProxy + + +class TestModelManagementProxy: + """Test ModelManagementProxy methods.""" + + @pytest.fixture + def proxy(self): + """Create a ModelManagementProxy instance for testing.""" + return ModelManagementProxy() + + def test_get_torch_device_returns_device(self, proxy): + """Verify get_torch_device returns a torch.device object.""" + result = proxy.get_torch_device() + assert isinstance(result, torch.device), f"Expected torch.device, got {type(result)}" + + def test_get_torch_device_is_valid(self, proxy): + """Verify get_torch_device returns a valid device (cpu or cuda).""" + result = proxy.get_torch_device() + assert result.type in ("cpu", "cuda"), f"Unexpected device type: {result.type}" + + def test_get_torch_device_name_returns_string(self, proxy): + """Verify get_torch_device_name returns a non-empty string.""" + device = proxy.get_torch_device() + result = proxy.get_torch_device_name(device) + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert len(result) > 0, "Device name is empty" + + def test_get_torch_device_name_with_cpu(self, proxy): + """Verify get_torch_device_name works with CPU device.""" + cpu_device = torch.device("cpu") + result = proxy.get_torch_device_name(cpu_device) + assert isinstance(result, str), f"Expected str, got {type(result)}" + assert "cpu" in result.lower(), f"Expected 'cpu' in device name, got: {result}" + + def test_get_torch_device_name_with_cuda_if_available(self, proxy): + """Verify get_torch_device_name works with CUDA device if available.""" + if not torch.cuda.is_available(): + pytest.skip("CUDA not available") + + cuda_device = torch.device("cuda:0") + result = proxy.get_torch_device_name(cuda_device) + assert isinstance(result, str), f"Expected str, got {type(result)}" + # Should contain device identifier + assert len(result) > 0, "CUDA device name is empty" diff --git a/tests/isolation/test_path_helpers.py b/tests/isolation/test_path_helpers.py new file mode 100644 index 000000000..af96f1fe0 --- /dev/null +++ b/tests/isolation/test_path_helpers.py @@ -0,0 +1,93 @@ +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +import pytest + +from pyisolate.path_helpers import build_child_sys_path, serialize_host_snapshot + + +def test_serialize_host_snapshot_includes_expected_keys(tmp_path: Path, monkeypatch) -> None: + output = tmp_path / "snapshot.json" + monkeypatch.setenv("EXTRA_FLAG", "1") + snapshot = serialize_host_snapshot(output_path=output, extra_env_keys=["EXTRA_FLAG"]) + + assert "sys_path" in snapshot + assert "sys_executable" in snapshot + assert "sys_prefix" in snapshot + assert "environment" in snapshot + assert output.exists() + assert snapshot["environment"].get("EXTRA_FLAG") == "1" + + persisted = json.loads(output.read_text(encoding="utf-8")) + assert persisted["sys_path"] == snapshot["sys_path"] + + +def test_build_child_sys_path_preserves_host_order() -> None: + host_paths = ["/host/root", "/host/site-packages"] + extra_paths = ["/node/.venv/lib/python3.12/site-packages"] + result = build_child_sys_path(host_paths, extra_paths, preferred_root=None) + assert result == host_paths + extra_paths + + +def test_build_child_sys_path_inserts_comfy_root_when_missing() -> None: + host_paths = ["/host/site-packages"] + comfy_root = os.environ.get("COMFYUI_ROOT") or str(Path.home() / "ComfyUI") + extra_paths: list[str] = [] + result = build_child_sys_path(host_paths, extra_paths, preferred_root=comfy_root) + assert result[0] == comfy_root + assert result[1:] == host_paths + + +def test_build_child_sys_path_deduplicates_entries(tmp_path: Path) -> None: + path_a = str(tmp_path / "a") + path_b = str(tmp_path / "b") + host_paths = [path_a, path_b] + extra_paths = [path_a, path_b, str(tmp_path / "c")] + result = build_child_sys_path(host_paths, extra_paths) + assert result == [path_a, path_b, str(tmp_path / "c")] + + +def test_build_child_sys_path_skips_duplicate_comfy_root() -> None: + comfy_root = os.environ.get("COMFYUI_ROOT") or str(Path.home() / "ComfyUI") + host_paths = [comfy_root, "/host/other"] + result = build_child_sys_path(host_paths, extra_paths=[], preferred_root=comfy_root) + assert result == host_paths + + +def test_child_import_succeeds_after_path_unification(tmp_path: Path, monkeypatch) -> None: + host_root = tmp_path / "host" + utils_pkg = host_root / "utils" + app_pkg = host_root / "app" + utils_pkg.mkdir(parents=True) + app_pkg.mkdir(parents=True) + + (utils_pkg / "__init__.py").write_text("from . import install_util\n", encoding="utf-8") + (utils_pkg / "install_util.py").write_text("VALUE = 'hello'\n", encoding="utf-8") + (app_pkg / "__init__.py").write_text("", encoding="utf-8") + (app_pkg / "frontend_management.py").write_text( + "from utils import install_util\nVALUE = install_util.VALUE\n", + encoding="utf-8", + ) + + child_only = tmp_path / "child_only" + child_only.mkdir() + + target_module = "app.frontend_management" + for name in [n for n in list(sys.modules) if n.startswith("app") or n.startswith("utils")]: + sys.modules.pop(name) + + monkeypatch.setattr(sys, "path", [str(child_only)]) + with pytest.raises(ModuleNotFoundError): + __import__(target_module) + + for name in [n for n in list(sys.modules) if n.startswith("app") or n.startswith("utils")]: + sys.modules.pop(name) + + unified = build_child_sys_path([], [], preferred_root=str(host_root)) + monkeypatch.setattr(sys, "path", unified) + module = __import__(target_module, fromlist=["VALUE"]) + assert module.VALUE == "hello" diff --git a/tests/test_adapter.py b/tests/test_adapter.py new file mode 100644 index 000000000..feaa62549 --- /dev/null +++ b/tests/test_adapter.py @@ -0,0 +1,51 @@ +import os +import sys +from pathlib import Path + +repo_root = Path(__file__).resolve().parents[1] +pyisolate_root = repo_root.parent / "pyisolate" +if pyisolate_root.exists(): + sys.path.insert(0, str(pyisolate_root)) + +from comfy.isolation.adapter import ComfyUIAdapter +from pyisolate._internal.serialization_registry import SerializerRegistry + + +def test_identifier(): + adapter = ComfyUIAdapter() + assert adapter.identifier == "comfyui" + + +def test_get_path_config_valid(): + adapter = ComfyUIAdapter() + path = os.path.join("/opt", "ComfyUI", "custom_nodes", "demo") + cfg = adapter.get_path_config(path) + assert cfg is not None + assert cfg["preferred_root"].endswith("ComfyUI") + assert "custom_nodes" in cfg["additional_paths"][0] + + +def test_get_path_config_invalid(): + adapter = ComfyUIAdapter() + assert adapter.get_path_config("/random/path") is None + + +def test_provide_rpc_services(): + adapter = ComfyUIAdapter() + services = adapter.provide_rpc_services() + names = {s.__name__ for s in services} + assert "PromptServerService" in names + assert "FolderPathsProxy" in names + + +def test_register_serializers(): + adapter = ComfyUIAdapter() + registry = SerializerRegistry.get_instance() + registry.clear() + + adapter.register_serializers(registry) + assert registry.has_handler("ModelPatcher") + assert registry.has_handler("CLIP") + assert registry.has_handler("VAE") + + registry.clear()