From f7df03f4e0dcf71ebfb4e0c3d09dd99b791c735f Mon Sep 17 00:00:00 2001 From: "Can H. Tartanoglu" Date: Mon, 4 May 2026 16:09:17 +0200 Subject: [PATCH] test: add unit tests for dependency_management flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lift the on/off value parser to `manager_util.is_off_value` so the prestartup hook and the new tests share one definition, and cover the flag's contract: - default is enabled (today's behavior is preserved bit-for-bit) - PIPFixer.fix_broken short-circuits without spawning subprocess when the flag is off - PIPFixer.fix_broken still reaches subprocess when the flag is on - make_pip_cmd is not gated — read paths (`pip list`, `freeze`, `show`) the UI and security check rely on continue to work - value parser accepts off/false/0/no/disabled (case- and whitespace-insensitive) and treats unknown values as "on" so a typo doesn't silently disable installs --- comfyui_manager/common/manager_util.py | 13 +++ comfyui_manager/prestartup_script.py | 7 +- .../common/test_dependency_management_flag.py | 109 ++++++++++++++++++ 3 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 tests/common/test_dependency_management_flag.py diff --git a/comfyui_manager/common/manager_util.py b/comfyui_manager/common/manager_util.py index 1b38b890..f56a3400 100644 --- a/comfyui_manager/common/manager_util.py +++ b/comfyui_manager/common/manager_util.py @@ -39,6 +39,19 @@ bypass_ssl = False # inventory, and security checks keep working. dependency_management_enabled = True + +_OFF_VALUES = frozenset({'off', 'false', '0', 'no', 'disabled'}) + + +def is_off_value(value): + """Return True if a config or env-var string spells "off". + + Anything else — including the empty string and unknown words — is treated + as "on", so a typo doesn't silently disable dependency management. + """ + return value.strip().lower() in _OFF_VALUES + + def is_manager_pip_package(): return not os.path.exists(os.path.join(comfyui_manager_path, '..', 'custom_nodes')) diff --git a/comfyui_manager/prestartup_script.py b/comfyui_manager/prestartup_script.py index 1a2ff6b6..56cfc334 100644 --- a/comfyui_manager/prestartup_script.py +++ b/comfyui_manager/prestartup_script.py @@ -101,14 +101,11 @@ def read_dependency_management_mode(): # startup (PIPFixer auto-heal, unified resolver, per-node pip install). # Intended for distro-packaged / externally-managed Python environments # where the interpreter is read-only (Nix, Guix, system packages, etc.). - def _is_off(value): - return value.strip().lower() in ('off', 'false', '0', 'no', 'disabled') - env_value = os.environ.get('COMFYUI_MANAGER_DEPENDENCY_MANAGEMENT') if env_value is not None: - manager_util.dependency_management_enabled = not _is_off(env_value) + manager_util.dependency_management_enabled = not manager_util.is_off_value(env_value) elif 'dependency_management' in default_conf: - manager_util.dependency_management_enabled = not _is_off(default_conf['dependency_management']) + manager_util.dependency_management_enabled = not manager_util.is_off_value(default_conf['dependency_management']) if not manager_util.dependency_management_enabled: logging.info("[ComfyUI-Manager] dependency management disabled; auto-install/upgrade/uninstall paths will be skipped.") diff --git a/tests/common/test_dependency_management_flag.py b/tests/common/test_dependency_management_flag.py new file mode 100644 index 00000000..f0c97902 --- /dev/null +++ b/tests/common/test_dependency_management_flag.py @@ -0,0 +1,109 @@ +""" +Tests for the `dependency_management` opt-out switch. + +Covers the module-level flag, the value parser shared with prestartup, and +the early-return in `PIPFixer.fix_broken()`. The prestartup gates (unified +resolver, lazy install) are exercised end-to-end; the unit tests here lock +in the read paths and the short-circuit semantics. +""" + +import subprocess +import sys +import types +import unittest +from unittest import mock + + +def _import_manager_util(): + # `comfyui_manager/__init__.py` imports `comfy.cli_args` at module load + # time; stub the host-side modules so the unit under test can be imported + # without a full ComfyUI runtime. + for name in ("comfy", "comfy.cli_args"): + sys.modules.setdefault(name, types.ModuleType(name)) + sys.modules["comfy.cli_args"].args = types.SimpleNamespace(base_directory=None) + + from comfyui_manager.common import manager_util # noqa: WPS433 + return manager_util + + +class DependencyManagementFlagTest(unittest.TestCase): + def setUp(self): + self.manager_util = _import_manager_util() + self._prev = self.manager_util.dependency_management_enabled + self.addCleanup(setattr, self.manager_util, "dependency_management_enabled", self._prev) + + def test_default_is_enabled(self): + # Default must preserve today's behavior. + self.assertTrue(self._prev) + + def test_pip_fixer_short_circuits_when_disabled(self): + self.manager_util.dependency_management_enabled = False + + class _Stub(self.manager_util.PIPFixer): + def __init__(self): # bypass parent init + pass + + with mock.patch.object(subprocess, "check_output") as patched: + _Stub().fix_broken() + + patched.assert_not_called() + + def test_pip_fixer_runs_when_enabled(self): + # Enabled path should at least call subprocess (via get_installed_packages). + self.manager_util.dependency_management_enabled = True + + class _Stub(self.manager_util.PIPFixer): + def __init__(self): + self.prev_pip_versions = {} + self.comfyui_path = "/nonexistent" + self.manager_files_path = "/nonexistent" + + with mock.patch.object(subprocess, "check_output", return_value="Package Version\n------- -------\n") as patched: + try: + _Stub().fix_broken() + except Exception: + # Other internal paths (opencv, frontend) may raise on a stub + # tree — we only care that subprocess was reached at least once. + pass + + self.assertGreaterEqual(patched.call_count, 1) + + def test_make_pip_cmd_not_gated(self): + # make_pip_cmd is consulted for both read and write ops (pip list, + # pip install, …). Gating it would break read paths the UI relies on. + # Assert the helper still produces a runnable command when the flag + # is off — gating happens at call sites that perform writes. + self.manager_util.dependency_management_enabled = False + + cmd = self.manager_util.make_pip_cmd(["list"]) + self.assertIsInstance(cmd, list) + self.assertGreaterEqual(len(cmd), 3) + self.assertIn("list", cmd) + + cmd = self.manager_util.make_pip_cmd(["install", "some-pkg"]) + self.assertIn("install", cmd) + self.assertIn("some-pkg", cmd) + + +class IsOffValueTest(unittest.TestCase): + """`is_off_value` is the single source of truth for the config/env vocabulary.""" + + def setUp(self): + self.manager_util = _import_manager_util() + + def test_off_synonyms(self): + for value in ("off", "OFF", " false ", "0", "no", "Disabled"): + with self.subTest(value=value): + self.assertTrue(self.manager_util.is_off_value(value)) + + def test_on_values(self): + # Anything not in the off vocabulary — including the empty string and + # typos — keeps dependency management enabled. Better to be noisy than + # to silently swallow installs. + for value in ("on", "true", "1", "yes", "enabled", "auto", "", " ", "off-ish"): + with self.subTest(value=value): + self.assertFalse(self.manager_util.is_off_value(value)) + + +if __name__ == "__main__": + unittest.main()