From 572b5a8f4ed2efd9ffd6244082b84490d33b061b Mon Sep 17 00:00:00 2001 From: Charlotte Kostelic Date: Thu, 31 Oct 2024 15:59:58 -0400 Subject: [PATCH] Fix exception handling (#10) * added exception handling to get_vendor_files * added mock_connect fixture * renamed fixtures for consistency --- tests/conftest.py | 98 +++++++++++++++++++++---------------- tests/test_commands.py | 61 +++++++---------------- tests/test_utils.py | 14 +----- tests/test_validator.py | 25 +++++----- vendor_file_cli/commands.py | 42 +++++++++------- 5 files changed, 110 insertions(+), 130 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 76746e6..d4fad3c 100644 --- a/tests/conftest.py +++ b/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): diff --git a/tests/test_commands.py b/tests/test_commands.py index b1becf4..c7f8582 100644 --- a/tests/test_commands.py +++ b/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 diff --git a/tests/test_utils.py b/tests/test_utils.py index b988910..a5844f8 100644 --- a/tests/test_utils.py +++ b/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 ( diff --git a/tests/test_validator.py b/tests/test_validator.py index ccbbb7d..18920bd 100644 --- a/tests/test_validator.py +++ b/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, diff --git a/vendor_file_cli/commands.py b/vendor_file_cli/commands.py index 4fa77f3..3f3f3e8 100644 --- a/vendor_file_cli/commands.py +++ b/vendor_file_cli/commands.py @@ -4,6 +4,7 @@ import logging.handlers import datetime import os +from file_retriever.errors import FileRetrieverError from vendor_file_cli.validator import ( validate_file, get_single_file, @@ -40,30 +41,33 @@ def get_vendor_files( """ for vendor in vendors: vendor_dst = os.environ[f"{vendor.upper()}_DST"] - with connect("nsdrop") as nsdrop_client: - with connect(vendor) as vendor_client: - files = get_vendor_file_list( - vendor=vendor, - timedelta=datetime.timedelta(days=days, hours=hours), - nsdrop_client=nsdrop_client, - vendor_client=vendor_client, - ) - logger.info( - f"({vendor_client.name}) {len(files)} file(s) on " - f"{vendor_client.name} server to copy to NSDROP" - ) - for file in files: - get_single_file( + try: + with connect("nsdrop") as nsdrop_client: + with connect(vendor) as vendor_client: + files = get_vendor_file_list( vendor=vendor, - file=file, - vendor_client=vendor_client, + timedelta=datetime.timedelta(days=days, hours=hours), nsdrop_client=nsdrop_client, + vendor_client=vendor_client, ) - if len(files) > 0: logger.info( - f"({nsdrop_client.name}) {len(files)} file(s) " - f"copied to `{vendor_dst}`" + f"({vendor_client.name}) {len(files)} file(s) on " + f"{vendor_client.name} server to copy to NSDROP" ) + for file in files: + get_single_file( + vendor=vendor, + file=file, + vendor_client=vendor_client, + nsdrop_client=nsdrop_client, + ) + if len(files) > 0: + logger.info( + f"({nsdrop_client.name}) {len(files)} file(s) " + f"copied to `{vendor_dst}`" + ) + except FileRetrieverError: + continue def validate_files(vendor: str, files: list | None) -> None: