Fix exception handling (#10)

* added exception handling to get_vendor_files

* added mock_connect fixture

* renamed fixtures for consistency

tests/conftest.py

@@ -1,18 +1,21 @@
 import datetime
+import ftplib
 import io
-import logging
 import os
-from googleapiclient.errors import HttpError  # type: ignore
 from pydantic_core import ValidationError, InitErrorDetails
 from pymarc import Record, Field, Subfield, Indicators
 import pytest
 from click.testing import CliRunner
 from file_retriever.file import File, FileInfo
 from file_retriever.connect import Client
-from file_retriever._clients import _ftpClient, _sftpClient
 
 
-class MockFileInfo(FileInfo):
+@pytest.fixture(autouse=True)
+def set_caplog_level(caplog):
+    caplog.set_level("DEBUG")
+
+
+class StubFileInfo(FileInfo):
     def __init__(self, file_name: str | None = None):
         today = datetime.datetime.now(tz=datetime.timezone.utc)
         mtime = (today - datetime.timedelta(days=10)).timestamp()
@@ -21,7 +24,7 @@ def __init__(self, file_name: str | None = None):
         super().__init__(file_name, mtime, 33188, 140401, 0, 0, None)
 
 
-def mock_marc() -> Record:
+def stub_marc() -> Record:
     bib = Record()
     bib.leader = "00454cam a22001575i 4500"
     bib.add_field(Field(tag="001", data="on1381158740"))
@@ -37,17 +40,17 @@ def mock_marc() -> Record:
 
 @pytest.fixture
 def stub_record() -> Record:
-    return mock_marc()
+    return stub_marc()
 
 
 @pytest.fixture
 def stub_file_info() -> FileInfo:
-    return MockFileInfo(file_name="foo.mrc")
+    return StubFileInfo(file_name="foo.mrc")
 
 
 @pytest.fixture
 def stub_file(stub_file_info, mock_valid_record) -> File:
-    return File.from_fileinfo(stub_file_info, io.BytesIO(mock_marc().as_marc21()))
+    return File.from_fileinfo(stub_file_info, io.BytesIO(stub_marc().as_marc21()))
 
 
 @pytest.fixture
@@ -73,17 +76,26 @@ class MockSession:
     def _check_dir(self, *args, **kwargs):
         pass
 
+    def _is_file(self, dir, file_name, *args, **kwargs) -> bool:
+        return True
+
     def close(self, *args, **kwargs):
         pass
 
+    def fetch_file(self, file, *args, **kwargs) -> File:
+        return File.from_fileinfo(file, io.BytesIO(stub_marc().as_marc21()))
+
     def get_file_data(self, file_name, *args, **kwargs) -> FileInfo:
-        return MockFileInfo(file_name=file_name)
+        return StubFileInfo(file_name=file_name)
+
+    def is_active(self, *args, **kwargs) -> bool:
+        return True
 
     def list_file_data(self, dir, *args, **kwargs) -> list[FileInfo]:
         if "NSDROP" in dir:
-            return [MockFileInfo(file_name="bar.mrc")]
+            return [StubFileInfo(file_name="bar.mrc")]
         else:
-            return [MockFileInfo(file_name="foo.mrc")]
+            return [StubFileInfo(file_name="foo.mrc")]
 
     def list_file_names(self, dir, *args, **kwargs) -> list[str]:
         if "NSDROP" in dir:
@@ -93,29 +105,49 @@ def list_file_names(self, dir, *args, **kwargs) -> list[str]:
         else:
             return ["foo.mrc"]
 
-    def fetch_file(self, file, *args, **kwargs) -> File:
-        return File.from_fileinfo(file, io.BytesIO(mock_marc().as_marc21()))
-
     def write_file(self, file, *args, **kwargs) -> FileInfo:
-        return MockFileInfo(file_name=file.file_name)
+        return StubFileInfo(file_name=file.file_name)
 
 
 @pytest.fixture
-def mock_Client(monkeypatch, mock_sheet_config):
+def stub_client(monkeypatch, mock_sheet_config):
     original_connect_to_server = Client._Client__connect_to_server
 
     def mock_connect_to_server(self, username, password):
         original_connect_to_server(self, username, password)
         return MockSession()
 
-    monkeypatch.setattr(_ftpClient, "_connect_to_server", lambda *args, **kwargs: None)
-    monkeypatch.setattr(_sftpClient, "_connect_to_server", lambda *args, **kwargs: None)
+    def stub_response(*args, **kwargs):
+        pass
+
+    monkeypatch.setattr("ftplib.FTP.connect", stub_response)
+    monkeypatch.setattr("ftplib.FTP.login", stub_response)
+    monkeypatch.setattr("paramiko.SSHClient.connect", stub_response)
+    monkeypatch.setattr("paramiko.SSHClient.load_system_host_keys", stub_response)
+    monkeypatch.setattr("paramiko.SSHClient.open_sftp", stub_response)
     monkeypatch.setattr(Client, "_Client__connect_to_server", mock_connect_to_server)
     monkeypatch.setattr(Client, "check_file", lambda *args, **kwargs: False)
-    monkeypatch.setattr(Client, "is_file", True)
-    monkeypatch.setattr(
-        "vendor_file_cli.validator.write_data_to_sheet", lambda *args, **kwargs: None
-    )
+    monkeypatch.setattr("vendor_file_cli.validator.write_data_to_sheet", stub_response)
+    monkeypatch.setattr(os.path, "isfile", lambda *args, **kwargs: True)
+
+    def stub_client_response(name):
+        return Client(
+            name=name.upper(),
+            username=os.environ[f"{name.upper()}_USER"],
+            password=os.environ[f"{name.upper()}_PASSWORD"],
+            host=os.environ[f"{name.upper()}_HOST"],
+            port=os.environ[f"{name.upper()}_PORT"],
+        )
+
+    return stub_client_response
+
+
+@pytest.fixture
+def stub_client_auth_error(monkeypatch, stub_client):
+    def mock_connect_to_server(*args, **kwargs):
+        raise ftplib.error_perm
+
+    monkeypatch.setattr("ftplib.FTP.login", mock_connect_to_server)
 
 
 @pytest.fixture
@@ -156,7 +188,7 @@ def unset_env_var(monkeypatch, mock_vendor_creds) -> None:
 
 
 @pytest.fixture
-def cli_runner(monkeypatch, mock_Client) -> CliRunner:
+def cli_runner(monkeypatch, stub_client) -> CliRunner:
     runner = CliRunner()
 
     def mock_logging(*args, **kwargs):
@@ -201,11 +233,10 @@ def run_local_server(self, *args, **kwargs):
 
 
 @pytest.fixture
-def mock_sheet_config(monkeypatch, caplog, mock_open_file):
+def mock_sheet_config(monkeypatch, mock_open_file):
     def build_sheet(*args, **kwargs):
         return MockResource()
 
-    caplog.set_level(logging.DEBUG)
     monkeypatch.setattr("googleapiclient.discovery.build", build_sheet)
     monkeypatch.setattr("googleapiclient.discovery.build_from_document", build_sheet)
     monkeypatch.setattr(
@@ -251,23 +282,6 @@ def values(self, *args, **kwargs):
         return self
 
 
-class MockError:
-    def __init__(self):
-        self.status = 400
-        self.reason = "bad_request"
-
-
-@pytest.fixture
-def mock_sheet_http_error(monkeypatch):
-    def mock_error(*args, **kwargs):
-        raise HttpError(
-            resp=MockError(), content=b"{'message':  'Bad Request'}", uri="foo"
-        )
-
-    monkeypatch.setattr("googleapiclient.discovery.build", mock_error)
-    monkeypatch.setattr("googleapiclient.discovery.build_from_document", mock_error)
-
-
 @pytest.fixture
 def mock_sheet_timeout_error(monkeypatch):
     def mock_error(*args, **kwargs):

tests/test_commands.py

@@ -1,12 +1,7 @@
-from vendor_file_cli.commands import (
-    get_vendor_files,
-    validate_files,
-)
-from vendor_file_cli.validator import get_single_file
-from vendor_file_cli.utils import connect
+from vendor_file_cli.commands import get_vendor_files, validate_files
 
 
-def test_get_vendor_files(mock_Client, caplog):
+def test_get_vendor_files(stub_client, caplog):
     get_vendor_files(vendors=["leila"], days=300)
     assert "(NSDROP) Connecting to " in caplog.text
     assert "(LEILA) Connecting to " in caplog.text
@@ -17,58 +12,36 @@ def test_get_vendor_files(mock_Client, caplog):
     assert "(NSDROP) Client session closed" in caplog.text
 
 
-def test_get_vendor_files_no_files(mock_Client, caplog):
-    get_vendor_files(vendors=["eastview"], days=1, hours=1)
+def test_get_vendor_files_invalid_creds(stub_client_auth_error, caplog):
+    get_vendor_files(vendors=["leila", "eastview", "midwest_nypl"], days=300)
+    assert (
+        "file_retriever._clients",
+        40,
+        "(LEILA) Unable to authenticate with provided credentials: ",
+    ) in caplog.record_tuples
     assert "(NSDROP) Connecting to ftp.nsdrop.com via SFTP client" in caplog.text
     assert "(EASTVIEW) Connecting to ftp.eastview.com via SFTP client" in caplog.text
-    assert "(EASTVIEW) 0 file(s) on EASTVIEW server to copy to NSDROP" in caplog.text
+    assert "(EASTVIEW) 1 file(s) on EASTVIEW server to copy to NSDROP" in caplog.text
     assert "(EASTVIEW) Client session closed" in caplog.text
     assert "(NSDROP) Client session closed" in caplog.text
 
 
-def test_get_single_file_no_validation(mock_Client, stub_file_info, caplog):
-    vendor_client = connect("midwest_nypl")
-    nsdrop_client = connect("nsdrop")
-    get_single_file(
-        vendor="midwest_nypl",
-        file=stub_file_info,
-        vendor_client=vendor_client,
-        nsdrop_client=nsdrop_client,
-    )
-    assert (
-        "(MIDWEST_NYPL) Connecting to ftp.midwest_nypl.com via FTP client"
-        in caplog.text
-    )
+def test_get_vendor_files_no_files(stub_client, caplog):
+    get_vendor_files(vendors=["eastview"], days=1, hours=1)
     assert "(NSDROP) Connecting to ftp.nsdrop.com via SFTP client" in caplog.text
-    assert "(NSDROP) Validating bar file: foo.mrc" not in caplog.text
-    assert (
-        "(NSDROP) Writing foo.mrc to `NSDROP/vendor_records/midwest_nypl`"
-        in caplog.text
-    )
-
-
-def test_get_single_file_with_validation(mock_Client, stub_file_info, caplog):
-    vendor_client = connect("eastview")
-    nsdrop_client = connect("nsdrop")
-    get_single_file(
-        vendor="eastview",
-        file=stub_file_info,
-        vendor_client=vendor_client,
-        nsdrop_client=nsdrop_client,
-    )
     assert "(EASTVIEW) Connecting to ftp.eastview.com via SFTP client" in caplog.text
-    assert "(NSDROP) Connecting to ftp.nsdrop.com via SFTP client" in caplog.text
-    assert "(NSDROP) Validating eastview file: foo.mrc" in caplog.text
-    assert "(NSDROP) Writing foo.mrc to `NSDROP/vendor_records/eastview`" in caplog.text
+    assert "(EASTVIEW) 0 file(s) on EASTVIEW server to copy to NSDROP" in caplog.text
+    assert "(EASTVIEW) Client session closed" in caplog.text
+    assert "(NSDROP) Client session closed" in caplog.text
 
 
-def test_validate_files(mock_Client, caplog):
+def test_validate_files(stub_client, caplog):
     validate_files(vendor="eastview", files=None)
     assert "(NSDROP) Connecting to " in caplog.text
     assert "(NSDROP) Validating eastview file: bar.mrc" in caplog.text
 
 
-def test_validate_files_with_list(mock_Client, caplog):
+def test_validate_files_with_list(stub_client, caplog):
     validate_files(vendor="eastview", files=["foo.mrc"])
     assert "(NSDROP) Connecting to " in caplog.text
     assert "(NSDROP) Validating eastview file: foo.mrc" in caplog.text

tests/test_utils.py

@@ -38,7 +38,7 @@ def test_configure_sheet_generate_new_creds(mock_sheet_config_no_creds):
     assert creds.refresh_token is not None
 
 
-def test_connect(mock_Client):
+def test_connect(stub_client):
     client = connect("leila")
     assert client.name == "LEILA"
     assert client.host == "ftp.leila.com"
@@ -155,17 +155,7 @@ def test_write_data_to_sheet(mock_sheet_config):
     assert sorted(list(keys)) == sorted(["spreadsheetId", "tableRange"])
 
 
-def test_write_data_to_sheet_http_error(
-    mock_sheet_config, mock_sheet_http_error, caplog
-):
-    data = write_data_to_sheet({"file_name": ["foo.mrc"], "vendor_code": ["FOO"]})
-    assert data is None
-    assert "Error occurred while sending data to google sheet: " in caplog.text
-
-
-def test_write_data_to_sheet_timeout_error(
-    mock_sheet_config, mock_sheet_timeout_error, caplog
-):
+def test_write_data_to_sheet_error(mock_sheet_config, mock_sheet_timeout_error, caplog):
     data = write_data_to_sheet({"file_name": ["foo.mrc"], "vendor_code": ["FOO"]})
     assert data is None
     assert (

tests/test_validator.py

@@ -6,13 +6,12 @@
     validate_file,
     validate_single_record,
 )
-from vendor_file_cli.utils import connect
 
 
 @pytest.mark.parametrize("vendor", ["midwest_nypl", "bakertaylor_bpl"])
-def test_get_single_file_no_validation(mock_Client, stub_file_info, vendor, caplog):
-    vendor_client = connect(vendor)
-    nsdrop_client = connect("nsdrop")
+def test_get_single_file_no_validation(stub_client, stub_file_info, vendor, caplog):
+    vendor_client = stub_client(vendor)
+    nsdrop_client = stub_client("nsdrop")
     get_single_file(
         vendor=vendor,
         file=stub_file_info,
@@ -30,9 +29,9 @@ def test_get_single_file_no_validation(mock_Client, stub_file_info, vendor, capl
     )
 
 
-def test_get_single_file_with_validation(mock_Client, stub_file_info, caplog):
-    vendor_client = connect("eastview")
-    nsdrop_client = connect("nsdrop")
+def test_get_single_file_with_validation(stub_client, stub_file_info, caplog):
+    vendor_client = stub_client("eastview")
+    nsdrop_client = stub_client("nsdrop")
     get_single_file(
         vendor="eastview",
         file=stub_file_info,
@@ -45,9 +44,9 @@ def test_get_single_file_with_validation(mock_Client, stub_file_info, caplog):
     assert "(NSDROP) Writing foo.mrc to `NSDROP/vendor_records/eastview`" in caplog.text
 
 
-def test_get_single_file_bakertaylor_bpl_root(mock_Client, stub_file_info, caplog):
-    vendor_client = connect("bakertaylor_bpl")
-    nsdrop_client = connect("nsdrop")
+def test_get_single_file_bakertaylor_bpl_root(stub_client, stub_file_info, caplog):
+    vendor_client = stub_client("bakertaylor_bpl")
+    nsdrop_client = stub_client("nsdrop")
     stub_file_info.file_name = "ADDfoo.mrc"
     get_single_file(
         vendor="bakertaylor_bpl",
@@ -67,10 +66,10 @@ def test_get_single_file_bakertaylor_bpl_root(mock_Client, stub_file_info, caplo
 
 
 @pytest.mark.parametrize("vendor", ["midwest_nypl", "bakertaylor_bpl"])
-def test_get_vendor_file_list(mock_Client, vendor, caplog):
+def test_get_vendor_file_list(stub_client, vendor, caplog):
     file_list = []
-    with connect("nsdrop") as nsdrop_client:
-        with connect(vendor) as vendor_client:
+    with stub_client("nsdrop") as nsdrop_client:
+        with stub_client(vendor) as vendor_client:
             file_list.extend(
                 get_vendor_file_list(
                     vendor=vendor,