mirror of
https://github.com/Comfy-Org/ComfyUI.git
synced 2026-08-29 04:17:14 +08:00
refactor(assets): hoist mid-file imports to module scope in tests (D21)
Re-measured inventory: 56 nested imports across 14 test files. All hoisted to module scope. Empirically tested every import folder_paths hoist — none caused regression. No folder_paths exceptions needed. Side effect: hoisting contextmanager + SASession out of the mock_create_session fixture body in services/conftest.py fixed 3 pre-existing test failures (monkeypatch was targeting stale module references when the imports were nested inside the fixture). Ruff cleanup: removed duplicate import tempfile (services/conftest.py), unused sqlalchemy.select (test_delete_b.py), unused os (test_lifecycle.py) — all were pre-existing issues hidden by the nested-import pattern. Zero assertion changes — only import lines moved. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -13,6 +13,8 @@ from typing import Callable, Iterator, Optional
|
||||
import pytest
|
||||
import requests
|
||||
|
||||
from .helpers import assert_hash_fields_consistent
|
||||
|
||||
|
||||
def pytest_addoption(parser: pytest.Parser) -> None:
|
||||
"""
|
||||
@@ -272,7 +274,6 @@ def seeded_asset(request: pytest.FixtureRequest, http: requests.Session, api_bas
|
||||
r = http.post(api_base + "/api/assets", files=files, data=form_data, timeout=120)
|
||||
body = r.json()
|
||||
assert r.status_code == 201, body
|
||||
from .helpers import assert_hash_fields_consistent
|
||||
assert_hash_fields_consistent(body)
|
||||
return body
|
||||
|
||||
|
||||
@@ -83,9 +83,6 @@ def test_preview_cleanup_on_delete(session):
|
||||
|
||||
def test_concurrent_create_content_same_path(tmp_path):
|
||||
"""Concurrent inserts for the same live path: exactly one live row wins."""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
db_path = str(tmp_path / "concurrent.db")
|
||||
engine = create_engine(f"sqlite:///{db_path}", connect_args={"check_same_thread": False})
|
||||
Base.metadata.create_all(engine)
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import tempfile
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
import tempfile
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import create_engine, event
|
||||
from sqlalchemy.orm import Session
|
||||
from sqlalchemy.orm import Session, Session as SASession
|
||||
|
||||
from app.assets.database.models import Base
|
||||
|
||||
@@ -49,9 +49,6 @@ def session(db_engine):
|
||||
@pytest.fixture
|
||||
def mock_create_session(db_engine):
|
||||
"""Patch create_session to use our in-memory database."""
|
||||
from contextlib import contextmanager
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
@contextmanager
|
||||
def _create_session():
|
||||
with SASession(db_engine) as sess:
|
||||
|
||||
@@ -15,6 +15,7 @@ from unittest.mock import patch
|
||||
|
||||
from sqlalchemy import event, select
|
||||
|
||||
import folder_paths
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
from comfy_execution.asset_enrichment import (
|
||||
register_cached_outputs,
|
||||
@@ -33,8 +34,6 @@ def _assets_enabled(enabled: bool = True):
|
||||
|
||||
|
||||
def _write_output_file(name: str) -> Path:
|
||||
import folder_paths
|
||||
|
||||
path = Path(folder_paths.get_output_directory()) / name
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(b"cached output")
|
||||
|
||||
@@ -2,13 +2,13 @@ import os
|
||||
|
||||
from sqlalchemy import event, select
|
||||
|
||||
import folder_paths
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
from app.assets.database.queries.records import mark_content_missing
|
||||
from app.assets.services.ingest import register_cached_output, register_executed_output
|
||||
|
||||
|
||||
def test_cached_save_creates_delivery_record(mock_create_session, db_engine):
|
||||
import folder_paths
|
||||
from app.assets.services.ingest import register_cached_output, register_executed_output
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
path = os.path.join(output_dir, "test_cached_save.png")
|
||||
@@ -48,10 +48,6 @@ def test_cached_save_against_missing_content_is_nonevent(mock_create_session):
|
||||
no longer falls back to a fresh executed registration - it returns None and
|
||||
creates nothing, leaving only the original (now-missing) content row.
|
||||
"""
|
||||
import folder_paths
|
||||
from app.assets.database.queries.records import mark_content_missing
|
||||
from app.assets.services.ingest import register_cached_output, register_executed_output
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
path = os.path.join(output_dir, "test_cached_save_missing.png")
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
import os
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import select, update
|
||||
from sqlalchemy import update
|
||||
|
||||
import folder_paths
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
from app.assets.database.queries.records import (
|
||||
create_content,
|
||||
@@ -62,8 +63,6 @@ def test_missing_content_record_deletable(mock_create_session, session):
|
||||
|
||||
|
||||
def test_reregister_same_path_fresh_ids(mock_create_session, monkeypatch):
|
||||
import folder_paths
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
f = os.path.join(output_dir, "test_delete_reregister.png")
|
||||
|
||||
@@ -5,7 +5,9 @@ import pytest
|
||||
from sqlalchemy import select
|
||||
|
||||
import app.assets.mode as mode_module
|
||||
import folder_paths
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
from app.assets.services.ingest import register_executed_output
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
@@ -20,9 +22,6 @@ def hashing_off():
|
||||
|
||||
def test_new_path_save_off_mode_hash_null(mock_create_session):
|
||||
"""New file registered in off mode: content row has hash=NULL."""
|
||||
import folder_paths
|
||||
from app.assets.services.ingest import register_executed_output
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
f = os.path.join(output_dir, "test_ingest_b_new.png")
|
||||
@@ -49,9 +48,6 @@ def test_new_path_save_off_mode_hash_null(mock_create_session):
|
||||
|
||||
def test_overwrite_at_live_path_marks_old_missing(mock_create_session):
|
||||
"""Overwriting a live path marks the old content row missing; old record's job_id unchanged."""
|
||||
import folder_paths
|
||||
from app.assets.services.ingest import register_executed_output
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
f = os.path.join(output_dir, "test_ingest_b_overwrite.png")
|
||||
|
||||
@@ -1,13 +1,15 @@
|
||||
"""Todo 16: startup/shutdown temp wipe ordering and failure behavior."""
|
||||
import os
|
||||
import tempfile
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import create_engine, select
|
||||
from sqlalchemy.orm import Session
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
from app.assets import lifecycle
|
||||
from app.assets.database.models import Asset, AssetContent, Base
|
||||
from app.assets.database.queries.records import create_content, create_record
|
||||
from app.assets.lifecycle import (
|
||||
@@ -19,12 +21,11 @@ from app.assets.lifecycle import (
|
||||
wipe_temp_db_rows,
|
||||
)
|
||||
from app.assets.scanner import get_temp_prefixes, sync_temp_references_safely
|
||||
from app.assets.seeder import asset_seeder
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def autoclean_unit_test_assets():
|
||||
from app.assets import lifecycle
|
||||
|
||||
lifecycle._excluded_scan_roots.clear()
|
||||
yield
|
||||
lifecycle._excluded_scan_roots.clear()
|
||||
@@ -40,9 +41,6 @@ def session():
|
||||
|
||||
@pytest.fixture
|
||||
def mock_create_session(session):
|
||||
from contextlib import contextmanager
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
engine = session.bind
|
||||
|
||||
@contextmanager
|
||||
@@ -164,8 +162,6 @@ def test_rmtree_failure_excludes_temp_from_scan(session, comfy_dirs, mock_create
|
||||
def test_shutdown_skips_cleanup_when_seeder_join_times_out(session, comfy_dirs, mock_create_session, caplog):
|
||||
record_id, _ = _seed_temp_rows(session, comfy_dirs)
|
||||
|
||||
from app.assets.seeder import asset_seeder
|
||||
|
||||
with patch("app.assets.lifecycle.wipe_temp_db_rows") as wipe_mock:
|
||||
with patch.object(asset_seeder, "shutdown", return_value=False):
|
||||
joined = asset_seeder.shutdown()
|
||||
|
||||
@@ -17,6 +17,7 @@ from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import folder_paths
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
@@ -37,8 +38,6 @@ def _assets_enabled(enabled: bool = True):
|
||||
|
||||
|
||||
def _write_output_file(name: str, data: bytes) -> Path:
|
||||
import folder_paths
|
||||
|
||||
path = Path(folder_paths.get_output_directory()) / name
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(data)
|
||||
|
||||
@@ -3,18 +3,25 @@ from __future__ import annotations
|
||||
import json
|
||||
import os
|
||||
import uuid
|
||||
from contextlib import contextmanager
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from aiohttp import web
|
||||
from aiohttp.test_utils import make_mocked_request
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
from app.assets import mode
|
||||
from app.assets.api import routes
|
||||
import app.assets.mode as mode_module
|
||||
import folder_paths
|
||||
from app.assets.database.models import Asset, AssetContent
|
||||
from app.assets.database.queries.records import create_record
|
||||
from app.assets.scanner import enrich_asset
|
||||
from app.assets.scanner_changes import recover_missing_content
|
||||
from app.assets.services import asset_management, ingest
|
||||
from app.assets.services.asset_management import get_asset_detail
|
||||
from app.assets.services.ingest import (
|
||||
HashMismatchError,
|
||||
upload_from_temp_path,
|
||||
@@ -36,8 +43,6 @@ def hashing_on():
|
||||
|
||||
|
||||
def _write_temp(content: bytes) -> str:
|
||||
import folder_paths
|
||||
|
||||
uploads_root = os.path.join(
|
||||
folder_paths.get_temp_directory(), "uploads", uuid.uuid4().hex
|
||||
)
|
||||
@@ -190,15 +195,6 @@ def test_recovery_matches_prefixed_stored_hash(session, temp_dir):
|
||||
async def test_all_read_surfaces_agree_on_prefixed_hash(
|
||||
db_engine, monkeypatch, hashing_on
|
||||
):
|
||||
from contextlib import contextmanager
|
||||
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
from app.assets import mode
|
||||
from app.assets.api import routes
|
||||
from app.assets.services import asset_management, ingest
|
||||
from app.assets.services.asset_management import get_asset_detail
|
||||
|
||||
@contextmanager
|
||||
def _factory():
|
||||
with SASession(db_engine) as sess:
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import time as _time
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@@ -10,7 +12,6 @@ def test_missing_content_disappears_from_list_after_rm(
|
||||
)
|
||||
next((comfy_tmp_base_dir / "output").glob("*.png")).unlink()
|
||||
|
||||
import time as _time
|
||||
response = None
|
||||
for _attempt in range(5):
|
||||
response = http.post(
|
||||
|
||||
@@ -29,11 +29,21 @@ import json
|
||||
import sqlite3
|
||||
import time
|
||||
import uuid
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
from sqlalchemy import create_engine, select
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
import app.assets.mode as mode_module
|
||||
import folder_paths
|
||||
from app.assets.database.models import AssetContent, Base
|
||||
from app.assets.services.ingest import register_executed_output
|
||||
|
||||
from .helpers import trigger_sync_seed_assets
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# DB access helpers (read the subprocess's sqlite file directly, read-only)
|
||||
@@ -192,8 +202,6 @@ def test_seeded_file_not_hashed_in_on_mode(
|
||||
"scanner hashing gate only observable with the hashing flag OFF; "
|
||||
"a full-suite run forces the shared server into hash mode"
|
||||
)
|
||||
from .helpers import trigger_sync_seed_assets
|
||||
|
||||
db_path = _db_path(comfy_tmp_base_dir, request)
|
||||
ckpt_dir = comfy_tmp_base_dir / "models" / "checkpoints"
|
||||
ckpt_dir.mkdir(parents=True, exist_ok=True)
|
||||
@@ -225,15 +233,6 @@ def test_output_not_hashed_in_on_mode(monkeypatch):
|
||||
the DB pointed at an in-memory engine. This is deterministic regardless of
|
||||
the shared subprocess's mode.
|
||||
"""
|
||||
from contextlib import contextmanager
|
||||
|
||||
from sqlalchemy import create_engine, select
|
||||
from sqlalchemy.orm import Session as SASession
|
||||
|
||||
import app.assets.mode as mode_module
|
||||
import folder_paths
|
||||
from app.assets.database.models import AssetContent, Base
|
||||
|
||||
engine = create_engine("sqlite:///:memory:")
|
||||
Base.metadata.create_all(engine)
|
||||
|
||||
@@ -247,8 +246,6 @@ def test_output_not_hashed_in_on_mode(monkeypatch):
|
||||
"app.assets.services.ingest.create_session", _fake_create_session
|
||||
)
|
||||
|
||||
from app.assets.services.ingest import register_executed_output
|
||||
|
||||
output_dir = folder_paths.get_output_directory()
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
out_path = os.path.join(output_dir, f"render_{uuid.uuid4().hex}.png")
|
||||
|
||||
Reference in New Issue
Block a user