Refactor the Scheduler

- Improve the exception handling when loading the scheduled post
- Temporarily ignore the safety warning about Sphinx

CHANGELOG.md

@@ -3,6 +3,11 @@
 We follow [Semantic Versions](https://semver.org/).
 
 
+## Version 0.1.6  (unreleased)
+
+- Refactor the `Scheduler` to improve the exception handling when loading the scheduled post
+
+
 ## Version 0.1.5
 
 - Implement the post submission to the `RedditCutifier`

Makefile

@@ -14,7 +14,7 @@ unit:
 package:
 	poetry check
 	pip check
-	safety check --bare --full-report
+	safety check --bare --full-report --ignore=38330
 
 .PHONY: test
 test: lint unit package

slow_start_rewatch/scheduler.py

@@ -6,7 +6,7 @@
 from typing import Optional
 
 import click
-from ruamel.yaml import YAML  # type: ignore
+from ruamel.yaml import YAML, YAMLError  # type: ignore
 from structlog import get_logger
 
 from slow_start_rewatch.config import ROOT_DIR, Config
@@ -53,12 +53,6 @@ def load(self, username: str) -> None:
                     self.scheduled_post_file,
                 ),
             )
-        except (KeyError, AttributeError) as error:
-            log.exception("scheduled_post_invalid")
-            raise InvalidSchedule(
-                "Failed to parse the data about the scheduled post.",
-                hint="Make sure all the fields are filled in.",
-            ) from error
 
         self.scheduled_post = post
 
@@ -78,14 +72,32 @@ def parse_post_from_yaml(self, path: str) -> Post:
         yaml = YAML(typ="safe")
 
         with open(path) as post_file:
-            yaml_data = yaml.load(post_file.read())
+            yaml_content = post_file.read()
 
-        return Post(
-            submit_at=yaml_data["submit_at"],
-            subreddit=yaml_data["subreddit"],
-            title=yaml_data["title"],
-            body=yaml_data["body"],
-        )
+        try:
+            yaml_data = yaml.load(yaml_content)
+        except (YAMLError, AttributeError) as yaml_error:
+            log.exception("scheduled_post_invalid")
+            raise InvalidSchedule(
+                "Failed to parse the data about the scheduled post.",
+                hint="Repair the structure of the scheduled post file.",
+            ) from yaml_error
+
+        try:
+            post = Post(
+                submit_at=yaml_data["submit_at"],
+                subreddit=yaml_data["subreddit"],
+                title=yaml_data["title"],
+                body=yaml_data["body"],
+            )
+        except (AttributeError, KeyError) as missing_data_error:
+            log.exception("scheduled_post_incomplete")
+            raise InvalidSchedule(
+                "Incomplete scheduled post data.",
+                hint="Make sure all the fields are filled in.",
+            ) from missing_data_error
+
+        return post
 
     def create_default(self, username: str) -> None:
         """
@@ -97,17 +109,19 @@ def create_default(self, username: str) -> None:
         with open(DEFAULT_SCHEDULED_POST_FILE) as default_post_file:
             yaml_template = Template(default_post_file.read())
 
-        sample_time = datetime.now() + timedelta(minutes=DEFAULT_DELAY)
+        post_attributes = {
+            "submit_at": (
+                datetime.now() + timedelta(minutes=DEFAULT_DELAY)
+            ).isoformat(" ", "seconds"),
+            "subreddit": "u_{0}".format(username),
+        }
 
-        yaml_content = yaml_template.substitute(
-            submit_at=sample_time.isoformat(" ", "seconds"),
-            subreddit="u_{0}".format(username),
-        )
+        yaml_content = yaml_template.substitute(post_attributes)
 
         log.info(
             "scheduled_post_create_default",
-            submit_at=sample_time.isoformat(" ", "seconds"),
-            subreddit="u_{0}".format(username),
+            submit_at=post_attributes["submit_at"],
+            subreddit=post_attributes["subreddit"],
             path=self.scheduled_post_file,
         )
 

tests/test_scheduler.py

@@ -2,20 +2,35 @@
 
 import os
 import shutil
+from datetime import datetime, timedelta
 from pathlib import Path
+from unittest.mock import patch
 
 import pytest
+from ruamel.yaml import YAML  # type: ignore
 
 from slow_start_rewatch.exceptions import InvalidSchedule, MissingSchedule
 from slow_start_rewatch.scheduler import Scheduler
 from tests.conftest import TEST_ROOT_DIR, MockConfig
 
 TEST_SCHEDULER_PATH = Path(TEST_ROOT_DIR).joinpath("test_scheduler")
+VALID_POST_FILENAME = "scheduled_post.yml"
+INVALID_POST_FILENAME = "scheduled_post_invalid.yml"
+INCOMPLETE_POST_FILENAME = "scheduled_post_incomplete.yml"
 
 
-def test_loading_existing_file(scheduler_config_valid, post):
-    """Test loading the Post when configured with a valid file."""
-    scheduler = Scheduler(scheduler_config_valid)
+@patch.object(Scheduler, "parse_post_from_yaml")
+def test_load(  # noqa: WPS211
+    mock_parse_post_from_yaml,
+    scheduler_config,
+    reddit,
+    post,
+):
+    """Test loading of the scheduled post."""
+    post.submit_at = datetime.now() + timedelta(minutes=1)
+    mock_parse_post_from_yaml.return_value = post
+
+    scheduler = Scheduler(scheduler_config)
 
     assert not scheduler.scheduled_post
 
@@ -24,67 +39,125 @@ def test_loading_existing_file(scheduler_config_valid, post):
     assert scheduler.scheduled_post == post
 
 
-def test_loading_invalid_file(scheduler_config_invalid):
-    """Test loading the Post when configured with a invalid file."""
-    scheduler = Scheduler(scheduler_config_invalid)
+@patch.object(Scheduler, "create_default")
+@patch.object(Scheduler, "parse_post_from_yaml")
+def test_load_with_exception(  # noqa: WPS211
+    mock_parse_post_from_yaml,
+    mock_create_default,
+    scheduler_config,
+    reddit,
+    post,
+):
+    """Test loading of the scheduled post with error."""
+    mock_parse_post_from_yaml.side_effect = FileNotFoundError
 
-    assert not scheduler.scheduled_post
+    scheduler = Scheduler(scheduler_config)
 
-    with pytest.raises(InvalidSchedule):
+    with pytest.raises(MissingSchedule):
         scheduler.load("cute_tester")
 
+    assert mock_create_default.call_count == 1
 
-def test_loading_nonexistent_file(scheduler_config_missing):
+
+def test_parse_post_from_yaml(
+    scheduler_config,
+    reddit,
+    post,
+    post_files,
+):
     """
-    Test loading the Post when the source file is missing.
+    Test parsing of the scheduled post from YAML.
+
+    1. A valid file
 
-    When the source file is missing Scheduler should create a sample file.
+    2. An invalid file that cannot be parsed
+
+    3. A file with missing fields
     """
-    scheduler = Scheduler(scheduler_config_missing)
+    scheduler = Scheduler(scheduler_config)
 
-    scheduled_post_file = scheduler_config_missing["scheduled_post_file"]
+    assert scheduler.parse_post_from_yaml(
+        post_files[VALID_POST_FILENAME],
+    ) == post
 
-    assert not os.path.exists(scheduled_post_file)
+    with pytest.raises(InvalidSchedule) as invalid_error:
+        scheduler.parse_post_from_yaml(
+            post_files[INVALID_POST_FILENAME],
+        )
 
-    with pytest.raises(MissingSchedule):
-        scheduler.load("cute_tester")
+    # Comply with PT012: https://pypi.org/project/flake8-pytest-style/
+    assert "Failed to parse" in str(invalid_error.value)  # noqa: WPS441
 
-    with open(scheduled_post_file, "r") as sample_post_file:
-        yaml_content = sample_post_file.read()
+    with pytest.raises(InvalidSchedule) as incomplete_error:
+        scheduler.parse_post_from_yaml(
+            post_files[INCOMPLETE_POST_FILENAME],
+        )
 
-    assert "cute_tester" in yaml_content
-    assert "${" not in yaml_content
+    assert "Incomplete" in str(incomplete_error.value)  # noqa: WPS441
 
 
-@pytest.fixture()
-def scheduler_config_valid(tmpdir):
-    """Return mock Config contaning a path to a valid Scheduled Post."""
+def test_create_default(
+    reddit,
+    tmpdir,
+):
+    """
+    Test creating a sample source file.
+
+    Check that all placeholders are substituted.
+
+    Check the parsed post contains the expected fields.
+    """
     scheduled_post_path = tmpdir.join("scheduled_post.yml")
 
-    shutil.copyfile(
-        TEST_SCHEDULER_PATH.joinpath("scheduled_post.yml"),
-        scheduled_post_path,
+    scheduler = Scheduler(
+        MockConfig({"scheduled_post_file": str(scheduled_post_path)}),
     )
 
-    return MockConfig({"scheduled_post_file": str(scheduled_post_path)})
+    assert not os.path.exists(scheduled_post_path)
+
+    scheduler.create_default("cute_tester")
+
+    with open(scheduled_post_path, "r") as scheduled_post_file:
+        yaml_content = scheduled_post_file.read()
+
+    assert "${" not in yaml_content
+
+    yaml_data = YAML(typ="safe").load(yaml_content)
+
+    assert yaml_data["subreddit"] == "u_cute_tester"
+    assert yaml_data["submit_at"] > datetime.now()
 
 
 @pytest.fixture()
-def scheduler_config_invalid(tmpdir):
-    """Return mock Config contaning a path to an invalid Scheduled Post."""
-    scheduled_post_path = tmpdir.join("scheduled_post.yml")
+def post_files(tmpdir):
+    """Create temporary post YAML files and return their paths."""
+    file_names = [
+        VALID_POST_FILENAME,
+        INVALID_POST_FILENAME,
+        INCOMPLETE_POST_FILENAME,
+    ]
 
-    shutil.copyfile(
-        TEST_SCHEDULER_PATH.joinpath("scheduled_post_invalid.yml"),
-        scheduled_post_path,
-    )
+    scheduled_post_paths = {}
 
-    return MockConfig({"scheduled_post_file": str(scheduled_post_path)})
+    for file_name in file_names:
+        scheduled_post_path = tmpdir.join(file_name)
+        shutil.copyfile(
+            TEST_SCHEDULER_PATH.joinpath(file_name),
+            scheduled_post_path,
+        )
+        scheduled_post_paths[file_name] = scheduled_post_path
+
+    return scheduled_post_paths
 
 
 @pytest.fixture()
-def scheduler_config_missing(tmpdir):
-    """Return mock Config contaning a path to a missing Scheduled Post."""
-    scheduled_post_path = tmpdir.join("scheduled_post.yml")
+def scheduler_config(tmpdir):
+    """Return mock Config contaning a path to a valid scheduled post."""
+    scheduled_post_path = tmpdir.join(VALID_POST_FILENAME)
+
+    shutil.copyfile(
+        TEST_SCHEDULER_PATH.joinpath(VALID_POST_FILENAME),
+        scheduled_post_path,
+    )
 
     return MockConfig({"scheduled_post_file": str(scheduled_post_path)})

tests/test_scheduler/scheduled_post_incomplete.yml

@@ -0,0 +1,28 @@
+# Left out to cause a validation error:
+# submit_at: 2018-01-06 12:00:00
+
+subreddit: anime
+
+title: "Slow Start - Episode 1 Discussion"
+
+body: |
+  *Slow Start*, Episode 1: The First Butterflies
+
+  ---
+
+  **Streams:**
+
+  * [Crunchyroll](https://www.crunchyroll.com/slow-start/episode-1-the-first-butterflies-759027)
+  * [VRV](https://vrv.co/watch/G69P1KQ0Y/Slow-Start:The-First-Butterflies)
+
+  ---
+
+  **Show Information:**
+
+  * [MyAnimeList](https://myanimelist.net/anime/35540)
+  * [AniDB](http://anidb.net/perl-bin/animedb.pl?show=anime&aid=13160)
+  * [AniList](https://anilist.co/anime/98693/SlowStart)
+  * [Official Website](http://slow-start.com)
+  * [US Website](http://slowstart-usa.com/)
+  * [Twitter](https://twitter.com/slosta_anime)
+  * [US Facebook Page](https://www.facebook.com/SlowStartUSA/)

tests/test_scheduler/scheduled_post_invalid.yml

@@ -1,27 +1,28 @@
-submit_at: # Left empty to cause a validation error
+submit_at: 2018-01-06 12:00:00
 
 subreddit: anime
 
 title: "Slow Start - Episode 1 Discussion"
 
+# Removed indentation to cause YAML parsing error:
 body: |
-  *Slow Start*, Episode 1: The First Butterflies
+*Slow Start*, Episode 1: The First Butterflies
 
-  ---
+---
 
-  **Streams:**
+**Streams:**
 
-  * [Crunchyroll](https://www.crunchyroll.com/slow-start/episode-1-the-first-butterflies-759027)
-  * [VRV](https://vrv.co/watch/G69P1KQ0Y/Slow-Start:The-First-Butterflies)
+* [Crunchyroll](https://www.crunchyroll.com/slow-start/episode-1-the-first-butterflies-759027)
+* [VRV](https://vrv.co/watch/G69P1KQ0Y/Slow-Start:The-First-Butterflies)
 
-  ---
+---
 
-  **Show Information:**
+**Show Information:**
 
-  * [MyAnimeList](https://myanimelist.net/anime/35540)
-  * [AniDB](http://anidb.net/perl-bin/animedb.pl?show=anime&aid=13160)
-  * [AniList](https://anilist.co/anime/98693/SlowStart)
-  * [Official Website](http://slow-start.com)
-  * [US Website](http://slowstart-usa.com/)
-  * [Twitter](https://twitter.com/slosta_anime)
-  * [US Facebook Page](https://www.facebook.com/SlowStartUSA/)
+* [MyAnimeList](https://myanimelist.net/anime/35540)
+* [AniDB](http://anidb.net/perl-bin/animedb.pl?show=anime&aid=13160)
+* [AniList](https://anilist.co/anime/98693/SlowStart)
+* [Official Website](http://slow-start.com)
+* [US Website](http://slowstart-usa.com/)
+* [Twitter](https://twitter.com/slosta_anime)
+* [US Facebook Page](https://www.facebook.com/SlowStartUSA/)