Merge pull request #11 from clpo13/add-tests

Add and improve some tests

7 files changed, 200 insertions, 39 deletions

diff --git a/src/wikiget/dl.py b/src/wikiget/dl.py
index c0af6e9..20d8a07 100644
--- a/src/wikiget/dl.py
+++ b/src/wikiget/dl.py
@@ -49,6 +49,35 @@ def prep_download(dl: str, args: Namespace) -> File:
     return file
 
 
+def process_download(args: Namespace) -> None:
+    if args.batch:
+        # batch download mode
+        errors = batch_download(args)
+        if errors:
+            # return non-zero exit code if any problems were encountered, even if some
+            # downloads completed successfully
+            logger.warning(
+                f"{errors} problem{'s'[:errors^1]} encountered during batch processing"
+            )
+            sys.exit(1)  # completed with errors
+    else:
+        # single download mode
+        try:
+            file = prep_download(args.FILE, args)
+        except ParseError as e:
+            logger.error(e)
+            sys.exit(1)
+        except FileExistsError as e:
+            logger.warning(e)
+            sys.exit(1)
+        except (ConnectionError, HTTPError, InvalidResponse, LoginError, APIError):
+            sys.exit(1)
+
+        errors = download(file, args)
+        if errors:
+            sys.exit(1)  # completed with errors
+
+
 def batch_download(args: Namespace) -> int:
     errors = 0
 
diff --git a/src/wikiget/wikiget.py b/src/wikiget/wikiget.py
index 55bcddc..33f7b2c 100644
--- a/src/wikiget/wikiget.py
+++ b/src/wikiget/wikiget.py
@@ -17,14 +17,9 @@
 
 import argparse
 import logging
-import sys
-
-from mwclient import APIError, InvalidResponse, LoginError
-from requests import ConnectionError, HTTPError
 
 import wikiget
-from wikiget.dl import batch_download, download, prep_download
-from wikiget.exceptions import ParseError
+from wikiget.dl import process_download
 from wikiget.logging import configure_logging
 
 logger = logging.getLogger(__name__)
@@ -134,29 +129,4 @@ def main() -> None:
     logger.info(f"Starting download session using wikiget {wikiget.__version__}")
     logger.debug(f"User agent: {wikiget.USER_AGENT}")
 
-    if args.batch:
-        # batch download mode
-        errors = batch_download(args)
-        if errors:
-            # return non-zero exit code if any problems were encountered, even if some
-            # downloads completed successfully
-            logger.warning(
-                f"{errors} problem{'s'[:errors^1]} encountered during batch processing"
-            )
-            sys.exit(1)  # completed with errors
-    else:
-        # single download mode
-        try:
-            file = prep_download(args.FILE, args)
-        except ParseError as e:
-            logger.error(e)
-            sys.exit(1)
-        except FileExistsError as e:
-            logger.warning(e)
-            sys.exit(1)
-        except (ConnectionError, HTTPError, InvalidResponse, LoginError, APIError):
-            sys.exit(1)
-
-        errors = download(file, args)
-        if errors:
-            sys.exit(1)  # completed with errors
+    process_download(args)
diff --git a/tests/test_client.py b/tests/test_client.py
index 41ce948..cf6e29c 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -16,6 +16,7 @@
 # along with Wikiget. If not, see <https://www.gnu.org/licenses/>.
 
 import logging
+from unittest.mock import MagicMock, patch
 
 import pytest
 
@@ -25,17 +26,30 @@ 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:
+        """
+        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)
-        _ = 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:
+        """
+        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_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 <https://www.gnu.org/licenses/>.
 
 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
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