From 23df17b97e61fec8e4df4506a45cb257dfd33b93 Mon Sep 17 00:00:00 2001 From: Cody Logan Date: Wed, 1 Nov 2023 09:52:03 -0700 Subject: Refactor download processing code and add some tests --- tests/test_dl.py | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/test_dl.py b/tests/test_dl.py index 69ff2bb..61efbe0 100644 --- a/tests/test_dl.py +++ b/tests/test_dl.py @@ -16,10 +16,12 @@ # along with Wikiget. If not, see . from pathlib import Path +from unittest.mock import MagicMock, patch import pytest -from wikiget.dl import prep_download +from wikiget.dl import prep_download, process_download +from wikiget.file import File from wikiget.wikiget import construct_parser @@ -44,3 +46,63 @@ class TestPrepDownload: args = construct_parser().parse_args(["File:Example.jpg", "-o", str(tmp_file)]) with pytest.raises(FileExistsError): _ = prep_download(args.FILE, args) + + +class TestProcessDownload: + @patch("wikiget.dl.batch_download") + def test_batch_download(self, mock_batch_download: MagicMock) -> None: + """ + A successful batch download should not return any errors. + """ + args = construct_parser().parse_args(["-a", "batch.txt"]) + mock_batch_download.return_value = 0 + process_download(args) + assert mock_batch_download.called + + @patch("wikiget.dl.batch_download") + def test_batch_download_with_errors( + self, mock_batch_download: MagicMock, caplog: pytest.LogCaptureFixture + ) -> None: + """ + Any errors during batch download should create a log message containing the + number of errors and result in a non-zero exit code. + """ + args = construct_parser().parse_args(["-a", "batch.txt"]) + mock_batch_download.return_value = 4 + with pytest.raises(SystemExit) as e: + process_download(args) + assert mock_batch_download.called + assert e.value.code == 1 + assert "4 problems encountered during batch processing" in caplog.text + + @patch("wikiget.dl.prep_download") + @patch("wikiget.dl.download") + def test_single_download( + self, mock_download: MagicMock, mock_prep_download: MagicMock + ) -> None: + """ + A successful download should not return any errors. + """ + args = construct_parser().parse_args(["File:Example.jpg"]) + mock_download.return_value = 0 + mock_prep_download.return_value = MagicMock(File) + process_download(args) + assert mock_prep_download.called + assert mock_download.called + + @patch("wikiget.dl.prep_download") + @patch("wikiget.dl.download") + def test_single_download_with_errors( + self, mock_download: MagicMock, mock_prep_download: MagicMock + ) -> None: + """ + Any errors during download should result in a non-zero exit code. + """ + args = construct_parser().parse_args(["File:Example.jpg"]) + mock_download.return_value = 1 + mock_prep_download.return_value = MagicMock(File) + with pytest.raises(SystemExit) as e: + process_download(args) + assert mock_prep_download.called + assert mock_download.called + assert e.value.code == 1 -- cgit v1.2.3 From 8b923f0cee8fed90d6eef313429eb593877e0f2d Mon Sep 17 00:00:00 2001 From: Cody Logan Date: Wed, 1 Nov 2023 11:32:53 -0700 Subject: Test connect_to_site using mocking --- tests/test_client.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/test_client.py b/tests/test_client.py index 41ce948..832bf21 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -16,6 +16,7 @@ # along with Wikiget. If not, see . import logging +from unittest.mock import MagicMock, patch import pytest @@ -25,15 +26,19 @@ from wikiget.wikiget import construct_parser # TODO: don't hit the actual API when doing tests -@pytest.mark.skip(reason="skip tests that query a live API") class TestQueryApi: - args = construct_parser().parse_args(["File:Example.jpg"]) - - def test_connect_to_site(self, caplog: pytest.LogCaptureFixture) -> None: + @patch("mwclient.Site.__new__") + def test_connect_to_site( + self, mock_site: MagicMock, caplog: pytest.LogCaptureFixture + ) -> None: caplog.set_level(logging.DEBUG) - _ = connect_to_site("commons.wikimedia.org", self.args) + mock_site.return_value = MagicMock() + args = construct_parser().parse_args(["File:Example.jpg"]) + _ = connect_to_site("commons.wikimedia.org", args) + assert mock_site.called assert "Connecting to commons.wikimedia.org" in caplog.text + @pytest.mark.skip(reason="skip tests that query a live API") def test_query_api(self, caplog: pytest.LogCaptureFixture) -> None: caplog.set_level(logging.DEBUG) site = connect_to_site("commons.wikimedia.org", self.args) -- cgit v1.2.3 From f61853c366a1554f545b34783e1aa282022598b9 Mon Sep 17 00:00:00 2001 From: Cody Logan Date: Wed, 1 Nov 2023 11:33:13 -0700 Subject: Add some explanatory comments to tests --- tests/test_client.py | 11 +++++++++- tests/test_file_class.py | 12 +++++++++++ tests/test_logging.py | 18 ++++++++++++++++ tests/test_parse.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/test_client.py b/tests/test_client.py index 832bf21..cf6e29c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -31,6 +31,10 @@ class TestQueryApi: def test_connect_to_site( self, mock_site: MagicMock, caplog: pytest.LogCaptureFixture ) -> None: + """ + The connect_to_site function should create a debug log message recording the + name of the site we're connecting to. + """ caplog.set_level(logging.DEBUG) mock_site.return_value = MagicMock() args = construct_parser().parse_args(["File:Example.jpg"]) @@ -40,7 +44,12 @@ class TestQueryApi: @pytest.mark.skip(reason="skip tests that query a live API") def test_query_api(self, caplog: pytest.LogCaptureFixture) -> None: + """ + The query_api function should create a debug log message containing the user + agent we're sending to the API. + """ caplog.set_level(logging.DEBUG) - site = connect_to_site("commons.wikimedia.org", self.args) + args = construct_parser().parse_args(["File:Example.jpg"]) + site = connect_to_site("commons.wikimedia.org", args) _ = query_api("Example.jpg", site) assert USER_AGENT in caplog.text diff --git a/tests/test_file_class.py b/tests/test_file_class.py index 1b76566..b935efc 100644 --- a/tests/test_file_class.py +++ b/tests/test_file_class.py @@ -24,6 +24,10 @@ from wikiget.file import File class TestFileClass: @pytest.fixture(scope="class") def file_with_name(self) -> File: + """ + A File object created with only a name should set its destination property to + the same value and its site property to the program's default site. + """ return File("foobar.jpg") def test_file_with_name(self, file_with_name: File) -> None: @@ -37,6 +41,10 @@ class TestFileClass: @pytest.fixture(scope="class") def file_with_name_and_dest(self) -> File: + """ + A File object created with a name and destination should set those properties + accordingly; they should not be the same value. + """ return File("foobar.jpg", dest="bazqux.jpg") def test_file_with_name_and_dest(self, file_with_name_and_dest: File) -> None: @@ -46,5 +54,9 @@ class TestFileClass: assert file_with_name_and_dest.dest != file_with_name_and_dest.name def test_file_with_name_and_site(self) -> None: + """ + A File object created with a name and site should set those properties + accordingly and not use the program's default site. + """ file = File("foobar.jpg", site="en.wikipedia.org") assert file.site == "en.wikipedia.org" diff --git a/tests/test_logging.py b/tests/test_logging.py index d02df62..b5ee6a0 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -27,6 +27,9 @@ logger = logging.getLogger() def test_custom_log_adapter(caplog: LogCaptureFixture) -> None: + """ + The custom log adapter should prepend the filename to log messages. + """ args = construct_parser().parse_args(["File:Example.jpg"]) configure_logging(args.verbose, args.logfile, quiet=args.quiet) adapter = FileLogAdapter(logger, {"filename": "Example.jpg"}) @@ -35,6 +38,9 @@ def test_custom_log_adapter(caplog: LogCaptureFixture) -> None: def test_file_logging(tmp_path: Path) -> None: + """ + Logging to a file should create the file in the specified location. + """ logfile_location = tmp_path / "test.log" args = construct_parser().parse_args( ["File:Example.jpg", "-l", str(logfile_location)] @@ -44,6 +50,9 @@ def test_file_logging(tmp_path: Path) -> None: def test_default_logging() -> None: + """ + The default log level should be set to WARNING. + """ args = construct_parser().parse_args(["File:Example.jpg"]) configure_logging(args.verbose, args.logfile, quiet=args.quiet) # each call of configure_logging() adds a new handler to the logger, so we need to @@ -53,6 +62,9 @@ def test_default_logging() -> None: def test_verbose_logging() -> None: + """ + When -v is passed, the log level should be set to INFO. + """ args = construct_parser().parse_args(["File:Example.jpg", "-v"]) configure_logging(args.verbose, args.logfile, quiet=args.quiet) handler = logger.handlers[-1] @@ -60,6 +72,9 @@ def test_verbose_logging() -> None: def test_very_verbose_logging() -> None: + """ + When -vv is passed, the log level should be set to DEBUG. + """ args = construct_parser().parse_args(["File:Example.jpg", "-vv"]) configure_logging(args.verbose, args.logfile, quiet=args.quiet) handler = logger.handlers[-1] @@ -67,6 +82,9 @@ def test_very_verbose_logging() -> None: def test_quiet_logging() -> None: + """ + When -q is passed, the log level should be set to ERROR. + """ args = construct_parser().parse_args(["File:Example.jpg", "-q"]) configure_logging(args.verbose, args.logfile, quiet=args.quiet) handler = logger.handlers[-1] diff --git a/tests/test_parse.py b/tests/test_parse.py index 79ac490..3cad21c 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -31,6 +31,10 @@ from wikiget.wikiget import construct_parser class TestGetDest: @pytest.fixture(scope="class") def file_with_filename(self) -> File: + """ + When a filename is passed to get_dest, it should create a File object with the + correct name and dest and the default site. + """ args = construct_parser().parse_args(["File:Example.jpg"]) return get_dest(args.FILE, args) @@ -45,6 +49,10 @@ class TestGetDest: @pytest.fixture(scope="class") def file_with_url(self) -> File: + """ + When a URL is passed to get_dest, it should create a File object with the + correct name and dest and the site from the URL. + """ args = construct_parser().parse_args( ["https://en.wikipedia.org/wiki/File:Example.jpg"] ) @@ -60,6 +68,9 @@ class TestGetDest: assert file_with_url.site == "en.wikipedia.org" def test_get_dest_with_bad_filename(self) -> None: + """ + The get_dest function should raise a ParseError if the filename is invalid. + """ args = construct_parser().parse_args(["Example.jpg"]) with pytest.raises(ParseError): _ = get_dest(args.FILE, args) @@ -67,6 +78,10 @@ class TestGetDest: def test_get_dest_with_different_site( self, caplog: pytest.LogCaptureFixture ) -> None: + """ + If a URL is passed to get_dest and a site is also given on the command line, + the site in the URL should be used and a warning log message created. + """ args = construct_parser().parse_args( [ "https://commons.wikimedia.org/wiki/File:Example.jpg", @@ -81,6 +96,9 @@ class TestGetDest: class TestReadBatchFile: @pytest.fixture() def dl_list(self, tmp_path: Path) -> Dict[int, str]: + """ + Create and process a test batch file with three lines, returning a dictionary. + """ tmp_file = tmp_path / "batch.txt" tmp_file.write_text("File:Foo.jpg\nFile:Bar.jpg\nFile:Baz.jpg\n") return read_batch_file(str(tmp_file)) @@ -88,6 +106,10 @@ class TestReadBatchFile: def test_batch_file_log( self, caplog: pytest.LogCaptureFixture, tmp_path: Path ) -> None: + """ + Reading in a batch file should create an info log message containing the name + of the batch file. + """ caplog.set_level(logging.INFO) tmp_file = tmp_path / "batch.txt" tmp_file.write_text("File:Foo.jpg\n") @@ -95,14 +117,25 @@ class TestReadBatchFile: assert f"Using file '{tmp_file}' for batch download" in caplog.text def test_batch_file_length(self, dl_list: Dict[int, str]) -> None: + """ + The processed batch dict should have the same number of items as lines in the + batch file. + """ assert len(dl_list) == 3 def test_batch_file_contents(self, dl_list: Dict[int, str]) -> None: + """ + The processed batch dict should have the correct line numbers and filenames as + keys and values, respectively. + """ expected_list = {1: "File:Foo.jpg", 2: "File:Bar.jpg", 3: "File:Baz.jpg"} assert dl_list == expected_list @pytest.fixture() def dl_list_stdin(self, monkeypatch: pytest.MonkeyPatch) -> Dict[int, str]: + """ + Pass three lines of filenames from stdin to read_batch_file and return a dict. + """ monkeypatch.setattr( "sys.stdin", io.StringIO("File:Foo.jpg\nFile:Bar.jpg\nFile:Baz.jpg\n") ) @@ -111,20 +144,35 @@ class TestReadBatchFile: def test_batch_stdin_log( self, caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch ) -> None: + """ + Using stdin for batch processing should create an info log message saying so. + """ caplog.set_level(logging.INFO) monkeypatch.setattr("sys.stdin", io.StringIO("File:Foo.jpg\n")) _ = read_batch_file("-") assert "Using stdin for batch download" in caplog.text def test_batch_stdin_length(self, dl_list_stdin: Dict[int, str]) -> None: + """ + The processed batch dict should have the same number of items as lines in the + input. + """ assert len(dl_list_stdin) == 3 def test_batch_stdin_contents(self, dl_list_stdin: Dict[int, str]) -> None: + """ + The processed batch dict should have the correct line numbers and filenames as + keys and values, respectively. + """ expected_list = {1: "File:Foo.jpg", 2: "File:Bar.jpg", 3: "File:Baz.jpg"} assert dl_list_stdin == expected_list @pytest.fixture() def dl_list_with_comment(self, tmp_path: Path) -> Dict[int, str]: + """ + Create and process a test batch file with four lines, one of which is + commented out and another of which is blank, and return a dictionary. + """ tmp_file = tmp_path / "batch.txt" tmp_file.write_text("File:Foo.jpg\n\n#File:Bar.jpg\nFile:Baz.jpg\n") return read_batch_file(str(tmp_file)) @@ -132,10 +180,18 @@ class TestReadBatchFile: def test_batch_file_with_comment_length( self, dl_list_with_comment: Dict[int, str] ) -> None: + """ + The processed batch dict should contain the same number of items as uncommented + and non-blank lines in the input. + """ assert len(dl_list_with_comment) == 2 def test_batch_file_with_comment_contents( self, dl_list_with_comment: Dict[int, str] ) -> None: + """ + The processed batch dict should have the correct line numbers and filenames as + keys and values, respectively, skipping any commented or blank lines. + """ expected_list = {1: "File:Foo.jpg", 4: "File:Baz.jpg"} assert dl_list_with_comment == expected_list -- cgit v1.2.3