test and readthedocs

docs/config.rst

@@ -29,7 +29,22 @@ and subscriptions.
 .. autoclass:: ytdl_sub.config.config_validator.ConfigOptions()
   :members:
   :member-order: bysource
+  :exclude-members: persist_logs
 
+persist_logs
+""""""""""""
+Within ``configuration``, define whether logs from subscription downloads
+should be persisted.
+
+.. code-block:: yaml
+
+   configuration:
+     persist_logs:
+       logs_directory: "/path/to/log/directory"
+
+.. autoclass:: ytdl_sub.config.config_validator.PersistLogsValidator()
+  :members:
+  :member-order: bysource
 
 presets
 ^^^^^^^

src/ytdl_sub/cli/main.py

@@ -1,4 +1,3 @@
-import argparse
 import gc
 import sys
 from datetime import datetime
@@ -26,15 +25,24 @@ _VIEW_EXTRA_ARGS_FORMATTER = "--preset _view --overrides.url {}"
 
 
 def _maybe_write_subscription_log_file(
-    config: ConfigFile, subscription: Subscription, success: bool
+    config: ConfigFile,
-) -> Optional[Path]:
+    subscription: Subscription,
+    dry_run: bool,
+    exception: Optional[Exception] = None,
+) -> None:
+    success: bool = exception is None
+
+    # If dry-run, do nothing
+    if dry_run:
+        return
+
     # If persisting logs is disabled, do nothing
     if not config.config_options.persist_logs:
-        return None
+        return
 
     # If persisting successful logs is disabled, do nothing
     if success and not config.config_options.persist_logs.keep_successful_logs:
-        return None
+        return
 
     log_time = datetime.now().strftime("%Y-%m-%d-%H%M%S")
     log_subscription_name = sanitize_filename(subscription.name).lower().replace(" ", "_")
@@ -43,12 +51,14 @@ def _maybe_write_subscription_log_file(
     log_filename = f"{log_time}.{log_subscription_name}.{log_success}.log"
     persist_log_path = Path(config.config_options.persist_logs.logs_directory) / log_filename
 
+    if not success:
+        Logger.log_exit_exception(exception=exception, log_filepath=persist_log_path)
+
     FileHandler.copy(Logger.debug_log_filename(), persist_log_path)
-    return persist_log_path
 
 
 def _download_subscriptions_from_yaml_files(
-    config: ConfigFile, args: argparse.Namespace
+    config: ConfigFile, subscription_paths: List[str], dry_run: bool
 ) -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
     """
     Downloads all subscriptions from one or many subscription yaml files.
@@ -57,8 +67,10 @@ def _download_subscriptions_from_yaml_files(
     ----------
     config
         Configuration file
-    args
+    subscription_paths
-        Arguments from argparse
+        Path to subscription files to download
+    dry_run
+        Whether to dry run or not
 
     Returns
     -------
@@ -69,7 +81,6 @@ def _download_subscriptions_from_yaml_files(
     Exception
         Any exception during download
     """
-    subscription_paths: List[str] = args.subscription_paths
     subscriptions: List[Subscription] = []
     output: List[Tuple[Subscription, FileHandlerTransactionLog]] = []
 
@@ -80,23 +91,22 @@ def _download_subscriptions_from_yaml_files(
     for subscription in subscriptions:
         logger.info(
             "Beginning subscription %s for %s",
-            ("dry run" if args.dry_run else "download"),
+            ("dry run" if dry_run else "download"),
             subscription.name,
         )
         logger.debug("Subscription full yaml:\n%s", subscription.as_yaml())
 
         try:
-            transaction_log = subscription.download(dry_run=args.dry_run)
+            transaction_log = subscription.download(dry_run=dry_run)
         except Exception as exc:  # pylint: disable=broad-except
-            persisted_log_path = _maybe_write_subscription_log_file(
+            _maybe_write_subscription_log_file(
-                config=config, subscription=subscription, success=False
+                config=config, subscription=subscription, dry_run=dry_run, exception=exc
             )
-            Logger.log_exit_exception(exception=exc, log_filepath=persisted_log_path)
             raise
         else:
             output.append((subscription, transaction_log))
             _maybe_write_subscription_log_file(
-                config=config, subscription=subscription, success=True
+                config=config, subscription=subscription, dry_run=dry_run
             )
             Logger.cleanup()  # Cleanup logger after each successful subscription download
         finally:
@@ -177,7 +187,11 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
 
     with working_directory_lock(config=config):
         if args.subparser == "sub":
-            transaction_logs = _download_subscriptions_from_yaml_files(config=config, args=args)
+            transaction_logs = _download_subscriptions_from_yaml_files(
+                config=config,
+                subscription_paths=args.subscription_paths,
+                dry_run=args.dry_run,
+            )
 
         # One-off download
         elif args.subparser == "dl":

src/ytdl_sub/config/config_file.py

@@ -1,5 +1,6 @@
 import os
 from typing import Any
+from typing import Dict
 
 from ytdl_sub.config.config_validator import ConfigValidator
 from ytdl_sub.config.preset import Preset
@@ -65,3 +66,11 @@ class ConfigFile(ConfigValidator):
         """
         config_dict = load_yaml(file_path=config_path)
         return ConfigFile.from_dict(config_dict)
+
+    def as_dict(self) -> Dict[str, Any]:
+        """
+        Returns
+        -------
+        The config in its dict form
+        """
+        return self._value

src/ytdl_sub/config/config_validator.py

@@ -48,14 +48,31 @@ class PersistLogsValidator(StrictDictValidator):
 
     @property
     def logs_directory(self) -> str:
+        """
+        Required. The directory to store the logs in.
+        """
         return self._logs_directory.value
 
     @property
     def keep_logs_after(self) -> Optional[str]:
+        """
+        Optional. Keep logs after this date, in yt-dlp datetime format.
+
+        .. code-block:: Markdown
+
+           A string in the format YYYYMMDD or
+           (now|today|yesterday|date)[+-][0-9](microsecond|second|minute|hour|day|week|month|year)(s)
+
+        For example, ``today-1week`` means keep 1 week's worth of logs. By default, ytdl-sub will
+        keep all log files.
+        """
         return self._keep_logs_after
 
     @property
     def keep_successful_logs(self) -> bool:
+        """
+        Optional. Whether to store logs when downloading is successful. Defaults to True.
+        """
         return self._keep_successful_logs.value
 
 
@@ -142,7 +159,9 @@ class ConfigOptions(StrictDictValidator):
 
     @property
     def persist_logs(self) -> Optional[PersistLogsValidator]:
-        # TODO: nested docstring???
+        """
+        Persist logs validator. readthedocs in the validator itself!
+        """
         return self._persist_logs
 
     @property

tests/conftest.py

@@ -139,7 +139,7 @@ def preset_dict_to_subscription_yaml_generator() -> Callable:
 # Example config fixtures
 
 
-def _load_config(config_path: Path, working_directory: Path) -> ConfigFile:
+def _load_config(config_path: Path, working_directory: str) -> ConfigFile:
     config_dict = load_yaml(file_path=config_path)
     config_dict["configuration"]["working_directory"] = working_directory
 
@@ -156,6 +156,11 @@ def music_video_config(music_video_config_path, working_directory) -> ConfigFile
     return _load_config(music_video_config_path, working_directory)
 
 
+@pytest.fixture()
+def music_video_subscription_path() -> Path:
+    return Path("examples/music_videos_subscriptions.yaml")
+
+
 @pytest.fixture()
 def channel_as_tv_show_config(working_directory) -> ConfigFile:
     return _load_config(

tests/unit/cli/test_main.py

@@ -1,24 +1,18 @@
-import sys
+import re
 import tempfile
+import time
+from pathlib import Path
+from typing import Callable
 from unittest.mock import patch
 
+import mergedeep
 import pytest
 
-from ytdl_sub.cli.main import main
+from ytdl_sub.cli.main import _download_subscriptions_from_yaml_files
-
+from ytdl_sub.config.config_file import ConfigFile
-
+from ytdl_sub.subscriptions.subscription import Subscription
-def test_args_after_sub_work():
+from ytdl_sub.utils.file_handler import FileHandlerTransactionLog
-    with patch.object(
+from ytdl_sub.utils.logger import Logger
-        sys,
-        "argv",
-        ["ytdl-sub", "-c", "examples/tv_show_config.yaml", "sub", "--log-level", "debug"],
-    ), patch("ytdl_sub.cli.main._download_subscriptions_from_yaml_files") as mock_sub:
-        main()
-
-        assert mock_sub.call_count == 1
-        assert mock_sub.call_args.kwargs["args"].config == "examples/tv_show_config.yaml"
-        assert mock_sub.call_args.kwargs["args"].subscription_paths == ["subscriptions.yaml"]
-        assert mock_sub.call_args.kwargs["args"].ytdl_sub_log_level == "debug"
 
 
 @pytest.fixture
@@ -28,21 +22,104 @@ def persist_logs_directory():
 
 
 @pytest.fixture
-def persist_logs_config():
+def persist_logs_config_factory(
-    pass
+    music_video_config: ConfigFile, persist_logs_directory: str
+) -> Callable:
+    def _persist_logs_config_factory(keep_successful_logs: bool) -> ConfigFile:
+        return ConfigFile.from_dict(
+            dict(
+                mergedeep.merge(
+                    music_video_config.as_dict(),
+                    {
+                        "configuration": {
+                            "persist_logs": {
+                                "logs_directory": persist_logs_directory,
+                                "keep_successful_logs": keep_successful_logs,
+                            },
+                        }
+                    },
+                )
+            )
+        )
+
+    return _persist_logs_config_factory
+
+
+@pytest.fixture
+def mock_subscription_download_factory():
+    def _mock_subscription_download_factory(mock_success_output: bool) -> Callable:
+        def _mock_download(self: Subscription, dry_run: bool) -> FileHandlerTransactionLog:
+            Logger.get().info(
+                "name=%s success=%s dry_run=%s", self.name, mock_success_output, dry_run
+            )
+            time.sleep(1)
+            if not mock_success_output:
+                raise ValueError("error")
+            return FileHandlerTransactionLog()
+
+        return _mock_download
+
+    return _mock_subscription_download_factory
 
 
 class TestPersistLogs:
+    @pytest.mark.parametrize("dry_run", [True, False])
     @pytest.mark.parametrize("mock_success_output", [True, False])
     @pytest.mark.parametrize("keep_successful_logs", [True, False])
     def test_subscription_logs_write_to_file(
-        self, persist_logs_directory: str, mock_success_output: bool, keep_successful_logs: bool
+        self,
+        persist_logs_directory: str,
+        persist_logs_config_factory: Callable,
+        mock_subscription_download_factory: Callable,
+        music_video_subscription_path: Path,
+        dry_run: bool,
+        mock_success_output: bool,
+        keep_successful_logs: bool,
     ):
+        num_subscriptions = 2
+        config = persist_logs_config_factory(keep_successful_logs=keep_successful_logs)
+        subscription_paths = [str(music_video_subscription_path)] * num_subscriptions
 
-        config_dict = {
+        with patch.object(
-            "working_directory": ".",
+            Subscription,
-            "persist_logs": {
+            "download",
-                "logs_directory": persist_logs_directory,
+            new=mock_subscription_download_factory(mock_success_output=mock_success_output),
-                "keep_successful_logs": keep_successful_logs,
+        ):
-            },
+            try:
-        }
+                _download_subscriptions_from_yaml_files(
+                    config=config, subscription_paths=subscription_paths, dry_run=dry_run
+                )
+            except ValueError:
+                assert not mock_success_output
+
+        log_directory_files = list(Path(persist_logs_directory).rglob("*"))
+
+        # If dry run or success but success logging disabled, expect 0 log files
+        if dry_run or (mock_success_output and not keep_successful_logs):
+            assert len(log_directory_files) == 0
+            return
+        # If not success, expect 1 log file
+        elif not mock_success_output:
+            assert len(log_directory_files) == 1
+            log_path = log_directory_files[0]
+            assert bool(re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.error\.log", log_path.name))
+            with open(log_path, "r", encoding="utf-8") as log_file:
+                assert log_file.readlines()[-1] == (
+                    f"Please upload the error log file '{str(log_path)}' and make a Github issue "
+                    f"at https://github.com/jmbannon/ytdl-sub/issues with your config and "
+                    f"command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!\n"
+                )
+        # If success and success logging, expect 3 log files
+        else:
+            assert len(log_directory_files) == num_subscriptions
+            for log_file_path in log_directory_files:
+                assert bool(
+                    re.match(
+                        r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.success\.log", log_file_path.name
+                    )
+                )
+                with open(log_file_path, "r", encoding="utf-8") as log_file:
+                    assert (
+                        log_file.readlines()[-1]
+                        == "[ytdl-sub] name=john_smith success=True dry_run=False\n"
+                    )

tests/unit/main/test_main.py

@@ -9,6 +9,7 @@ from src.ytdl_sub import __local_version__
 from src.ytdl_sub.main import main
 from ytdl_sub.utils.exceptions import ValidationException
 from ytdl_sub.utils.logger import Logger
+from ytdl_sub.utils.logger import LoggerLevels
 
 
 @pytest.fixture
@@ -66,3 +67,16 @@ def test_main_uncaught_error(capsys, mock_sys_exit, expected_uncaught_error_mess
     assert mock_error.call_args.args[0] == expected_uncaught_error_message
     assert mock_error.call_args.args[1] == __local_version__
     assert mock_error.call_args.args[2] == Logger.debug_log_filename()
+
+
+def test_args_after_sub_work(mock_sys_exit):
+    with mock_sys_exit(expected_exit_code=0), patch.object(
+        sys,
+        "argv",
+        ["ytdl-sub", "-c", "examples/tv_show_config.yaml", "sub", "--log-level", "verbose"],
+    ), patch("ytdl_sub.cli.main._download_subscriptions_from_yaml_files") as mock_sub:
+        main()
+
+        assert mock_sub.call_count == 1
+        assert mock_sub.call_args.kwargs["subscription_paths"] == ["subscriptions.yaml"]
+        assert Logger._LOGGER_LEVEL == LoggerLevels.VERBOSE