better tests

src/ytdl_sub/config/preset.py

@@ -128,12 +128,14 @@ class DownloadStrategyValidator(StrictDictValidator):
             raise self._validation_exception(error_message=value_exc)
 
 
-class Preset(StrictDictValidator):
+class _PresetShell(StrictDictValidator):
     # Have all present keys optional since parent presets could not have all the
     # required keys. They will get validated in the init after the mergedeep of dicts
     # and ensure required keys are present.
     _optional_keys = PRESET_KEYS
 
+
+class Preset(_PresetShell):
     @classmethod
     def _validate_download_strategy(cls, name: str, value: Dict) -> None:
         sources: List[str] = []
@@ -144,7 +146,7 @@ class Preset(StrictDictValidator):
         if len(sources) > 1:
             raise validation_exception(
                 name=name,
-                error_message=f"Contains the sources {', '.join(sources)} but can only have one",
+                error_message=f"Contains the sources {', '.join(sources)}' but can only have one",
             )
 
         # If no sources, nothing more to validate
@@ -165,8 +167,22 @@ class Preset(StrictDictValidator):
 
     @classmethod
     def preset_partial_validate(cls, config: ConfigValidator, name: str, value: Any) -> None:
+        """
+        Partially validates a preset. Used to ensure every preset in a ConfigFile looks sane.
+        Cannot fully validate each preset using the Preset init because required fields could
+        be missing, which become filled in a child preset.
+
+        Parameters
+        ----------
+        config
+            Config that this preset belongs to
+        name
+            Preset name
+        value
+            Preset value
+        """
         # Ensure value is a dict
-        _ = DictValidator(name=name, value=value)
+        _ = _PresetShell(name=name, value=value)
         assert isinstance(value, dict)
 
         cls._validate_download_strategy(name, value)

src/ytdl_sub/config/preset_options.py

@@ -185,6 +185,9 @@ class OutputOptions(StrictDictValidator):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate output options
+        """
         if isinstance(value, dict):
             value["output_directory"] = value.get("output_directory", "placeholder")
             value["file_name"] = value.get("file_name", "placeholder")

src/ytdl_sub/downloaders/generic/collection_validator.py

@@ -45,6 +45,9 @@ class CollectionUrlValidator(StrictDictValidator):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate a YouTube collection url
+        """
         if isinstance(value, dict):
             value["url"] = value.get("url", "placeholder")
         _ = cls(name, value)
@@ -158,6 +161,9 @@ class CollectionValidator(StrictDictValidator, AddsVariablesMixin):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate a generic collection
+        """
         if isinstance(value, dict):
             value["urls"] = value.get("urls", [{"url": "placeholder"}])
         _ = cls(name, value)

src/ytdl_sub/downloaders/youtube/channel.py

@@ -37,6 +37,9 @@ class YoutubeChannelDownloaderOptions(DownloaderValidator):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate a YouTube channel source
+        """
         if isinstance(value, dict):
             value["channel_url"] = value.get(
                 "channel_url", "https://www.youtube.com/c/ProjectZombie603"

src/ytdl_sub/plugins/audio_extract.py

@@ -33,6 +33,9 @@ class AudioExtractOptions(PluginOptions):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate audio extract options
+        """
         if isinstance(value, dict):
             value["codec"] = value.get("codec", "mp3")
         _ = cls(name, value)

src/ytdl_sub/plugins/nfo_tags.py

@@ -31,6 +31,9 @@ class SharedNfoTagsOptions(PluginOptions):
 
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
+        """
+        Partially validate NFO tag options
+        """
         if isinstance(value, dict):
             value["nfo_name"] = value.get("nfo_name", "placeholder")
             value["nfo_root"] = value.get("nfo_root", "placeholder")

tests/unit/config/test_preset.py

@@ -1,8 +1,11 @@
+import re
 from typing import Dict
+from typing import Optional
 
 import pytest
 
 from ytdl_sub.config.config_file import ConfigFile
+from ytdl_sub.config.preset import PRESET_KEYS
 from ytdl_sub.config.preset import Preset
 from ytdl_sub.plugins.nfo_tags import NfoTagsOptions
 from ytdl_sub.utils.exceptions import StringFormattingVariableNotFoundException
@@ -103,7 +106,7 @@ class TestPreset:
                 "youtube": youtube_video,
                 "output_options": dict(
                     output_options,
-                    **{"maintain_download_archive": True, "keep_files_after": "today-{ttl}"}
+                    **{"maintain_download_archive": True, "keep_files_after": "today-{ttl}"},
                 ),
                 "overrides": {"ttl": "2months"},
             },
@@ -206,6 +209,25 @@ class TestPreset:
 
 
 class TestPresetPartialValidate:
+    @classmethod
+    def _partial_validate(
+        cls, preset_dict: Dict, expected_error_message: Optional[str] = None
+    ) -> None:
+        def _config_create() -> None:
+            _ = ConfigFile(
+                name="test_partial_validate",
+                value={
+                    "configuration": {"working_directory": "."},
+                    "presets": {"partial_preset": preset_dict},
+                },
+            )
+
+        if expected_error_message:
+            with pytest.raises(ValidationException, match=re.escape(expected_error_message)):
+                _config_create()
+        else:
+            _config_create()
+
     @pytest.mark.parametrize(
         "preset_dict",
         [
@@ -216,33 +238,48 @@ class TestPresetPartialValidate:
             {"ytdl_options": {"format": "best"}},
         ],
     )
-    def test_partial_validate(self, preset_dict):
+    def test_success(self, preset_dict: Dict):
-        _ = ConfigFile(
+        self._partial_validate(
-            name="test_partial_validate",
+            preset_dict,
-            value={
-                "configuration": {"working_directory": "."},
-                "presets": {"partial_preset": preset_dict},
-            },
         )
 
-    @pytest.mark.parametrize(
+    def test_error__bad_preset_section(self):
-        "preset_dict",
+        self._partial_validate(
-        [
+            preset_dict={"does_not_exist": "lol"},
-            {"youtube": {}, "generic": {}},  # multiple sources
+            expected_error_message="Validation error in partial_preset: "
-            {"generic": {}},  # no download strategy
+            "'partial_preset' contains the field 'does_not_exist' which is not allowed. "
-            {"generic": {"download_strategy": "fail"}},  # bad download strategy
+            f"Allowed fields: {', '.join(sorted(PRESET_KEYS))}",
-            {"generic": {"download_strategy": "collection", "bad_key": "nope"}}  # bad strategy args
+        )
-        ],
+
-    )
+    def test_error__multiple_sources(self):
-    def test_partial_validate__bad_sources(self, preset_dict):
+        self._partial_validate(
-        with pytest.raises(ValidationException):
+            preset_dict={"youtube": {}, "generic": {}},
-            _ = ConfigFile(
+            expected_error_message="Validation error in partial_preset: "
-                name="test_partial_validate",
+            "Contains the sources generic, youtube but can only have one",
-                value={
+        )
-                    "configuration": {"working_directory": "."},
+
-                    "presets": {"partial_preset": preset_dict},
+    def test_error__no_download_strategy(self):
-                },
+        self._partial_validate(
-            )
+            preset_dict={"generic": {}},
+            expected_error_message="Validation error in partial_preset.generic: "
+            "missing the required field 'download_strategy'",
+        )
+
+    def test_error__bad_download_strategy(self):
+        self._partial_validate(
+            preset_dict={"generic": {"download_strategy": "fail"}},
+            expected_error_message="Validation error in partial_preset.generic: "
+            "Tried to use download strategy 'fail' with source 'generic', "
+            "which does not exist. Available download strategies: collection, source",
+        )
+
+    def test_error__bad_download_strategy_args(self):
+        self._partial_validate(
+            preset_dict={"generic": {"download_strategy": "collection", "bad_key": "nope"}},
+            expected_error_message="Validation error in partial_preset.generic: "
+            "'partial_preset.generic' contains the field 'bad_key' which is not allowed. "
+            "Allowed fields: urls",
+        )
 
     @pytest.mark.parametrize(
         "preset_dict",