From 97e84d399cb8cafee559a60925de28995ebaa59c Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Tue, 28 Apr 2026 03:14:45 -0700 Subject: [PATCH] test: scope nodes stub to a fixture to avoid sys.modules leakage Per coderabbit feedback: replace the module-level sys.modules['nodes'] = stub with a per-test fixture using monkeypatch.setitem so the fake module is torn down after each test, and reload app.node_replace_manager so it picks up the stub instead of any cached real import. Amp-Thread-ID: https://ampcode.com/threads/T-019dd37c-4751-72ef-9927-3182b5825db0 Co-authored-by: Amp --- .../app_test/node_replace_manager_test.py | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/tests-unit/app_test/node_replace_manager_test.py b/tests-unit/app_test/node_replace_manager_test.py index cf1be3034..8a3fd18bb 100644 --- a/tests-unit/app_test/node_replace_manager_test.py +++ b/tests-unit/app_test/node_replace_manager_test.py @@ -1,15 +1,29 @@ """Tests for NodeReplaceManager registration behavior.""" +import importlib import sys import types -# Stub `nodes` to avoid the heavy torch + ComfyUI import chain — register() -# only touches its own dict, so it doesn't actually need NODE_CLASS_MAPPINGS. -if "nodes" not in sys.modules: - _fake_nodes = types.ModuleType("nodes") - _fake_nodes.NODE_CLASS_MAPPINGS = {} - sys.modules["nodes"] = _fake_nodes +import pytest -from app.node_replace_manager import NodeReplaceManager # noqa: E402 + +@pytest.fixture +def NodeReplaceManager(monkeypatch): + """Provide NodeReplaceManager with `nodes` stubbed. + + `app.node_replace_manager` does `import nodes` at module level, which pulls in + torch + the full ComfyUI graph. register() doesn't actually need it, so we + stub `nodes` per-test (via monkeypatch so it's torn down) and reload the + module so it picks up the stub instead of any cached real import. + """ + fake_nodes = types.ModuleType("nodes") + fake_nodes.NODE_CLASS_MAPPINGS = {} + monkeypatch.setitem(sys.modules, "nodes", fake_nodes) + monkeypatch.delitem(sys.modules, "app.node_replace_manager", raising=False) + module = importlib.import_module("app.node_replace_manager") + yield module.NodeReplaceManager + # Drop the freshly-imported module so the next test (or a later real import + # of `nodes`) starts from a clean slate. + sys.modules.pop("app.node_replace_manager", None) class FakeNodeReplace: @@ -23,14 +37,14 @@ class FakeNodeReplace: self.output_mapping = output_mapping -def test_register_adds_replacement(): +def test_register_adds_replacement(NodeReplaceManager): manager = NodeReplaceManager() manager.register(FakeNodeReplace(new_node_id="NewNode", old_node_id="OldNode")) assert manager.has_replacement("OldNode") assert len(manager.get_replacement("OldNode")) == 1 -def test_register_allows_multiple_alternatives_for_same_old_node(): +def test_register_allows_multiple_alternatives_for_same_old_node(NodeReplaceManager): """Different new_node_ids for the same old_node_id should all be kept.""" manager = NodeReplaceManager() manager.register(FakeNodeReplace(new_node_id="AltA", old_node_id="OldNode")) @@ -40,7 +54,7 @@ def test_register_allows_multiple_alternatives_for_same_old_node(): assert {r.new_node_id for r in replacements} == {"AltA", "AltB"} -def test_register_is_idempotent_for_duplicate_pair(): +def test_register_is_idempotent_for_duplicate_pair(NodeReplaceManager): """Re-registering the same (old_node_id, new_node_id) should be a no-op.""" manager = NodeReplaceManager() manager.register(FakeNodeReplace(new_node_id="NewNode", old_node_id="OldNode")) @@ -49,7 +63,7 @@ def test_register_is_idempotent_for_duplicate_pair(): assert len(manager.get_replacement("OldNode")) == 1 -def test_register_idempotent_preserves_first_registration(): +def test_register_idempotent_preserves_first_registration(NodeReplaceManager): """First registration wins; later duplicates with different mappings are ignored.""" manager = NodeReplaceManager() first = FakeNodeReplace( @@ -67,7 +81,7 @@ def test_register_idempotent_preserves_first_registration(): assert replacements[0] is first -def test_register_dedupe_does_not_affect_other_old_nodes(): +def test_register_dedupe_does_not_affect_other_old_nodes(NodeReplaceManager): manager = NodeReplaceManager() manager.register(FakeNodeReplace(new_node_id="NewA", old_node_id="OldA")) manager.register(FakeNodeReplace(new_node_id="NewA", old_node_id="OldA"))