refactor: DLO-16/17/18/20 — CLI simplification, config Pydantic, planner split, type system unification
DLO-16: Reduce cli.py from 1073 to 83 lines by registering Click commands from commands/*.py modules DLO-17: Migrate Config to Pydantic BaseModel for validation DLO-18: Split planner.py (826 lines) into orchestration, path rendering, and duplicate handling modules DLO-20: Unify type system — convert 14 dataclasses to Pydantic BaseModel, keep TypedDicts as JSON schema hints Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.5
parent
8a60aaf9a9
commit
fe03a31dd4
+188
-234
@@ -4,6 +4,7 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from pydantic import ValidationError
|
||||
|
||||
from vlm.config import Config, create_default_config, load_config, validate_config
|
||||
|
||||
@@ -334,277 +335,235 @@ class TestCreateDefaultConfig:
|
||||
|
||||
|
||||
class TestValidateConfig:
|
||||
"""Test validate_config function."""
|
||||
|
||||
"""Test validation — with Pydantic, invalid values raise ValidationError at construction."""
|
||||
|
||||
def test_validate_valid_config(self):
|
||||
"""Test validating a valid configuration."""
|
||||
config = Config(library_root=Path("/mnt/nas/videos"))
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert errors == []
|
||||
|
||||
assert validate_config(config) == []
|
||||
|
||||
def test_validate_empty_library_root(self):
|
||||
"""Test validating config with empty library_root."""
|
||||
config = Config(library_root=Path(""))
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("library_root" in err for err in errors)
|
||||
|
||||
with pytest.raises(ValidationError, match="library_root"):
|
||||
Config(library_root=Path(""))
|
||||
|
||||
def test_validate_empty_video_extensions(self):
|
||||
"""Test validating config with empty video_extensions."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
video_extensions=[]
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("video_extensions" in err for err in errors)
|
||||
|
||||
with pytest.raises(ValidationError, match="video_extensions"):
|
||||
Config(library_root=Path("/mnt/nas/videos"), video_extensions=[])
|
||||
|
||||
def test_validate_invalid_video_extension_format(self):
|
||||
"""Test validating config with invalid video extension format."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
video_extensions=["mp4", ".mkv"] # Missing dot on first one
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("must start with '.'" in err for err in errors)
|
||||
|
||||
def test_validate_empty_templates(self):
|
||||
"""Test validating config with empty templates."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
movie_template="",
|
||||
series_template=""
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) >= 2
|
||||
assert any("movie_template" in err for err in errors)
|
||||
assert any("series_template" in err for err in errors)
|
||||
|
||||
def test_validate_invalid_log_level(self):
|
||||
"""Test validating config with invalid log level."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
log_level="INVALID"
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("log_level" in err for err in errors)
|
||||
|
||||
def test_validate_valid_log_levels(self):
|
||||
"""Test validating config with all valid log levels."""
|
||||
valid_levels = ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]
|
||||
|
||||
for level in valid_levels:
|
||||
config = Config(
|
||||
with pytest.raises(ValidationError, match="must start with"):
|
||||
Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
log_level=level
|
||||
video_extensions=["mp4", ".mkv"],
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert errors == [], f"Log level {level} should be valid"
|
||||
|
||||
|
||||
def test_validate_empty_templates(self):
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
movie_template="",
|
||||
series_template="",
|
||||
)
|
||||
errors = exc_info.value.errors()
|
||||
fields = {e["loc"][0] for e in errors}
|
||||
assert "movie_template" in fields
|
||||
assert "series_template" in fields
|
||||
|
||||
def test_validate_invalid_log_level(self):
|
||||
with pytest.raises(ValidationError, match="log_level"):
|
||||
Config(library_root=Path("/mnt/nas/videos"), log_level="INVALID")
|
||||
|
||||
def test_validate_valid_log_levels(self):
|
||||
for level in ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]:
|
||||
config = Config(library_root=Path("/mnt/nas/videos"), log_level=level)
|
||||
assert validate_config(config) == [], f"Log level {level} should be valid"
|
||||
|
||||
def test_validate_absolute_quarantine_dir(self):
|
||||
"""Test validating config with absolute quarantine_dir."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
quarantine_dir="/absolute/path"
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("must be relative" in err for err in errors)
|
||||
|
||||
with pytest.raises(ValidationError, match="must be relative"):
|
||||
Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
quarantine_dir="/absolute/path",
|
||||
)
|
||||
|
||||
def test_validate_empty_quarantine_dir(self):
|
||||
"""Test validating config with empty quarantine_dir."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
quarantine_dir=""
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
assert len(errors) > 0
|
||||
assert any("quarantine_dir" in err for err in errors)
|
||||
with pytest.raises(ValidationError, match="quarantine_dir"):
|
||||
Config(library_root=Path("/mnt/nas/videos"), quarantine_dir="")
|
||||
|
||||
def test_validate_workspace_dir_type(self):
|
||||
"""workspace_dir must be a Path object."""
|
||||
def test_workspace_dir_coerces_from_string(self):
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
workspace_dir="artifacts", # type: ignore[arg-type]
|
||||
workspace_dir="artifacts",
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("workspace_dir must be a Path object" in err for err in errors)
|
||||
|
||||
assert config.workspace_dir == Path("artifacts")
|
||||
|
||||
def test_validate_multiple_errors(self):
|
||||
"""Test validating config with multiple errors."""
|
||||
config = Config(
|
||||
library_root=Path(""),
|
||||
video_extensions=[],
|
||||
movie_template="",
|
||||
log_level="INVALID"
|
||||
)
|
||||
|
||||
errors = validate_config(config)
|
||||
|
||||
# Should have multiple errors
|
||||
assert len(errors) >= 4
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Config(
|
||||
library_root=Path(""),
|
||||
video_extensions=[],
|
||||
movie_template="",
|
||||
log_level="INVALID",
|
||||
)
|
||||
assert len(exc_info.value.errors()) >= 4
|
||||
|
||||
def test_validate_duplicate_keep_reputation_quality_time(self):
|
||||
"""Test validating config with by_reputation_quality_time strategy."""
|
||||
config = Config(
|
||||
library_root=Path("/mnt/nas/videos"),
|
||||
duplicate_keep="by_reputation_quality_time"
|
||||
duplicate_keep="by_reputation_quality_time",
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert errors == []
|
||||
assert validate_config(config) == []
|
||||
|
||||
def test_validate_empty_categories(self):
|
||||
"""Test validating config with empty categories."""
|
||||
config = Config(library_root=Path("/test"), categories={})
|
||||
errors = validate_config(config)
|
||||
assert any("categories" in e and "empty" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="categories"):
|
||||
Config(library_root=Path("/test"), categories={})
|
||||
|
||||
def test_validate_missing_required_category(self):
|
||||
"""Test validating config with missing required categories."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={"movie": ["movie"]} # Missing series, anime
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("series" in e or "anime" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="categories"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={"movie": ["movie"]},
|
||||
)
|
||||
|
||||
def test_validate_duplicate_directory_names(self):
|
||||
"""Test validating config with duplicate directory names."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", "videos"],
|
||||
"series": ["series", "videos"], # Duplicate
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("Duplicate" in e and "videos" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="Duplicate.*videos"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", "videos"],
|
||||
"series": ["series", "videos"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_case_insensitive_duplicates(self):
|
||||
"""Test validating config with case-insensitive duplicates."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["Movie"],
|
||||
"series": ["movie"], # Case-insensitive duplicate
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("Duplicate" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="Duplicate"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["Movie"],
|
||||
"series": ["movie"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_valid_custom_categories(self):
|
||||
"""Test validating config with valid custom categories."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", "movies"],
|
||||
"series": ["series", "tv"],
|
||||
"anime": ["anime"]
|
||||
}
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert errors == []
|
||||
assert validate_config(config) == []
|
||||
|
||||
def test_validate_categories_not_dict(self):
|
||||
"""Test validating config with categories not a dict."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories=["movie", "series"] # Wrong type
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("must be a dictionary" in e for e in errors)
|
||||
with pytest.raises(ValidationError):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories=["movie", "series"],
|
||||
)
|
||||
|
||||
def test_validate_category_list_not_list(self):
|
||||
"""Test validating config with category value not a list."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": "movie", # Should be a list
|
||||
"series": ["series"],
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("must be a list" in e for e in errors)
|
||||
with pytest.raises(ValidationError):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": "movie",
|
||||
"series": ["series"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_empty_category_list(self):
|
||||
"""Test validating config with empty category list."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": [], # Empty list
|
||||
"series": ["series"],
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("cannot be empty" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="cannot be empty"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": [],
|
||||
"series": ["series"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_rejects_unsupported_enrichment_provider(self):
|
||||
"""Test validating config with unsupported enrichment provider."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
enrichment_providers=["tmdb", "douban"],
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("unsupported providers" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="unsupported providers"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
enrichment_providers=["tmdb", "douban"],
|
||||
)
|
||||
|
||||
def test_validate_category_list_with_non_string(self):
|
||||
"""Test validating config with non-string in category list."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", 123], # Non-string
|
||||
"series": ["series"],
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("must contain strings" in e for e in errors)
|
||||
with pytest.raises(ValidationError):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", 123],
|
||||
"series": ["series"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_category_list_with_empty_string(self):
|
||||
"""Test validating config with empty string in category list."""
|
||||
config = Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", ""], # Empty string
|
||||
"series": ["series"],
|
||||
"anime": ["anime"]
|
||||
}
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("empty directory name" in e for e in errors)
|
||||
with pytest.raises(ValidationError, match="empty directory name"):
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
categories={
|
||||
"movie": ["movie", ""],
|
||||
"series": ["series"],
|
||||
"anime": ["anime"],
|
||||
},
|
||||
)
|
||||
|
||||
def test_validate_invalid_plan_thresholds(self):
|
||||
"""Plan season/episode thresholds must be positive integers."""
|
||||
config = Config(
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Config(
|
||||
library_root=Path("/test"),
|
||||
plan_max_season=0,
|
||||
plan_max_episode=-1,
|
||||
)
|
||||
fields = {e["loc"][0] for e in exc_info.value.errors()}
|
||||
assert "plan_max_season" in fields
|
||||
assert "plan_max_episode" in fields
|
||||
|
||||
def test_validate_config_with_model_construct_bypass(self):
|
||||
"""validate_config catches errors bypassed via model_construct."""
|
||||
config = Config.model_construct(
|
||||
library_root=Path("/test"),
|
||||
plan_max_season=0,
|
||||
plan_max_episode=-1,
|
||||
video_extensions=[],
|
||||
movie_template="movie/{title} ({year})/",
|
||||
series_template="series/{title}/Season {season:02d}/",
|
||||
movie_filename_template="{title} ({year}){ext}",
|
||||
series_filename_template="S{season:02d}E{episode:02d}{ext}",
|
||||
log_level="INFO",
|
||||
quarantine_dir=".quarantine",
|
||||
workspace_dir=Path("artifacts"),
|
||||
categories={"movie": ["movie"], "series": ["series"], "anime": ["anime"]},
|
||||
enrichment_enabled=True,
|
||||
enrichment_incremental=True,
|
||||
enrichment_refresh_mode="manual",
|
||||
enrichment_providers=["tmdb"],
|
||||
enrichment_cache_db=Path.home() / ".vlm" / "enrichment_cache.db",
|
||||
enrichment_max_concurrency=6,
|
||||
enrichment_min_match_score=0.75,
|
||||
translation_mode="bidirectional",
|
||||
translation_fallback_machine=True,
|
||||
tmdb_api_key=None,
|
||||
tmdb_bearer_token=None,
|
||||
tmdb_language="zh-CN",
|
||||
tmdb_region=None,
|
||||
tmdb_include_adult=False,
|
||||
openai_api_key=None,
|
||||
reputation_min_votes=50,
|
||||
reputation_low_score_threshold=6.0,
|
||||
reputation_policy="flag_for_review",
|
||||
naming_title_format="{title_zh} {title_en}",
|
||||
duplicate_keep="by_reputation",
|
||||
plan_max_season=15,
|
||||
plan_max_episode=100,
|
||||
plan_include_sample_files=False,
|
||||
)
|
||||
errors = validate_config(config)
|
||||
assert any("plan_max_season" in e for e in errors)
|
||||
assert any("plan_max_episode" in e for e in errors)
|
||||
assert any("video_extensions" in e for e in errors)
|
||||
|
||||
|
||||
class TestConfigIntegration:
|
||||
@@ -654,26 +613,21 @@ class TestConfigIntegration:
|
||||
assert loaded_config.library_root == default_config.library_root
|
||||
|
||||
def test_validation_workflow(self, tmp_path):
|
||||
"""Test workflow: load config -> validate -> report errors."""
|
||||
"""Test workflow: load config with invalid values raises ValidationError."""
|
||||
config_file = tmp_path / "config.yaml"
|
||||
|
||||
# Create config with some invalid values
|
||||
|
||||
config_data = {
|
||||
'library_root': '/mnt/nas/videos',
|
||||
'video_extensions': ['mp4', '.mkv'], # First one missing dot
|
||||
'log_level': 'INVALID'
|
||||
}
|
||||
|
||||
|
||||
with open(config_file, 'w') as f:
|
||||
yaml.dump(config_data, f)
|
||||
|
||||
# Load config
|
||||
config = load_config(config_file)
|
||||
|
||||
# Validate
|
||||
errors = validate_config(config)
|
||||
|
||||
# Should have errors
|
||||
assert len(errors) > 0
|
||||
assert any("must start with '.'" in err for err in errors)
|
||||
assert any("log_level" in err for err in errors)
|
||||
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
load_config(config_file)
|
||||
|
||||
messages = [e["msg"] for e in exc_info.value.errors()]
|
||||
assert any("must start with" in m for m in messages)
|
||||
assert any("log_level" in m for m in messages)
|
||||
|
||||
Reference in New Issue
Block a user