Files
darth-veitcher-comfydv/tests/test_logging.py
T
James VeitchandClaude Sonnet 4.6 f3574c0202 feat(006): Ollama model integration — 14 nodes for LLM inference in ComfyUI workflows (#8)
* fix: eliminate all identified broken windows before Ollama implementation

- pyproject.toml: add requires-python>=3.11, real description, remove dead
  [project.scripts] entry (no main() exists), add system/integration markers
- __init__.py: replace pytest-in-sys.modules guard with comfy-in-sys.modules
  (checks the actual condition; cleaner semantics)
- tests/conftest.py: remove duplicate module-level class definitions and the
  aiohttp mock (real aiohttp is installed; mock blocked integration tests)
- circuit_breaker.py: remove copy-pasted INPUT_TYPES boilerplate docstring;
  logger.debug → logger.warning for ComfyUI-absent branch (degraded state)
- random_choice.py: remove copy-pasted INPUT_TYPES boilerplate and dead
  triple-quoted string literal; fix bare re-raise to log before propagating
- format_string.py: print() → logger.warning(); remove # type: ignore on
  two dict assignments; remove cargo-culted # noqa: F401 (ruff doesn't flag)
- scripts/take_screenshots.py: remove unused h_pad and v_pad parameters

All 76 tests pass; ruff clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* design(006): full DESIGN phase artefacts for Ollama model integration

Adds spec, plan, tasks, 6 BDD feature files, ADRs, and epic file for
spec 006-ollama-model-integration (14 nodes ported from
darth-veitcher/comfyui-ollama-model-manager, Issue #1 fix, logging
harmonisation, live-service integration tests).

- specs/006-ollama-model-integration/{spec,plan,tasks}.md
- specs/006-ollama-model-integration/features/us{1-6}_*.feature
- specs/006-ollama-model-integration/{.beacon.toml,checklists/}
- project-management/ADRs/ADR-004-aiohttp-over-httpx-for-ollama.md
- project-management/ADRs/ADR-005-ollama-host-config-via-client-node.md
- project-management/Roadmap/epics/ollama-integration.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* chore(006): Phase 1 setup — Ollama fixtures, Justfile recipes, test skeleton

- conftest.py: add ollama_host (session), ollama_available (session),
  skip_if_no_ollama fixtures for @pytest.mark.integration tests
- Justfile: add test, test-unit, test-integration, test-system recipes
- tests/test_ollama.py: empty skeleton with module docstring and lazy-import
  block (uncommented phase-by-phase as ollama.py is built)

Closes T001 (system marker — done in broken-windows commit), T002, T003, T004.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* feat(006): Phase 3 foundational — ollama.py, ollama.js, __init__ wiring (T011-T014)

- src/comfydv/ollama.py: OllamaClientType, _run_async, _fetch_models,
  _post_json, _DEFAULT_MODELS (populated at import), /dv/ollama/models route,
  all 14 node classes (US1-US6) with correct ComfyUI contract
- src/js/ollama.js: app.registerExtension for OllamaModelSelector/LoadModel/
  ChatCompletion — refreshModelDropdown + ⟳ refresh button
- src/comfydv/__init__.py: register all 14 Ollama nodes in
  NODE_CLASS_MAPPINGS and NODE_DISPLAY_NAME_MAPPINGS

Closes T011, T012, T013, T014.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* test(006): full Ollama test suite — T015-T031 all US TDD-T commits (T-T032)

Add tests/test_ollama.py with 6 test classes covering all 14 nodes:
  - TestUS1OllamaConnection: client type, custom host, unreachable error
  - TestUS2ModelSelection: fetch list, selector output, COMBO type, empty fallback
  - TestUS3ModelLifecycle: COMBO type, empty-model guard, load/unload integration
  - TestUS4ChatCompletion: COMBO type, single-turn, multi-turn, history growth
  - TestUS5ComposableOptions: all 7 option nodes + chaining + deterministic
  - TestUS6HistoryInspection: debug output, length counts
  - TestNodeContracts: parametrised over all 14 nodes for ComfyUI contract

Also remove tests/__init__.py — its presence caused pytest to walk up past the
repo root and add the parent directory to sys.path, making the root-level
__init__.py win the 'comfydv' namespace over src/comfydv/.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* style: ruff format test_ollama.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* feat(006): T032/T034 — extend packaging tests and manager entry for 14 Ollama nodes

Update EXPECTED_NODENAMES in test_packaging.py to include all 14 Ollama node
display names. Update comfy-manager-entry.json nodename array and description
to register all 14 nodes with ComfyUI Manager.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* docs(006): T033 — add Ollama section to README and docs/index.md

Add all 14 Ollama nodes to the node reference table and add a dedicated
Ollama section covering minimal workflow, option nodes, and multi-turn
conversation patterns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* test(006): T035-T/I — TestLoggingConsistency: no print(), correct log levels

Add TestLoggingConsistency class to tests/test_logging.py:
- AST scan: assert zero print() calls across all src/comfydv/*.py
- circuit_breaker: ComfyUI-absent branch uses logger.warning() not debug
- random_choice: exception path calls logger.error() and uses plain raise
- ollama: absent-ComfyUI paths emit at least 2 logger.warning() calls

All checks pass (fixes were already in Phase 2 broken windows commit).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* docs: add Quickstart section to README — satisfies beacon doctor readme-completeness

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* chore(006): mark all 55 tasks complete in tasks.md — spec fully implemented

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

* docs(006): T036/T037 — add Ollama screenshots and docker extra_hosts for host Ollama routing

Captures 4 Ollama node screenshots (client, chat, workflow, options) via
Playwright against the live dev ComfyUI instance. Adds extra_hosts to
docker-compose.dev.yml so the container can reach host-machine Ollama.
Updates README and docs/index.md to embed all 8 screenshots.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RWysE6jYn4YjNorLBQ1cXb

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-29 00:55:15 +01:00

360 lines
15 KiB
Python

"""
Logging behaviour tests for comfydv nodes.
Verifies:
- US1: zero stdout/stderr during normal execution
- US2: ERROR records emitted on failure paths
- US3: DEBUG records appear when host opts in; NullHandler default is silent
- Consistency: zero print() in library code; correct log levels on all absent-ComfyUI paths
"""
import logging
import logging.handlers
import os
import sys
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src"))
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _load_format_string():
"""Load FormatString from source, bypassing __init__.py."""
import importlib.util
spec = importlib.util.spec_from_file_location(
"comfydv.format_string",
os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "format_string.py"
),
)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
cls = module.FormatString
cls.node_configs = {}
cls.RETURN_TYPES = ("STRING", "STRING")
cls.RETURN_NAMES = ("formatted_string", "saved_file_path")
cls.OUTPUT_IS_LIST = (False, False)
return cls
def _load_random_choice():
"""Load RandomChoice from source, pre-registering the utils sibling."""
import importlib.util
import types
src = os.path.join(os.path.dirname(__file__), "..", "src")
# Register comfydv package namespace so relative imports work
if "comfydv" not in sys.modules:
pkg = types.ModuleType("comfydv")
pkg.__path__ = [os.path.join(src, "comfydv")]
pkg.__package__ = "comfydv"
sys.modules["comfydv"] = pkg
# Register utils submodule so `from .utils import any_type` resolves
if "comfydv.utils" not in sys.modules:
utils_spec = importlib.util.spec_from_file_location(
"comfydv.utils",
os.path.join(src, "comfydv", "utils.py"),
)
utils_mod = importlib.util.module_from_spec(utils_spec)
sys.modules["comfydv.utils"] = utils_mod
utils_spec.loader.exec_module(utils_mod)
spec = importlib.util.spec_from_file_location(
"comfydv.random_choice",
os.path.join(src, "comfydv", "random_choice.py"),
)
module = importlib.util.module_from_spec(spec)
sys.modules["comfydv.random_choice"] = module
spec.loader.exec_module(module)
return module.RandomChoice
def _load_circuit_breaker():
"""Load CircuitBreaker from source."""
import importlib.util
spec = importlib.util.spec_from_file_location(
"comfydv.circuit_breaker",
os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "circuit_breaker.py"
),
)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module.CircuitBreaker
# ---------------------------------------------------------------------------
# US1: Silent Normal Operation
# ---------------------------------------------------------------------------
class TestUS1SilentNormalOperation:
"""US1: no stdout/stderr from comfydv during successful node execution."""
def test_format_string_produces_no_stdout(self, capsys):
"""T010-T (red): format_string() must not write to stdout."""
FormatString = _load_format_string()
FormatString.format_string(
template_type="Simple",
template="Hello {name}",
save_path="",
unique_id="t010",
name="Alice",
)
captured = capsys.readouterr()
assert captured.out == "", f"Unexpected stdout: {captured.out!r}"
assert captured.err == "", f"Unexpected stderr: {captured.err!r}"
def test_update_widget_produces_no_stdout(self, capsys):
"""T010-T (red): update_widget() must not write to stdout."""
FormatString = _load_format_string()
FormatString.update_widget("node1", "Simple", "Hello {name}")
captured = capsys.readouterr()
assert captured.out == "", f"Unexpected stdout: {captured.out!r}"
assert captured.err == "", f"Unexpected stderr: {captured.err!r}"
def test_is_changed_produces_no_info_records(self, caplog):
"""T011-T (red): IS_CHANGED must not emit records at INFO or above."""
FormatString = _load_format_string()
with caplog.at_level(logging.INFO, logger="comfydv.format_string"):
FormatString.IS_CHANGED(template="Hello {name}", template_type="Simple")
info_plus = [r for r in caplog.records if r.levelno >= logging.INFO]
assert info_plus == [], f"Unexpected INFO+ records: {info_plus}"
def test_update_widget_produces_no_info_records(self, caplog):
"""T011-T (red): update_widget must not emit records at INFO or above."""
FormatString = _load_format_string()
with caplog.at_level(logging.INFO, logger="comfydv.format_string"):
FormatString.update_widget("node1", "Simple", "Hello {name}")
info_plus = [r for r in caplog.records if r.levelno >= logging.INFO]
assert info_plus == [], f"Unexpected INFO+ records: {info_plus}"
def test_random_choice_produces_no_stdout(self, capsys):
"""T012-T (red): random_choice() must not write to stdout."""
RandomChoice = _load_random_choice()
rc = RandomChoice()
rc.random_choice(input1="a", input2="b", seed=42)
captured = capsys.readouterr()
assert captured.out == "", f"Unexpected stdout: {captured.out!r}"
assert captured.err == "", f"Unexpected stderr: {captured.err!r}"
# ---------------------------------------------------------------------------
# US2: Errors Still Surface
# ---------------------------------------------------------------------------
class TestUS2ErrorsStillSurface:
"""US2: error conditions must emit records at ERROR level."""
def test_jinja2_syntax_error_emits_error_record(self, caplog):
"""T020-T (red): invalid Jinja2 template must produce an ERROR log record."""
FormatString = _load_format_string()
with caplog.at_level(logging.DEBUG, logger="comfydv.format_string"):
FormatString.format_string(
template_type="Jinja2",
template="{{ unclosed",
save_path="",
unique_id="t020a",
)
error_records = [r for r in caplog.records if r.levelno >= logging.ERROR]
assert error_records, (
"Expected at least one ERROR record for invalid Jinja2 template"
)
def test_simple_missing_variable_emits_error_record(self, caplog):
"""T020-T (red): missing Simple template variable must produce an ERROR log record."""
FormatString = _load_format_string()
with caplog.at_level(logging.DEBUG, logger="comfydv.format_string"):
try:
FormatString.format_string(
template_type="Simple",
template="Hello {name}",
save_path="",
unique_id="t020b",
)
except KeyError:
pass
error_records = [r for r in caplog.records if r.levelno >= logging.ERROR]
assert error_records, (
"Expected at least one ERROR record for missing template variable"
)
def test_load_node_state_error_emits_error_record(self, caplog):
"""T020-T (red): failed load_node_state must produce an ERROR log record (not a print)."""
FormatString = _load_format_string()
with caplog.at_level(logging.DEBUG, logger="comfydv.format_string"):
# Trigger the except branch with a path that can be opened but is invalid JSON
import tempfile
with tempfile.NamedTemporaryFile(
mode="w", suffix=".json", delete=False
) as f:
f.write("not valid json {{{{")
bad_path = f.name
try:
FormatString.load_node_state(bad_path)
finally:
os.unlink(bad_path)
error_records = [r for r in caplog.records if r.levelno >= logging.ERROR]
assert error_records, (
"Expected at least one ERROR record for failed node state load"
)
def test_circuit_breaker_emits_log_record(self, caplog):
"""T021-T (red): CircuitBreaker.doit with status=False must emit a log record."""
CircuitBreaker = _load_circuit_breaker()
cb = CircuitBreaker()
with caplog.at_level(logging.DEBUG, logger="comfydv.circuit_breaker"):
try:
cb.doit(trigger="img", status=False)
except Exception:
pass
assert caplog.records, "Expected at least one log record from CircuitBreaker"
# ---------------------------------------------------------------------------
# US3: Developer Debug Mode
# ---------------------------------------------------------------------------
class TestUS3DeveloperDebugMode:
"""US3: DEBUG records appear when host opts in; zero records with default config."""
def test_debug_records_appear_with_opt_in(self, caplog):
"""T030-T: configuring DEBUG on comfydv logger must surface trace records."""
FormatString = _load_format_string()
with caplog.at_level(logging.DEBUG, logger="comfydv.format_string"):
FormatString.format_string(
template_type="Simple",
template="Hello {name}",
save_path="",
unique_id="t030a",
name="Alice",
)
debug_records = [r for r in caplog.records if r.levelno == logging.DEBUG]
assert debug_records, (
"Expected at least one DEBUG record when DEBUG level is enabled"
)
def test_null_handler_is_default(self):
"""T030-T: comfydv/__init__.py must register a NullHandler on the package logger."""
# Verify the contract at source level — the NullHandler registration must be present
init_path = os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "__init__.py"
)
with open(init_path) as f:
source = f.read()
assert "NullHandler" in source, (
"comfydv/__init__.py must call "
"logging.getLogger(__name__).addHandler(logging.NullHandler())"
)
# Verify the runtime effect: registering the handler means no records escape
# by default (the NullHandler absorbs them).
FormatString = _load_format_string()
test_handler = logging.handlers.MemoryHandler(
capacity=100, flushLevel=logging.CRITICAL
)
fmt_logger = logging.getLogger("comfydv.format_string")
fmt_logger.addHandler(test_handler)
# Without propagation to root (which has no handler), NullHandler absorbs at comfydv level
original_propagate = fmt_logger.propagate
fmt_logger.propagate = False
try:
FormatString.format_string(
template_type="Simple",
template="Hello {name}",
save_path="",
unique_id="null_test",
name="test",
)
# No records should have reached any external handler (they go to NullHandler)
flushed = test_handler.buffer
assert len(flushed) == 0 or all(
r.levelno < logging.INFO for r in flushed
), f"Records reached external handler: {flushed}"
finally:
fmt_logger.removeHandler(test_handler)
fmt_logger.propagate = original_propagate
# ---------------------------------------------------------------------------
# Logging Consistency (T035-T)
# ---------------------------------------------------------------------------
class TestLoggingConsistency:
"""Cross-module logging contract: no print(), correct levels on degraded paths."""
def test_no_print_calls_in_library_code(self):
"""All src/comfydv/*.py must be free of print() calls in executable code."""
import ast
src_dir = os.path.join(os.path.dirname(__file__), "..", "src", "comfydv")
violations = []
for fname in sorted(os.listdir(src_dir)):
if not fname.endswith(".py"):
continue
fpath = os.path.join(src_dir, fname)
tree = ast.parse(open(fpath).read())
for node in ast.walk(tree):
if (
isinstance(node, ast.Call)
and isinstance(node.func, ast.Name)
and node.func.id == "print"
):
violations.append(f"{fname}:{node.lineno}")
assert not violations, f"print() calls found in library code: {violations}"
def test_circuit_breaker_absent_comfyui_emits_warning(self, caplog):
"""circuit_breaker.py ComfyUI-absent branch must log at WARNING, not DEBUG."""
src = os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "circuit_breaker.py"
)
source = open(src).read()
# Verify at source level that the warning call is present
assert "logger.warning(" in source, (
"circuit_breaker.py must use logger.warning() for the ComfyUI-absent branch"
)
assert (
"logger.debug(" not in source
or "ComfyUI" not in source.split("logger.debug(")[1].split(")")[0]
if "logger.debug(" in source
else True
), "circuit_breaker.py ComfyUI-absent branch must not use logger.debug()"
def test_random_choice_exception_path_emits_error(self, caplog):
"""random_choice.py exception path must emit an ERROR record (not re-raise silently)."""
src = os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "random_choice.py"
)
source = open(src).read()
assert "logger.error(" in source, (
"random_choice.py must call logger.error() on the exception path"
)
assert "raise e" not in source, (
"random_choice.py must not bare-reraise with 'raise e' — use plain 'raise'"
)
def test_ollama_absent_comfyui_emits_warning(self):
"""ollama.py ComfyUI-absent branches must use logger.warning()."""
src = os.path.join(
os.path.dirname(__file__), "..", "src", "comfydv", "ollama.py"
)
source = open(src).read()
# The two absent-ComfyUI paths must both be warnings, not debug
warning_count = source.count("logger.warning(")
assert warning_count >= 2, (
f"ollama.py must have at least 2 logger.warning() calls for absent-ComfyUI paths, "
f"found {warning_count}"
)