From 14dcb78986c6ab6d3284d717a7ef2951fef446b3 Mon Sep 17 00:00:00 2001 From: vaclav-2012 <38167104+vaclav-2012@users.noreply.github.com> Date: Sun, 14 Jun 2020 03:22:10 +0200 Subject: [PATCH] Refactor the `Scheduler` - Improve the exception handling when loading the scheduled post - Temporarily ignore the safety warning about Sphinx --- CHANGELOG.md | 5 + Makefile | 2 +- slow_start_rewatch/scheduler.py | 56 ++++--- tests/test_scheduler.py | 149 +++++++++++++----- .../scheduled_post_incomplete.yml | 28 ++++ .../test_scheduler/scheduled_post_invalid.yml | 31 ++-- 6 files changed, 196 insertions(+), 75 deletions(-) create mode 100644 tests/test_scheduler/scheduled_post_incomplete.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 954f6d9..dece1b8 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/Makefile b/Makefile index 5860941..0d3d3d7 100644 --- a/Makefile +++ b/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 diff --git a/slow_start_rewatch/scheduler.py b/slow_start_rewatch/scheduler.py index 4ccff58..29205a7 100644 --- a/slow_start_rewatch/scheduler.py +++ b/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, ) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 18e40ab..b2ae80a 100644 --- a/tests/test_scheduler.py +++ b/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)}) diff --git a/tests/test_scheduler/scheduled_post_incomplete.yml b/tests/test_scheduler/scheduled_post_incomplete.yml new file mode 100644 index 0000000..73217e5 --- /dev/null +++ b/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/) diff --git a/tests/test_scheduler/scheduled_post_invalid.yml b/tests/test_scheduler/scheduled_post_invalid.yml index 9bce6e5..009c5f9 100644 --- a/tests/test_scheduler/scheduled_post_invalid.yml +++ b/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/)