fix mocks

src/ytdl_sub/downloaders/downloader.py

@@ -20,8 +20,7 @@ from ytdl_sub.downloaders.generic.validators import MultiUrlValidator
 from ytdl_sub.downloaders.generic.validators import UrlThumbnailListValidator
 from ytdl_sub.downloaders.generic.validators import UrlValidator
 from ytdl_sub.downloaders.ytdl_options_builder import YTDLOptionsBuilder
-from ytdl_sub.downloaders.ytdlp import extract_info_via_info_json
+from ytdl_sub.downloaders.ytdlp import YTDLP
-from ytdl_sub.downloaders.ytdlp import extract_info_with_retry
 from ytdl_sub.entries.entry import Entry
 from ytdl_sub.entries.entry_parent import EntryParent
 from ytdl_sub.entries.variables.kwargs import COMMENTS
@@ -266,7 +265,7 @@ class YtDlpDownloader(BaseDownloader[DownloaderOptionsT], ABC):
                 FileHandler.delete(info_json_file)
 
     def _extract_entry_info_with_retry(self, entry: Entry) -> Entry:
-        download_entry_dict = extract_info_with_retry(
+        download_entry_dict = YTDLP.extract_info_with_retry(
             ytdl_options_overrides=self.download_ytdl_options,
             is_downloaded_fn=None if self.is_dry_run else entry.is_downloaded,
             is_thumbnail_downloaded_fn=None
@@ -334,7 +333,7 @@ class YtDlpDownloader(BaseDownloader[DownloaderOptionsT], ABC):
         url = self.overrides.apply_formatter(collection_url.url)
 
         with self._separate_download_archives():
-            entry_dicts = extract_info_via_info_json(
+            entry_dicts = YTDLP.extract_info_via_info_json(
                 working_directory=self.working_directory,
                 ytdl_options_overrides=self.metadata_ytdl_options,
                 log_prefix_on_info_json_dl="Downloading metadata for",

src/ytdl_sub/downloaders/ytdlp.py

@@ -19,25 +19,27 @@ from ytdl_sub.thread.log_entries_downloaded_listener import LogEntriesDownloaded
 from ytdl_sub.utils.exceptions import FileNotDownloadedException
 from ytdl_sub.utils.logger import Logger
 
-download_logger = Logger.get(name="yt-dlp-downloader")
 
-_extract_entry_num_retries: int = 5
+class YTDLP:
-_extract_entry_retry_wait_sec: int = 5
+    _EXTRACT_ENTRY_NUM_RETRIES: int = 5
+    _EXTRACT_ENTRY_RETRY_WAIT_SEC: int = 5
 
+    logger = Logger.get(name="yt-dlp-downloader")
 
+    @classmethod
     @contextmanager
-def ytdlp_downloader(ytdl_options_overrides: Dict) -> ytdl.YoutubeDL:
+    def ytdlp_downloader(cls, ytdl_options_overrides: Dict) -> ytdl.YoutubeDL:
         """
         Context manager to interact with yt_dlp.
         """
-    download_logger.debug("ytdl_options: %s", str(ytdl_options_overrides))
+        cls.logger.debug("ytdl_options: %s", str(ytdl_options_overrides))
         with Logger.handle_external_logs(name="yt-dlp"):
             # Deep copy ytdl_options in case yt-dlp modifies the dict
             with ytdl.YoutubeDL(copy.deepcopy(ytdl_options_overrides)) as ytdl_downloader:
                 yield ytdl_downloader
 
-
+    @classmethod
-def extract_info(ytdl_options_overrides: Dict, **kwargs) -> Dict:
+    def extract_info(cls, ytdl_options_overrides: Dict, **kwargs) -> Dict:
         """
         Wrapper around yt_dlp.YoutubeDL.YoutubeDL.extract_info
         All kwargs will passed to the extract_info function.
@@ -49,11 +51,12 @@ def extract_info(ytdl_options_overrides: Dict, **kwargs) -> Dict:
         **kwargs
             arguments passed directory to YoutubeDL extract_info
         """
-    with ytdlp_downloader(ytdl_options_overrides) as ytdlp:
+        with cls.ytdlp_downloader(ytdl_options_overrides) as ytdlp:
             return ytdlp.extract_info(**kwargs)
 
-
+    @classmethod
     def extract_info_with_retry(
+        cls,
         ytdl_options_overrides: Dict,
         is_downloaded_fn: Optional[Callable[[], bool]] = None,
         is_thumbnail_downloaded_fn: Optional[Callable[[], bool]] = None,
@@ -86,11 +89,15 @@ def extract_info_with_retry(
         entry_files_exist = False
         copied_ytdl_options_overrides = copy.deepcopy(ytdl_options_overrides)
 
-    while not entry_files_exist and num_tries < _extract_entry_num_retries:
+        while not entry_files_exist and num_tries < cls._EXTRACT_ENTRY_NUM_RETRIES:
-        entry_dict = extract_info(ytdl_options_overrides=copied_ytdl_options_overrides, **kwargs)
+            entry_dict = cls.extract_info(
+                ytdl_options_overrides=copied_ytdl_options_overrides, **kwargs
+            )
 
             is_downloaded = is_downloaded_fn is None or is_downloaded_fn()
-        is_thumbnail_downloaded = is_thumbnail_downloaded_fn is None or is_thumbnail_downloaded_fn()
+            is_thumbnail_downloaded = (
+                is_thumbnail_downloaded_fn is None or is_thumbnail_downloaded_fn()
+            )
 
             if is_downloaded and is_thumbnail_downloaded:
                 return entry_dict
@@ -101,7 +108,7 @@ def extract_info_with_retry(
                 copied_ytdl_options_overrides["skip_download"] = True
                 copied_ytdl_options_overrides["writethumbnail"] = True
 
-        time.sleep(_extract_entry_retry_wait_sec)
+            time.sleep(cls._EXTRACT_ENTRY_RETRY_WAIT_SEC)
             num_tries += 1
 
             # Remove the download archive so it can retry without thinking its already downloaded,
@@ -109,11 +116,11 @@ def extract_info_with_retry(
             if "download_archive" in copied_ytdl_options_overrides:
                 del copied_ytdl_options_overrides["download_archive"]
 
-        if num_tries < _extract_entry_num_retries:
+            if num_tries < cls._EXTRACT_ENTRY_NUM_RETRIES:
-            download_logger.debug(
+                cls.logger.debug(
                     "Failed to download entry. Retrying %d / %d",
                     num_tries,
-                _extract_entry_num_retries,
+                    cls._EXTRACT_ENTRY_NUM_RETRIES,
                 )
 
         error_dict = {"ytdl_options": ytdl_options_overrides, "kwargs": kwargs}
@@ -121,8 +128,8 @@ def extract_info_with_retry(
             f"yt-dlp failed to download an entry with these arguments: {error_dict}"
         )
 
-
+    @classmethod
-def _get_entry_dicts_from_info_json_files(working_directory: str) -> List[Dict]:
+    def _get_entry_dicts_from_info_json_files(cls, working_directory: str) -> List[Dict]:
         """
         Parameters
         ----------
@@ -146,9 +153,11 @@ def _get_entry_dicts_from_info_json_files(working_directory: str) -> List[Dict]:
 
         return entry_dicts
 
-
+    @classmethod
     @contextlib.contextmanager
-def _listen_and_log_downloaded_info_json(working_directory: str, log_prefix: Optional[str]):
+    def _listen_and_log_downloaded_info_json(
+        cls, working_directory: str, log_prefix: Optional[str]
+    ):
         """
         Context manager that starts a separate thread that listens for new .info.json files,
         prints their titles as they appear
@@ -169,8 +178,9 @@ def _listen_and_log_downloaded_info_json(working_directory: str, log_prefix: Opt
         finally:
             info_json_listener.complete = True
 
-
+    @classmethod
     def extract_info_via_info_json(
+        cls,
         working_directory: str,
         ytdl_options_overrides: Dict,
         log_prefix_on_info_json_dl: Optional[str] = None,
@@ -197,21 +207,21 @@ def extract_info_via_info_json(
             arguments passed directory to YoutubeDL extract_info
         """
         try:
-        with _listen_and_log_downloaded_info_json(
+            with cls._listen_and_log_downloaded_info_json(
                 working_directory=working_directory, log_prefix=log_prefix_on_info_json_dl
             ):
-            _ = extract_info(ytdl_options_overrides=ytdl_options_overrides, **kwargs)
+                _ = cls.extract_info(ytdl_options_overrides=ytdl_options_overrides, **kwargs)
         except RejectedVideoReached:
-        download_logger.debug(
+            cls.logger.debug(
                 "RejectedVideoReached, stopping additional downloads "
                 "(Can be disable by setting `ytdl_options.break_on_reject` to False)."
             )
         except ExistingVideoReached:
-        download_logger.debug(
+            cls.logger.debug(
                 "ExistingVideoReached, stopping additional downloads. "
                 "(Can be disable by setting `ytdl_options.break_on_existing` to False)."
             )
         except MaxDownloadsReached:
-        download_logger.info("MaxDownloadsReached, stopping additional downloads.")
+            cls.logger.info("MaxDownloadsReached, stopping additional downloads.")
 
-    return _get_entry_dicts_from_info_json_files(working_directory=working_directory)
+        return cls._get_entry_dicts_from_info_json_files(working_directory=working_directory)

tests/e2e/bandcamp/test_bandcamp.py

@@ -4,6 +4,7 @@ from expected_download import assert_expected_downloads
 from expected_transaction_log import assert_transaction_log_matches
 
 import ytdl_sub.downloaders.downloader
+from ytdl_sub.downloaders.ytdlp import YTDLP
 from ytdl_sub.subscriptions.subscription import Subscription
 
 
@@ -65,7 +66,7 @@ class TestBandcamp:
         # Ensure another invocation will hit ExistingVideoReached
         if not dry_run:
             with assert_logs(
-                logger=ytdl_sub.downloaders.downloader.download_logger,
+                logger=YTDLP.logger,
                 expected_message="ExistingVideoReached, stopping additional downloads",
                 log_level="debug",
             ):

tests/e2e/plugins/test_date_range.py

@@ -7,6 +7,7 @@ from expected_download import assert_expected_downloads
 from expected_transaction_log import assert_transaction_log_matches
 
 import ytdl_sub.downloaders.downloader
+from ytdl_sub.downloaders.ytdlp import YTDLP
 from ytdl_sub.subscriptions.subscription import Subscription
 
 
@@ -68,7 +69,7 @@ class TestDateRange:
         if not dry_run:
             # try downloading again, ensure nothing more was downloaded
             with assert_logs(
-                logger=ytdl_sub.downloaders.downloader.download_logger,
+                logger=YTDLP.logger,
                 expected_message="ExistingVideoReached, stopping additional downloads",
                 log_level="debug",
             ):
@@ -139,7 +140,7 @@ class TestDateRange:
         # First, download recent vids. Always download since we want to test dry-run
         # on the rolling recent portion.
         with assert_logs(
-            logger=ytdl_sub.downloaders.downloader.download_logger,
+            logger=YTDLP.logger,
             expected_message="RejectedVideoReached, stopping additional downloads",
             log_level="debug",
         ):
@@ -159,7 +160,7 @@ class TestDateRange:
         # Then, download the rolling recent vids subscription. This should remove one of the
         # two videos
         with assert_logs(
-            logger=ytdl_sub.downloaders.downloader.download_logger,
+            logger=YTDLP.logger,
             expected_message="ExistingVideoReached, stopping additional downloads",
             log_level="debug",
         ):
@@ -186,7 +187,7 @@ class TestDateRange:
         # existing
         if not dry_run:
             with assert_logs(
-                logger=ytdl_sub.downloaders.downloader.download_logger,
+                logger=YTDLP.logger,
                 expected_message="ExistingVideoReached, stopping additional downloads",
                 log_level="debug",
             ):

tests/e2e/youtube/test_playlist.py

@@ -5,6 +5,7 @@ from expected_download import assert_expected_downloads
 from expected_transaction_log import assert_transaction_log_matches
 
 import ytdl_sub.downloaders.downloader
+from ytdl_sub.downloaders.ytdlp import YTDLP
 from ytdl_sub.subscriptions.subscription import Subscription
 
 
@@ -83,7 +84,7 @@ class TestPlaylist:
         # Ensure another invocation will hit ExistingVideoReached
         if not dry_run:
             with assert_logs(
-                logger=ytdl_sub.downloaders.downloader.download_logger,
+                logger=YTDLP.logger,
                 expected_message="ExistingVideoReached, stopping additional downloads",
                 log_level="debug",
             ):
@@ -131,7 +132,7 @@ class TestPlaylist:
             if not dry_run:
                 # Ensure another invocation will hit ExistingVideoReached
                 with assert_logs(
-                    logger=ytdl_sub.downloaders.downloader.download_logger,
+                    logger=YTDLP.logger,
                     expected_message="ExistingVideoReached, stopping additional downloads",
                     log_level="debug",
                 ):

tests/expected_download.py

@@ -1,6 +1,5 @@
 import json
 import os.path
-import sys
 from dataclasses import dataclass
 from pathlib import Path
 from typing import List

tests/unit/prebuilt_presets/conftest.py

@@ -12,6 +12,7 @@ from resources import copy_file_fixture
 
 from ytdl_sub.config.config_file import ConfigFile
 from ytdl_sub.downloaders.downloader import YtDlpDownloader
+from ytdl_sub.downloaders.ytdlp import YTDLP
 from ytdl_sub.entries.variables.kwargs import DESCRIPTION
 from ytdl_sub.entries.variables.kwargs import EPOCH
 from ytdl_sub.entries.variables.kwargs import EXT
@@ -100,23 +101,22 @@ def mock_entry_dict_factory(mock_downloaded_file_path) -> Callable:
 
 @pytest.fixture
 def mock_download_collection_thumbnail(mock_downloaded_file_path):
-    def _mock_download_thumbnail(output_path: str) -> bool:
+    def _mock_download_and_convert_url_thumbnail(
-        # mock_file_factory(file_name=output_path.split("/")[-1])
+        thumbnail_url: str, output_thumbnail_path: str
-        output_name = os.path.basename(output_path)
+    ) -> bool:
+        _ = thumbnail_url
+        output_name = os.path.basename(output_thumbnail_path)
         if "poster" in output_name or "show" in output_name:
-            copy_file_fixture(fixture_name="poster.jpg", output_file_path=output_path)
+            copy_file_fixture(fixture_name="poster.jpg", output_file_path=output_thumbnail_path)
             return True
         elif "fanart" in output_name:
-            copy_file_fixture(fixture_name="fanart.jpeg", output_file_path=output_path)
+            copy_file_fixture(fixture_name="fanart.jpeg", output_file_path=output_thumbnail_path)
             return True
         return False
 
-    with patch.object(
+    with patch(
-        YtDlpDownloader,
+        "ytdl_sub.downloaders.downloader.download_and_convert_url_thumbnail",
-        "_download_thumbnail",
+        new=_mock_download_and_convert_url_thumbnail,
-        new=lambda _, thumbnail_url, output_thumbnail_path: _mock_download_thumbnail(
-            output_thumbnail_path
-        ),
     ):
         yield  # TODO: create file here
 
@@ -126,11 +126,9 @@ def mock_download_collection_entries(
     mock_download_collection_thumbnail, mock_entry_dict_factory: Callable, working_directory: str
 ):
     @contextlib.contextmanager
-    def _mock_download_collection_entries_factory(is_youtube_channel: bool):
+    def _mock_download_collection_entries_factory(is_youtube_channel: bool, num_urls: int = 1):
         def _write_entries_to_working_dir(*args, **kwargs) -> List[Dict]:
-            if (len(args[0].collection.urls.list) == 1) or (
+            if num_urls == 1 or ("season.2" in kwargs["url"] and num_urls > 1):
-                "season.2" in kwargs["url"] and len(args[0].download_options.urls.list) > 1
-            ):
                 return [
                     mock_entry_dict_factory(
                         uid="21-1",
@@ -202,7 +200,7 @@ def mock_download_collection_entries(
             ]
 
         with patch.object(
-            YtDlpDownloader, "extract_info_via_info_json", new=_write_entries_to_working_dir
+            YTDLP, "extract_info_via_info_json", new=_write_entries_to_working_dir
         ), patch.object(
             YtDlpDownloader, "_extract_entry_info_with_retry", new=lambda _, entry: entry
         ):

tests/unit/prebuilt_presets/test_prebuilt_presets.py

@@ -215,7 +215,9 @@ class TestPrebuiltTvShowCollectionPresets:
             },
         )
 
-        with mock_download_collection_entries(is_youtube_channel=is_youtube_channel):
+        with mock_download_collection_entries(
+            is_youtube_channel=is_youtube_channel, num_urls=len(season_indices)
+        ):
             transaction_log = subscription.download(dry_run=False)
 
         assert_transaction_log_matches(