From 4b43d6f7c7d4c25baf35f2bcc876acf207dd3e56 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Tue, 25 Oct 2022 22:15:39 -0700 Subject: [PATCH] better tests --- src/ytdl_sub/config/preset.py | 22 ++++- src/ytdl_sub/config/preset_options.py | 3 + .../generic/collection_validator.py | 6 ++ src/ytdl_sub/downloaders/youtube/channel.py | 3 + src/ytdl_sub/plugins/audio_extract.py | 3 + src/ytdl_sub/plugins/nfo_tags.py | 3 + tests/unit/config/test_preset.py | 89 +++++++++++++------ 7 files changed, 100 insertions(+), 29 deletions(-) diff --git a/src/ytdl_sub/config/preset.py b/src/ytdl_sub/config/preset.py index b7c810a8..9dba29a8 100644 --- a/src/ytdl_sub/config/preset.py +++ b/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) diff --git a/src/ytdl_sub/config/preset_options.py b/src/ytdl_sub/config/preset_options.py index 886ec379..d355e807 100644 --- a/src/ytdl_sub/config/preset_options.py +++ b/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") diff --git a/src/ytdl_sub/downloaders/generic/collection_validator.py b/src/ytdl_sub/downloaders/generic/collection_validator.py index 6547ce2c..ba10467f 100644 --- a/src/ytdl_sub/downloaders/generic/collection_validator.py +++ b/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) diff --git a/src/ytdl_sub/downloaders/youtube/channel.py b/src/ytdl_sub/downloaders/youtube/channel.py index 44b730a9..ca019a64 100644 --- a/src/ytdl_sub/downloaders/youtube/channel.py +++ b/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" diff --git a/src/ytdl_sub/plugins/audio_extract.py b/src/ytdl_sub/plugins/audio_extract.py index 5c04195f..0840a37d 100644 --- a/src/ytdl_sub/plugins/audio_extract.py +++ b/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) diff --git a/src/ytdl_sub/plugins/nfo_tags.py b/src/ytdl_sub/plugins/nfo_tags.py index 5c938607..ffd8c225 100644 --- a/src/ytdl_sub/plugins/nfo_tags.py +++ b/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") diff --git a/tests/unit/config/test_preset.py b/tests/unit/config/test_preset.py index 42aa2ab6..279bfc61 100644 --- a/tests/unit/config/test_preset.py +++ b/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): - _ = ConfigFile( - name="test_partial_validate", - value={ - "configuration": {"working_directory": "."}, - "presets": {"partial_preset": preset_dict}, - }, + def test_success(self, preset_dict: Dict): + self._partial_validate( + preset_dict, ) - @pytest.mark.parametrize( - "preset_dict", - [ - {"youtube": {}, "generic": {}}, # multiple sources - {"generic": {}}, # no download strategy - {"generic": {"download_strategy": "fail"}}, # bad download strategy - {"generic": {"download_strategy": "collection", "bad_key": "nope"}} # bad strategy args - ], - ) - def test_partial_validate__bad_sources(self, preset_dict): - with pytest.raises(ValidationException): - _ = ConfigFile( - name="test_partial_validate", - value={ - "configuration": {"working_directory": "."}, - "presets": {"partial_preset": preset_dict}, - }, - ) + def test_error__bad_preset_section(self): + self._partial_validate( + preset_dict={"does_not_exist": "lol"}, + expected_error_message="Validation error in partial_preset: " + "'partial_preset' contains the field 'does_not_exist' which is not allowed. " + f"Allowed fields: {', '.join(sorted(PRESET_KEYS))}", + ) + + def test_error__multiple_sources(self): + self._partial_validate( + preset_dict={"youtube": {}, "generic": {}}, + expected_error_message="Validation error in partial_preset: " + "Contains the sources generic, youtube but can only have one", + ) + + 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",