fixes from PR

 - reverts max-line-length preference to 100
 - fixes some tests
 - clarifies a couple of edge cases

Author: jason-raitz

.flake8

@@ -1,4 +1,4 @@
 [flake8]
-max-line-length = 120
+max-line-length = 100
 # Equivalent to allow-init-docstring, which we set on pydoclint.
 extend-ignore = DOC301,F401

CHANGELOG.md

@@ -15,7 +15,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - README examples
-- lengthened max-line-length for pylint/flake8 to 120 to accomodate added typing
 
 ### Deprecated
 - N/A

pyproject.toml

@@ -62,4 +62,4 @@ skip-checking-raises = true
 style = "sphinx"
 
 [tool.pylint.format]
-max-line-length = 120
+max-line-length = 100

tests/test_fetch.py

@@ -28,9 +28,7 @@ def test_fetch_metadata_success(
     client: TINDClient,
 ) -> None:
     """fetch_metadata returns a PyMARC Record for a valid record ID."""
-    requests_mock.get(
-        f"{BASE_URL}/record/12345/", text=sample_marc_xml, status_code=200
-    )
+    requests_mock.get(f"{BASE_URL}/record/12345/", text=sample_marc_xml, status_code=200)
     record = client.fetch_metadata("12345")
     assert record["245"]["a"] == "Sample Title"
 
@@ -42,9 +40,7 @@ def test_fetch_metadata_404(requests_mock: req_mock.Mocker, client: TINDClient)
         client.fetch_metadata("99999")
 
 
-def test_fetch_metadata_empty_body(
-    requests_mock: req_mock.Mocker, client: TINDClient
-) -> None:
+def test_fetch_metadata_empty_body(requests_mock: req_mock.Mocker, client: TINDClient) -> None:
     """fetch_metadata raises RecordNotFoundError when the response body is empty."""
     requests_mock.get(f"{BASE_URL}/record/11111/", text="   ", status_code=200)
     with pytest.raises(RecordNotFoundError):
@@ -96,9 +92,7 @@ def test_fetch_file_not_found(
 # ---------------------------------------------------------------------------
 
 
-def test_fetch_file_metadata_success(
-    requests_mock: req_mock.Mocker, client: TINDClient
-) -> None:
+def test_fetch_file_metadata_success(requests_mock: req_mock.Mocker, client: TINDClient) -> None:
     """fetch_file_metadata returns a list of file metadata dicts."""
     payload = [{"name": "file.pdf", "size": 1024}]
     requests_mock.get(
@@ -110,9 +104,7 @@ def test_fetch_file_metadata_success(
     assert result[0]["name"] == "file.pdf"
 
 
-def test_fetch_file_metadata_error(
-    requests_mock: req_mock.Mocker, client: TINDClient
-) -> None:
+def test_fetch_file_metadata_error(requests_mock: req_mock.Mocker, client: TINDClient) -> None:
     """fetch_file_metadata raises TINDError on non-200 responses."""
     requests_mock.get(
         f"{BASE_URL}/record/12345/files",
@@ -128,9 +120,7 @@ def test_fetch_file_metadata_error(
 # ---------------------------------------------------------------------------
 
 
-def test_fetch_ids_search_success(
-    requests_mock: req_mock.Mocker, client: TINDClient
-) -> None:
+def test_fetch_ids_search_success(requests_mock: req_mock.Mocker, client: TINDClient) -> None:
     """fetch_ids_search returns the list of record IDs from the search response."""
     requests_mock.get(
         f"{BASE_URL}/search",
@@ -141,9 +131,7 @@ def test_fetch_ids_search_success(
     assert ids == ["1", "2", "3"]
 
 
-def test_fetch_ids_search_error(
-    requests_mock: req_mock.Mocker, client: TINDClient
-) -> None:
+def test_fetch_ids_search_error(requests_mock: req_mock.Mocker, client: TINDClient) -> None:
     """fetch_ids_search raises TINDError on non-200 responses."""
     requests_mock.get(
         f"{BASE_URL}/search",
@@ -184,6 +172,7 @@ def test_search_returns_xml(
     assert len(results) >= 1
     assert requests_mock.call_count == 1
 
+
 # ---------------------------------------------------------------------------
 # write_search_results_to_file / _iter_xml_records
 # ---------------------------------------------------------------------------
@@ -197,7 +186,7 @@ def test_write_search_results_to_file_with_malformed_output_filename(
 ) -> None:
     """write_search_results_to_file raises ValueError for a malformed output filename."""
     with pytest.raises(ValueError, match="output_file_name"):
-        client.write_search_results_to_file("", output_file_name = malformed_filename)
+        client.write_search_results_to_file("", output_file_name=malformed_filename)
 
 
 def test_write_search_results_to_file_empty_filename(client: TINDClient) -> None:
@@ -247,15 +236,18 @@ def test_write_search_results_to_file_success(
     tree = E.parse(tmp_path / "out.xml")
     records = tree.getroot().findall(f"{{{marc21_ns}}}record")
     assert len(records) == 3
-    assert tree.getroot().findtext(f"{{{marc21_ns}}}record/{{{marc21_ns}}}controlfield[@tag='001']") == "27320"
+    assert (
+        tree.getroot().findtext(f"{{{marc21_ns}}}record/{{{marc21_ns}}}controlfield[@tag='001']")
+        == "27320"
+    )
 
 
-def test_write_search_results_to_file_count_mismatch(
+def test_write_search_results_to_file_matched_but_no_records_returned(
     requests_mock: req_mock.Mocker,
     client: TINDClient,
     tmp_path: Path,
 ) -> None:
-    """write_search_results_to_file raises TINDError when streamed record count != ID count."""
+    """write_search_results_to_file raises TINDError when API returns no records for matched IDs"""
     client.default_storage_dir = str(tmp_path)
     requests_mock.get(
         f"{BASE_URL}/search",
@@ -266,13 +258,39 @@ def test_write_search_results_to_file_count_mismatch(
             {"text": (FIXTURES / "end-of-batch-tind-response.xml").read_text(), "status_code": 200},
         ],
     )
-    with pytest.raises(TINDError, match="Expected 3 records"):
+    with pytest.raises(TINDError, match="API did not return any."):
+        client.write_search_results_to_file("collection:'test'", "mismatch.xml")
+
+
+def test_write_search_results_to_file_matched_but_api_mismatch(
+    requests_mock: req_mock.Mocker,
+    client: TINDClient,
+    tmp_path: Path,
+) -> None:
+    """write_search_results_to_file raises TINDError when streamed record count != ID count."""
+    client.default_storage_dir = str(tmp_path)
+    requests_mock.get(
+        f"{BASE_URL}/search",
+        response_list=[
+            # fetch_ids_search says 3 hits
+            {
+                "text": json.dumps({"hits": ["27320", "28819", "29563", "123123"]}),
+                "status_code": 200,
+            },
+            # first paginated XML batch, but only 3 records instead of 4 as expected from the IDs
+            {"text": (FIXTURES / "1st-batch-tind-response.xml").read_text(), "status_code": 200},
+            # but the XML stream returns nothing immediately
+            {"text": (FIXTURES / "end-of-batch-tind-response.xml").read_text(), "status_code": 200},
+        ],
+    )
+    with pytest.raises(TINDError, match="Expected 4 records"):
         client.write_search_results_to_file("collection:'test'", "mismatch.xml")
 
+
 def test_write_search_results_to_file_malformed_xml_response(
-        requests_mock: req_mock.Mocker,
-        client: TINDClient,
-        tmp_path: Path,
+    requests_mock: req_mock.Mocker,
+    client: TINDClient,
+    tmp_path: Path,
 ) -> None:
     """write_search_results_to_file raises TINDError when the API returns malformed XML."""
     client.default_storage_dir = str(tmp_path)

tind_client/client.py

@@ -6,6 +6,7 @@
 import os
 import re
 from io import StringIO
+from pathlib import Path
 from typing import Any, Iterator
 import xml.etree.ElementTree as E
 
@@ -22,6 +23,7 @@
 # remove namespace that ElementTree adds to records when passed
 _NS_DECL: str = f' xmlns="{NS}"'
 
+
 class TINDClient:
     """Client for interacting with a TIND DA instance.
 
@@ -63,9 +65,7 @@ def fetch_metadata(self, record: str) -> Record:
         # records. Additionally, if the XML is malformed, the parser function may return
         # multiple records. We need to ensure that exactly one record is parsed.
         if len(records) != 1:
-            raise RecordNotFoundError(
-                f"Record {record} did not match exactly one record in TIND."
-            )
+            raise RecordNotFoundError(f"Record {record} did not match exactly one record in TIND.")
 
         return records[0]
 
@@ -84,9 +84,7 @@ def fetch_file(self, file_url: str, output_dir: str = "") -> str:
             raise ValueError("URL is not a valid TIND file download URL.")
 
         output_target = output_dir or self.default_storage_dir
-        (status, saved_to) = tind_download(
-            file_url, output_dir=output_target, api_key=self.api_key
-        )
+        (status, saved_to) = tind_download(file_url, output_dir=output_target, api_key=self.api_key)
 
         if status != 200:
             raise RecordNotFoundError("Referenced file could not be downloaded.")
@@ -179,9 +177,11 @@ def search(self, query: str, result_format: str = "xml") -> list[Any]:
 
         return recs
 
-    def write_search_results_to_file(self, query: str = "", output_file_name: str = "tind.xml") -> int:
+    def write_search_results_to_file(
+        self, query: str = "", output_file_name: str = "tind.xml"
+    ) -> int:
         """Search TIND and stream results to an XML file.
-        
+
         :param str query: A TIND search query string.
         :param str output_file_name: filename for the output XML file.
         :returns int: The number of records written to the file.
@@ -195,23 +195,29 @@ def write_search_results_to_file(self, query: str = "", output_file_name: str =
 
         recs_written = 0
         output_path = os.path.join(self.default_storage_dir, output_file_name)
-        with open(output_path, "w", encoding="utf-8") as f:
-            f.write(f'<?xml version="1.0" encoding="UTF-8"?>\n<collection xmlns="{NS}">\n')
-            for record in self._iter_xml_records(query):
-                record_xml = E.tostring(record, encoding="unicode")
-                f.write(record_xml.replace(_NS_DECL, ""))
-                f.write("\n")
-                recs_written += 1
-
-            f.write("</collection>\n")
+        try:
+            with open(output_path, "w", encoding="utf-8") as f:
+                f.write(f'<?xml version="1.0" encoding="UTF-8"?>\n<collection xmlns="{NS}">\n')
+                for record in self._iter_xml_records(query):
+                    record_xml = E.tostring(record, encoding="unicode")
+                    f.write(record_xml.replace(_NS_DECL, ""))
+                    f.write("\n")
+                    recs_written += 1
+                if recs_written == 0:
+                    # We expected records but didn't receive any through pagination
+                    raise TINDError(f"Matched {total_hits} tind ids, but API did not return any.")
+                f.write("</collection>\n")
+        except Exception:
+            Path(output_path).unlink(missing_ok=True)
+            raise
 
         if recs_written != total_hits:
             raise TINDError(f"Expected {total_hits} records, but wrote {recs_written} to file.")
         return recs_written
 
     def _iter_xml_records(self, query: str) -> Iterator[E.Element]:
         """Yield every ``<record>`` element from all pages of a search.
-        
+
         Issues the initial search request, then yields records one at a time,
         and continues to issue paginated search requests until all records have been yielded.
         :param str query: A TIND search query string.