From deffb7a8d95ceb085a907f8061b65b8cf22037da Mon Sep 17 00:00:00 2001 From: Julian Risch Date: Sat, 15 Aug 2026 13:36:24 +0200 Subject: [PATCH 1/3] test: Unstructured - increase unit tests coverage Cover the code paths that were previously only reachable through the integration-marked tests, which the unit coverage badge excludes: - _create_documents: all three document_creation_mode branches, page bucketing, element index/category, and the deepcopy of caller metadata - run: file/directory expansion, metadata zipping, and both ValueError paths, which raise before any API call is made - _partition_file_into_elements: api_url/api_key/unstructured_kwargs forwarding and the swallow-and-warn failure path - to_dict/from_dict round trip with no API key The tests build real unstructured Element objects rather than mocks, so they stay honest against the SDK, and patch partition_via_api as the only external boundary. Also set --strict-markers, which this integration was missing. Unit coverage: 34% -> 97%. Co-Authored-By: Claude Opus 5 (1M context) --- integrations/unstructured/pyproject.toml | 1 + .../unstructured/tests/test_converter_unit.py | 175 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 integrations/unstructured/tests/test_converter_unit.py diff --git a/integrations/unstructured/pyproject.toml b/integrations/unstructured/pyproject.toml index 2d2293c914..6417cffdf1 100644 --- a/integrations/unstructured/pyproject.toml +++ b/integrations/unstructured/pyproject.toml @@ -163,4 +163,5 @@ exclude_lines = ["no cov", "if __name__ == .__main__.:", "if TYPE_CHECKING:"] [tool.pytest.ini_options] minversion = "6.0" +addopts = "--strict-markers" markers = ["integration: integration tests"] \ No newline at end of file diff --git a/integrations/unstructured/tests/test_converter_unit.py b/integrations/unstructured/tests/test_converter_unit.py new file mode 100644 index 0000000000..0ed6986b86 --- /dev/null +++ b/integrations/unstructured/tests/test_converter_unit.py @@ -0,0 +1,175 @@ +# SPDX-FileCopyrightText: 2023-present deepset GmbH +# +# SPDX-License-Identifier: Apache-2.0 +from pathlib import Path +from unittest.mock import patch + +import pytest +from haystack.utils import Secret +from unstructured.documents.elements import ElementMetadata, Text, Title + +from haystack_integrations.components.converters.unstructured import UnstructuredFileConverter + +CONVERTER_MODULE = "haystack_integrations.components.converters.unstructured.converter" + +LOCAL_API_URL = "http://localhost:8000/general/v0/general" + + +def _element(text: str, **metadata) -> Text: + """Build a real Unstructured element, so the fakes stay honest against the SDK.""" + return Text(text, metadata=ElementMetadata(**metadata)) + + +@pytest.fixture +def converter() -> UnstructuredFileConverter: + """A converter pointing at a local API, so no API key is required.""" + return UnstructuredFileConverter(api_url=LOCAL_API_URL) + + +class TestCreateDocuments: + """`_create_documents` is a pure static method: elements in, Haystack Documents out.""" + + def test_one_doc_per_file_joins_all_elements_with_the_separator(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[_element("first"), _element("second"), _element("third")], + document_creation_mode="one-doc-per-file", + separator="|", + meta={"key": "value"}, + ) + + assert len(docs) == 1 + assert docs[0].content == "first|second|third" + assert docs[0].meta == {"key": "value", "file_path": "a/file.pdf"} + + def test_one_doc_per_page_groups_elements_by_page_number(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[ + _element("page one, first", page_number=1), + _element("page two", page_number=2), + _element("page one, second", page_number=1), + ], + document_creation_mode="one-doc-per-page", + separator="|", + meta={}, + ) + + assert len(docs) == 2 + assert docs[0].content == "page one, first|page one, second|" + assert docs[0].meta["page_number"] == 1 + assert docs[1].content == "page two|" + assert docs[1].meta["page_number"] == 2 + + def test_one_doc_per_element_records_the_index_and_category_of_each_element(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[Text("body text"), Title("A Heading")], + document_creation_mode="one-doc-per-element", + separator="\n\n", + meta={"key": "value"}, + ) + + assert len(docs) == 2 + assert docs[0].content == "body text" + assert docs[0].meta["element_index"] == 0 + assert docs[0].meta["category"] == "UncategorizedText" + assert docs[1].content == "A Heading" + assert docs[1].meta["element_index"] == 1 + assert docs[1].meta["category"] == "Title" + assert all(doc.meta["key"] == "value" for doc in docs) + + @pytest.mark.parametrize("document_creation_mode", ["one-doc-per-file", "one-doc-per-page", "one-doc-per-element"]) + def test_the_caller_metadata_is_never_mutated(self, document_creation_mode): + meta = {"key": "value"} + + UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[_element("text", page_number=1)], + document_creation_mode=document_creation_mode, + separator="\n\n", + meta=meta, + ) + + assert meta == {"key": "value"} + + +class TestPartitionFileIntoElements: + def test_forwards_the_api_settings_and_the_extra_kwargs(self): + converter = UnstructuredFileConverter( + api_url=LOCAL_API_URL, + api_key=Secret.from_token("secret-key"), + unstructured_kwargs={"strategy": "hi_res"}, + ) + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) + + assert len(elements) == 1 + mock_partition.assert_called_once_with( + filename="a/file.pdf", + api_url=LOCAL_API_URL, + api_key="secret-key", + strategy="hi_res", + ) + + def test_returns_no_elements_and_warns_when_the_api_call_fails(self, converter, caplog): + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.side_effect = RuntimeError("API is down") + elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) + + assert elements == [] + assert "a/file.pdf" in caplog.text + assert "API is down" in caplog.text + + +class TestRun: + def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, converter, tmp_path): + (tmp_path / "first.txt").write_text("first") + (tmp_path / "second.txt").write_text("second") + (tmp_path / "nested.dir").mkdir() + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + documents = converter.run(paths=[tmp_path])["documents"] + + assert len(documents) == 2 + assert mock_partition.call_count == 2 + + def test_zips_a_metadata_list_with_the_given_file_paths(self, converter, tmp_path): + first = tmp_path / "first.txt" + first.write_text("first") + second = tmp_path / "second.txt" + second.write_text("second") + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + documents = converter.run(paths=[first, second], meta=[{"source": "one"}, {"source": "two"}])["documents"] + + assert [doc.meta["source"] for doc in documents] == ["one", "two"] + + def test_rejects_a_metadata_list_when_paths_contain_a_directory(self, converter, tmp_path): + (tmp_path / "first.txt").write_text("first") + + with pytest.raises(ValueError, match="`meta` can only be a dictionary"): + converter.run(paths=[tmp_path], meta=[{"source": "one"}]) + + def test_rejects_a_metadata_list_whose_length_does_not_match_the_paths(self, converter, tmp_path): + first = tmp_path / "first.txt" + first.write_text("first") + + with pytest.raises(ValueError): + converter.run(paths=[first], meta=[{"source": "one"}, {"source": "two"}]) + + +class TestSerializationWithoutApiKey: + def test_to_dict_and_from_dict_round_trip_a_none_api_key(self): + converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, api_key=None) + + converter_dict = converter.to_dict() + assert converter_dict["init_parameters"]["api_key"] is None + + deserialized = UnstructuredFileConverter.from_dict(converter_dict) + assert deserialized.api_key is None + assert deserialized.api_url == LOCAL_API_URL From 05f838d81bf4a460f8cf2058749d3a40c0e601ce Mon Sep 17 00:00:00 2001 From: Julian Risch Date: Thu, 20 Aug 2026 18:23:56 +0200 Subject: [PATCH 2/3] test: fold the new Unstructured unit tests into test_converter.py Address review feedback on #3794: - Merge test_converter_unit.py into test_converter.py so the converter's tests live in one file, with the api_key=None serialization round-trip next to the existing to_dict/from_dict tests. Reuse the new LOCAL_API_URL constant in the existing integration tests instead of repeating the literal ten times. - Pin the ValueError message in the metadata-length-mismatch test, since run() has two ValueError paths. - Put a file inside the subdirectory in the directory test, so it actually pins the non-recursive glob rather than only the is_file() guard. Co-Authored-By: Claude Opus 5 (1M context) --- .../unstructured/tests/test_converter.py | 207 +++++++++++++++--- .../unstructured/tests/test_converter_unit.py | 175 --------------- 2 files changed, 180 insertions(+), 202 deletions(-) delete mode 100644 integrations/unstructured/tests/test_converter_unit.py diff --git a/integrations/unstructured/tests/test_converter.py b/integrations/unstructured/tests/test_converter.py index 063289b071..8e92d98791 100644 --- a/integrations/unstructured/tests/test_converter.py +++ b/integrations/unstructured/tests/test_converter.py @@ -1,10 +1,30 @@ # SPDX-FileCopyrightText: 2023-present deepset GmbH # # SPDX-License-Identifier: Apache-2.0 +from pathlib import Path +from unittest.mock import patch + import pytest +from haystack.utils import Secret +from unstructured.documents.elements import ElementMetadata, Text, Title from haystack_integrations.components.converters.unstructured import UnstructuredFileConverter +CONVERTER_MODULE = "haystack_integrations.components.converters.unstructured.converter" + +LOCAL_API_URL = "http://localhost:8000/general/v0/general" + + +def _element(text: str, **metadata) -> Text: + """Build a real Unstructured element, so the fakes stay honest against the SDK.""" + return Text(text, metadata=ElementMetadata(**metadata)) + + +@pytest.fixture +def converter() -> UnstructuredFileConverter: + """A converter pointing at a local API, so no API key is required.""" + return UnstructuredFileConverter(api_url=LOCAL_API_URL) + class TestUnstructuredFileConverter: @pytest.mark.usefixtures("set_env_variables") @@ -74,13 +94,21 @@ def test_from_dict(self, monkeypatch): assert converter.unstructured_kwargs == {"foo": "bar"} assert not converter.progress_bar + def test_to_dict_and_from_dict_round_trip_a_none_api_key(self): + converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, api_key=None) + + converter_dict = converter.to_dict() + assert converter_dict["init_parameters"]["api_key"] is None + + deserialized = UnstructuredFileConverter.from_dict(converter_dict) + assert deserialized.api_key is None + assert deserialized.api_url == LOCAL_API_URL + @pytest.mark.integration def test_run_one_doc_per_file(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-file" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-file") documents = local_converter.run([pdf_path])["documents"] @@ -91,9 +119,7 @@ def test_run_one_doc_per_file(self, samples_path): def test_run_one_doc_per_page(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-page" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-page") documents = local_converter.run([pdf_path])["documents"] @@ -106,9 +132,7 @@ def test_run_one_doc_per_page(self, samples_path): def test_run_one_doc_per_element(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-element" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") documents = local_converter.run([pdf_path])["documents"] @@ -124,9 +148,7 @@ def test_run_one_doc_per_element(self, samples_path): def test_run_one_doc_per_file_with_meta(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-file" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-file") documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] @@ -140,9 +162,7 @@ def test_run_one_doc_per_file_with_meta(self, samples_path): def test_run_one_doc_per_page_with_meta(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-page" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-page") documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] assert len(documents) == 4 @@ -156,9 +176,7 @@ def test_run_one_doc_per_page_with_meta(self, samples_path): def test_run_one_doc_per_element_with_meta(self, samples_path): pdf_path = samples_path / "sample_pdf.pdf" meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-element" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] @@ -182,9 +200,7 @@ def test_run_one_doc_per_element_with_meta_list_two_files(self, samples_path): {"custom_meta": "sample_pdf.pdf", "common_meta": "common"}, {"custom_meta": "sample_pdf2.pdf", "common_meta": "common"}, ] - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-element" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") documents = local_converter.run(paths=pdf_path, meta=meta)["documents"] @@ -202,9 +218,7 @@ def test_run_one_doc_per_element_with_meta_list_two_files(self, samples_path): def test_run_one_doc_per_element_with_meta_list_folder_fail(self, samples_path): pdf_path = [samples_path] meta = [{"custom_meta": "foobar", "common_meta": "common"}, {"other_meta": "barfoo", "common_meta": "common"}] - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-element" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") with pytest.raises(ValueError): local_converter.run(paths=pdf_path, meta=meta)["documents"] @@ -213,9 +227,7 @@ def test_run_one_doc_per_element_with_meta_list_folder(self, samples_path): pdf_path = [samples_path] meta = {"common_meta": "common"} - local_converter = UnstructuredFileConverter( - api_url="http://localhost:8000/general/v0/general", document_creation_mode="one-doc-per-element" - ) + local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") documents = local_converter.run(paths=pdf_path, meta=meta)["documents"] @@ -227,3 +239,144 @@ def test_run_one_doc_per_element_with_meta_list_folder(self, samples_path): assert "category" in doc.meta assert "common_meta" in doc.meta assert doc.meta["common_meta"] == "common" + + +class TestCreateDocuments: + """`_create_documents` is a pure static method: elements in, Haystack Documents out.""" + + def test_one_doc_per_file_joins_all_elements_with_the_separator(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[_element("first"), _element("second"), _element("third")], + document_creation_mode="one-doc-per-file", + separator="|", + meta={"key": "value"}, + ) + + assert len(docs) == 1 + assert docs[0].content == "first|second|third" + assert docs[0].meta == {"key": "value", "file_path": "a/file.pdf"} + + def test_one_doc_per_page_groups_elements_by_page_number(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[ + _element("page one, first", page_number=1), + _element("page two", page_number=2), + _element("page one, second", page_number=1), + ], + document_creation_mode="one-doc-per-page", + separator="|", + meta={}, + ) + + assert len(docs) == 2 + assert docs[0].content == "page one, first|page one, second|" + assert docs[0].meta["page_number"] == 1 + assert docs[1].content == "page two|" + assert docs[1].meta["page_number"] == 2 + + def test_one_doc_per_element_records_the_index_and_category_of_each_element(self): + docs = UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[Text("body text"), Title("A Heading")], + document_creation_mode="one-doc-per-element", + separator="\n\n", + meta={"key": "value"}, + ) + + assert len(docs) == 2 + assert docs[0].content == "body text" + assert docs[0].meta["element_index"] == 0 + assert docs[0].meta["category"] == "UncategorizedText" + assert docs[1].content == "A Heading" + assert docs[1].meta["element_index"] == 1 + assert docs[1].meta["category"] == "Title" + assert all(doc.meta["key"] == "value" for doc in docs) + + @pytest.mark.parametrize("document_creation_mode", ["one-doc-per-file", "one-doc-per-page", "one-doc-per-element"]) + def test_the_caller_metadata_is_never_mutated(self, document_creation_mode): + meta = {"key": "value"} + + UnstructuredFileConverter._create_documents( + filepath=Path("a/file.pdf"), + elements=[_element("text", page_number=1)], + document_creation_mode=document_creation_mode, + separator="\n\n", + meta=meta, + ) + + assert meta == {"key": "value"} + + +class TestPartitionFileIntoElements: + def test_forwards_the_api_settings_and_the_extra_kwargs(self): + converter = UnstructuredFileConverter( + api_url=LOCAL_API_URL, + api_key=Secret.from_token("secret-key"), + unstructured_kwargs={"strategy": "hi_res"}, + ) + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) + + assert len(elements) == 1 + mock_partition.assert_called_once_with( + filename="a/file.pdf", + api_url=LOCAL_API_URL, + api_key="secret-key", + strategy="hi_res", + ) + + def test_returns_no_elements_and_warns_when_the_api_call_fails(self, converter, caplog): + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.side_effect = RuntimeError("API is down") + elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) + + assert elements == [] + assert "a/file.pdf" in caplog.text + assert "API is down" in caplog.text + + +class TestRun: + def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, converter, tmp_path): + (tmp_path / "first.txt").write_text("first") + (tmp_path / "second.txt").write_text("second") + # the directory is globbed non-recursively, so neither the subdirectory itself + # nor the file inside it is converted + nested = tmp_path / "nested.dir" + nested.mkdir() + (nested / "inner.txt").write_text("inner") + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + documents = converter.run(paths=[tmp_path])["documents"] + + assert len(documents) == 2 + assert mock_partition.call_count == 2 + + def test_zips_a_metadata_list_with_the_given_file_paths(self, converter, tmp_path): + first = tmp_path / "first.txt" + first.write_text("first") + second = tmp_path / "second.txt" + second.write_text("second") + + with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: + mock_partition.return_value = [_element("text")] + documents = converter.run(paths=[first, second], meta=[{"source": "one"}, {"source": "two"}])["documents"] + + assert [doc.meta["source"] for doc in documents] == ["one", "two"] + + def test_rejects_a_metadata_list_when_paths_contain_a_directory(self, converter, tmp_path): + (tmp_path / "first.txt").write_text("first") + + with pytest.raises(ValueError, match="`meta` can only be a dictionary"): + converter.run(paths=[tmp_path], meta=[{"source": "one"}]) + + def test_rejects_a_metadata_list_whose_length_does_not_match_the_paths(self, converter, tmp_path): + first = tmp_path / "first.txt" + first.write_text("first") + + with pytest.raises(ValueError, match="length of the metadata list"): + converter.run(paths=[first], meta=[{"source": "one"}, {"source": "two"}]) diff --git a/integrations/unstructured/tests/test_converter_unit.py b/integrations/unstructured/tests/test_converter_unit.py deleted file mode 100644 index 0ed6986b86..0000000000 --- a/integrations/unstructured/tests/test_converter_unit.py +++ /dev/null @@ -1,175 +0,0 @@ -# SPDX-FileCopyrightText: 2023-present deepset GmbH -# -# SPDX-License-Identifier: Apache-2.0 -from pathlib import Path -from unittest.mock import patch - -import pytest -from haystack.utils import Secret -from unstructured.documents.elements import ElementMetadata, Text, Title - -from haystack_integrations.components.converters.unstructured import UnstructuredFileConverter - -CONVERTER_MODULE = "haystack_integrations.components.converters.unstructured.converter" - -LOCAL_API_URL = "http://localhost:8000/general/v0/general" - - -def _element(text: str, **metadata) -> Text: - """Build a real Unstructured element, so the fakes stay honest against the SDK.""" - return Text(text, metadata=ElementMetadata(**metadata)) - - -@pytest.fixture -def converter() -> UnstructuredFileConverter: - """A converter pointing at a local API, so no API key is required.""" - return UnstructuredFileConverter(api_url=LOCAL_API_URL) - - -class TestCreateDocuments: - """`_create_documents` is a pure static method: elements in, Haystack Documents out.""" - - def test_one_doc_per_file_joins_all_elements_with_the_separator(self): - docs = UnstructuredFileConverter._create_documents( - filepath=Path("a/file.pdf"), - elements=[_element("first"), _element("second"), _element("third")], - document_creation_mode="one-doc-per-file", - separator="|", - meta={"key": "value"}, - ) - - assert len(docs) == 1 - assert docs[0].content == "first|second|third" - assert docs[0].meta == {"key": "value", "file_path": "a/file.pdf"} - - def test_one_doc_per_page_groups_elements_by_page_number(self): - docs = UnstructuredFileConverter._create_documents( - filepath=Path("a/file.pdf"), - elements=[ - _element("page one, first", page_number=1), - _element("page two", page_number=2), - _element("page one, second", page_number=1), - ], - document_creation_mode="one-doc-per-page", - separator="|", - meta={}, - ) - - assert len(docs) == 2 - assert docs[0].content == "page one, first|page one, second|" - assert docs[0].meta["page_number"] == 1 - assert docs[1].content == "page two|" - assert docs[1].meta["page_number"] == 2 - - def test_one_doc_per_element_records_the_index_and_category_of_each_element(self): - docs = UnstructuredFileConverter._create_documents( - filepath=Path("a/file.pdf"), - elements=[Text("body text"), Title("A Heading")], - document_creation_mode="one-doc-per-element", - separator="\n\n", - meta={"key": "value"}, - ) - - assert len(docs) == 2 - assert docs[0].content == "body text" - assert docs[0].meta["element_index"] == 0 - assert docs[0].meta["category"] == "UncategorizedText" - assert docs[1].content == "A Heading" - assert docs[1].meta["element_index"] == 1 - assert docs[1].meta["category"] == "Title" - assert all(doc.meta["key"] == "value" for doc in docs) - - @pytest.mark.parametrize("document_creation_mode", ["one-doc-per-file", "one-doc-per-page", "one-doc-per-element"]) - def test_the_caller_metadata_is_never_mutated(self, document_creation_mode): - meta = {"key": "value"} - - UnstructuredFileConverter._create_documents( - filepath=Path("a/file.pdf"), - elements=[_element("text", page_number=1)], - document_creation_mode=document_creation_mode, - separator="\n\n", - meta=meta, - ) - - assert meta == {"key": "value"} - - -class TestPartitionFileIntoElements: - def test_forwards_the_api_settings_and_the_extra_kwargs(self): - converter = UnstructuredFileConverter( - api_url=LOCAL_API_URL, - api_key=Secret.from_token("secret-key"), - unstructured_kwargs={"strategy": "hi_res"}, - ) - - with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: - mock_partition.return_value = [_element("text")] - elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) - - assert len(elements) == 1 - mock_partition.assert_called_once_with( - filename="a/file.pdf", - api_url=LOCAL_API_URL, - api_key="secret-key", - strategy="hi_res", - ) - - def test_returns_no_elements_and_warns_when_the_api_call_fails(self, converter, caplog): - with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: - mock_partition.side_effect = RuntimeError("API is down") - elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) - - assert elements == [] - assert "a/file.pdf" in caplog.text - assert "API is down" in caplog.text - - -class TestRun: - def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, converter, tmp_path): - (tmp_path / "first.txt").write_text("first") - (tmp_path / "second.txt").write_text("second") - (tmp_path / "nested.dir").mkdir() - - with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: - mock_partition.return_value = [_element("text")] - documents = converter.run(paths=[tmp_path])["documents"] - - assert len(documents) == 2 - assert mock_partition.call_count == 2 - - def test_zips_a_metadata_list_with_the_given_file_paths(self, converter, tmp_path): - first = tmp_path / "first.txt" - first.write_text("first") - second = tmp_path / "second.txt" - second.write_text("second") - - with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: - mock_partition.return_value = [_element("text")] - documents = converter.run(paths=[first, second], meta=[{"source": "one"}, {"source": "two"}])["documents"] - - assert [doc.meta["source"] for doc in documents] == ["one", "two"] - - def test_rejects_a_metadata_list_when_paths_contain_a_directory(self, converter, tmp_path): - (tmp_path / "first.txt").write_text("first") - - with pytest.raises(ValueError, match="`meta` can only be a dictionary"): - converter.run(paths=[tmp_path], meta=[{"source": "one"}]) - - def test_rejects_a_metadata_list_whose_length_does_not_match_the_paths(self, converter, tmp_path): - first = tmp_path / "first.txt" - first.write_text("first") - - with pytest.raises(ValueError): - converter.run(paths=[first], meta=[{"source": "one"}, {"source": "two"}]) - - -class TestSerializationWithoutApiKey: - def test_to_dict_and_from_dict_round_trip_a_none_api_key(self): - converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, api_key=None) - - converter_dict = converter.to_dict() - assert converter_dict["init_parameters"]["api_key"] is None - - deserialized = UnstructuredFileConverter.from_dict(converter_dict) - assert deserialized.api_key is None - assert deserialized.api_url == LOCAL_API_URL From ef898977baeeaf8258a9f9584da5ab307e6b770d Mon Sep 17 00:00:00 2001 From: Julian Risch Date: Thu, 20 Aug 2026 19:11:52 +0200 Subject: [PATCH 3/3] test: split the Unstructured test suite into focused classes Address @anakin87's review on #3794: - Rename the `converter` fixture to `local_converter` and move it to conftest.py, next to the other shared fixtures. It comes with a `local_api_url` fixture, so the tests that build their own converter with a specific `document_creation_mode` no longer repeat the URL literal, and their local variables can just be called `converter` without shadowing the fixture name. - Split `TestUnstructuredFileConverter` into `TestInit`, `TestSerde` and `TestRunIntegration`, matching the focused classes added for the new unit tests. `TestRunIntegration` carries the `integration` marker on the class instead of repeating it on all nine tests. - Reuse a `HOSTED_API_URL` constant for the hosted default, and pin the error message in the pre-existing folder-plus-meta-list test, which was the last bare `pytest.raises(ValueError)` in the suite. Co-Authored-By: Claude Opus 5 (1M context) --- integrations/unstructured/tests/conftest.py | 16 + .../unstructured/tests/test_converter.py | 320 +++++++++--------- 2 files changed, 173 insertions(+), 163 deletions(-) diff --git a/integrations/unstructured/tests/conftest.py b/integrations/unstructured/tests/conftest.py index fa02cc5dda..6694105ed6 100644 --- a/integrations/unstructured/tests/conftest.py +++ b/integrations/unstructured/tests/conftest.py @@ -2,6 +2,10 @@ import pytest +from haystack_integrations.components.converters.unstructured import UnstructuredFileConverter + +LOCAL_API_URL = "http://localhost:8000/general/v0/general" + @pytest.fixture def set_env_variables(monkeypatch): @@ -11,3 +15,15 @@ def set_env_variables(monkeypatch): @pytest.fixture def samples_path(): return Path(__file__).parent / "samples" + + +@pytest.fixture +def local_api_url() -> str: + """URL of an Unstructured API running locally, which needs no API key.""" + return LOCAL_API_URL + + +@pytest.fixture +def local_converter(local_api_url) -> UnstructuredFileConverter: + """A converter pointing at a local API, so no API key is required.""" + return UnstructuredFileConverter(api_url=local_api_url) diff --git a/integrations/unstructured/tests/test_converter.py b/integrations/unstructured/tests/test_converter.py index 8e92d98791..a88738afd0 100644 --- a/integrations/unstructured/tests/test_converter.py +++ b/integrations/unstructured/tests/test_converter.py @@ -12,7 +12,7 @@ CONVERTER_MODULE = "haystack_integrations.components.converters.unstructured.converter" -LOCAL_API_URL = "http://localhost:8000/general/v0/general" +HOSTED_API_URL = "https://api.unstructured.io/general/v0/general" def _element(text: str, **metadata) -> Text: @@ -20,17 +20,11 @@ def _element(text: str, **metadata) -> Text: return Text(text, metadata=ElementMetadata(**metadata)) -@pytest.fixture -def converter() -> UnstructuredFileConverter: - """A converter pointing at a local API, so no API key is required.""" - return UnstructuredFileConverter(api_url=LOCAL_API_URL) - - -class TestUnstructuredFileConverter: +class TestInit: @pytest.mark.usefixtures("set_env_variables") def test_init_default(self): converter = UnstructuredFileConverter() - assert converter.api_url == "https://api.unstructured.io/general/v0/general" + assert converter.api_url == HOSTED_API_URL assert converter.api_key.resolve_value() == "test-api-key" assert converter.document_creation_mode == "one-doc-per-file" assert converter.separator == "\n\n" @@ -54,8 +48,10 @@ def test_init_with_parameters(self): def test_init_hosted_without_api_key_raises_error(self): with pytest.raises(ValueError): - UnstructuredFileConverter(api_url="https://api.unstructured.io/general/v0/general") + UnstructuredFileConverter(api_url=HOSTED_API_URL) + +class TestSerde: @pytest.mark.usefixtures("set_env_variables") def test_to_dict(self): converter = UnstructuredFileConverter() @@ -64,7 +60,7 @@ def test_to_dict(self): assert converter_dict == { "type": "haystack_integrations.components.converters.unstructured.converter.UnstructuredFileConverter", "init_parameters": { - "api_url": "https://api.unstructured.io/general/v0/general", + "api_url": HOSTED_API_URL, "api_key": {"env_vars": ["UNSTRUCTURED_API_KEY"], "strict": False, "type": "env_var"}, "document_creation_mode": "one-doc-per-file", "separator": "\n\n", @@ -94,151 +90,15 @@ def test_from_dict(self, monkeypatch): assert converter.unstructured_kwargs == {"foo": "bar"} assert not converter.progress_bar - def test_to_dict_and_from_dict_round_trip_a_none_api_key(self): - converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, api_key=None) + def test_to_dict_and_from_dict_round_trip_a_none_api_key(self, local_api_url): + converter = UnstructuredFileConverter(api_url=local_api_url, api_key=None) converter_dict = converter.to_dict() assert converter_dict["init_parameters"]["api_key"] is None deserialized = UnstructuredFileConverter.from_dict(converter_dict) assert deserialized.api_key is None - assert deserialized.api_url == LOCAL_API_URL - - @pytest.mark.integration - def test_run_one_doc_per_file(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-file") - - documents = local_converter.run([pdf_path])["documents"] - - assert len(documents) == 1 - assert documents[0].meta == {"file_path": str(pdf_path)} - - @pytest.mark.integration - def test_run_one_doc_per_page(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-page") - - documents = local_converter.run([pdf_path])["documents"] - - assert len(documents) == 4 - for i, doc in enumerate(documents, start=1): - assert doc.meta["file_path"] == str(pdf_path) - assert doc.meta["page_number"] == i - - @pytest.mark.integration - def test_run_one_doc_per_element(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") - - documents = local_converter.run([pdf_path])["documents"] - - assert len(documents) > 4 - for doc in documents: - assert doc.meta["file_path"] == str(pdf_path) - assert "page_number" in doc.meta - - # elements have a category attribute that is saved in the document meta - assert "category" in doc.meta - - @pytest.mark.integration - def test_run_one_doc_per_file_with_meta(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-file") - - documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] - - assert len(documents) == 1 - assert documents[0].meta["file_path"] == str(pdf_path) - assert "custom_meta" in documents[0].meta - assert documents[0].meta["custom_meta"] == "foobar" - assert documents[0].meta == {"file_path": str(pdf_path), "custom_meta": "foobar"} - - @pytest.mark.integration - def test_run_one_doc_per_page_with_meta(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-page") - - documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] - assert len(documents) == 4 - for i, doc in enumerate(documents, start=1): - assert doc.meta["file_path"] == str(pdf_path) - assert doc.meta["page_number"] == i - assert "custom_meta" in doc.meta - assert doc.meta["custom_meta"] == "foobar" - - @pytest.mark.integration - def test_run_one_doc_per_element_with_meta(self, samples_path): - pdf_path = samples_path / "sample_pdf.pdf" - meta = {"custom_meta": "foobar"} - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") - - documents = local_converter.run(paths=[pdf_path], meta=meta)["documents"] - - assert len(documents) > 4 - first_element_index = 0 - for doc in documents: - assert doc.meta["file_path"] == str(pdf_path) - assert "page_number" in doc.meta - - # elements have a category attribute that is saved in the document meta - assert "category" in doc.meta - assert "custom_meta" in doc.meta - assert doc.meta["custom_meta"] == "foobar" - assert doc.meta["element_index"] == first_element_index - first_element_index += 1 - - @pytest.mark.integration - def test_run_one_doc_per_element_with_meta_list_two_files(self, samples_path): - pdf_path = [samples_path / "sample_pdf.pdf", samples_path / "sample_pdf2.pdf"] - meta = [ - {"custom_meta": "sample_pdf.pdf", "common_meta": "common"}, - {"custom_meta": "sample_pdf2.pdf", "common_meta": "common"}, - ] - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") - - documents = local_converter.run(paths=pdf_path, meta=meta)["documents"] - - assert len(documents) > 4 - for doc in documents: - assert doc.meta["custom_meta"] == doc.meta["filename"] - assert "file_path" in doc.meta - assert "page_number" in doc.meta - # elements have a category attribute that is saved in the document meta - assert "category" in doc.meta - assert "common_meta" in doc.meta - assert doc.meta["common_meta"] == "common" - - @pytest.mark.integration - def test_run_one_doc_per_element_with_meta_list_folder_fail(self, samples_path): - pdf_path = [samples_path] - meta = [{"custom_meta": "foobar", "common_meta": "common"}, {"other_meta": "barfoo", "common_meta": "common"}] - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") - with pytest.raises(ValueError): - local_converter.run(paths=pdf_path, meta=meta)["documents"] - - @pytest.mark.integration - def test_run_one_doc_per_element_with_meta_list_folder(self, samples_path): - pdf_path = [samples_path] - meta = {"common_meta": "common"} - - local_converter = UnstructuredFileConverter(api_url=LOCAL_API_URL, document_creation_mode="one-doc-per-element") - - documents = local_converter.run(paths=pdf_path, meta=meta)["documents"] - - assert len(documents) > 4 - for doc in documents: - assert "file_path" in doc.meta - assert "page_number" in doc.meta - # elements have a category attribute that is saved in the document meta - assert "category" in doc.meta - assert "common_meta" in doc.meta - assert doc.meta["common_meta"] == "common" + assert deserialized.api_url == local_api_url class TestCreateDocuments: @@ -310,9 +170,9 @@ def test_the_caller_metadata_is_never_mutated(self, document_creation_mode): class TestPartitionFileIntoElements: - def test_forwards_the_api_settings_and_the_extra_kwargs(self): + def test_forwards_the_api_settings_and_the_extra_kwargs(self, local_api_url): converter = UnstructuredFileConverter( - api_url=LOCAL_API_URL, + api_url=local_api_url, api_key=Secret.from_token("secret-key"), unstructured_kwargs={"strategy": "hi_res"}, ) @@ -324,15 +184,15 @@ def test_forwards_the_api_settings_and_the_extra_kwargs(self): assert len(elements) == 1 mock_partition.assert_called_once_with( filename="a/file.pdf", - api_url=LOCAL_API_URL, + api_url=local_api_url, api_key="secret-key", strategy="hi_res", ) - def test_returns_no_elements_and_warns_when_the_api_call_fails(self, converter, caplog): + def test_returns_no_elements_and_warns_when_the_api_call_fails(self, local_converter, caplog): with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: mock_partition.side_effect = RuntimeError("API is down") - elements = converter._partition_file_into_elements(filepath=Path("a/file.pdf")) + elements = local_converter._partition_file_into_elements(filepath=Path("a/file.pdf")) assert elements == [] assert "a/file.pdf" in caplog.text @@ -340,7 +200,7 @@ def test_returns_no_elements_and_warns_when_the_api_call_fails(self, converter, class TestRun: - def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, converter, tmp_path): + def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, local_converter, tmp_path): (tmp_path / "first.txt").write_text("first") (tmp_path / "second.txt").write_text("second") # the directory is globbed non-recursively, so neither the subdirectory itself @@ -351,12 +211,12 @@ def test_converts_every_file_in_a_directory_and_ignores_subdirectories(self, con with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: mock_partition.return_value = [_element("text")] - documents = converter.run(paths=[tmp_path])["documents"] + documents = local_converter.run(paths=[tmp_path])["documents"] assert len(documents) == 2 assert mock_partition.call_count == 2 - def test_zips_a_metadata_list_with_the_given_file_paths(self, converter, tmp_path): + def test_zips_a_metadata_list_with_the_given_file_paths(self, local_converter, tmp_path): first = tmp_path / "first.txt" first.write_text("first") second = tmp_path / "second.txt" @@ -364,19 +224,153 @@ def test_zips_a_metadata_list_with_the_given_file_paths(self, converter, tmp_pat with patch(f"{CONVERTER_MODULE}.partition_via_api") as mock_partition: mock_partition.return_value = [_element("text")] - documents = converter.run(paths=[first, second], meta=[{"source": "one"}, {"source": "two"}])["documents"] + documents = local_converter.run(paths=[first, second], meta=[{"source": "one"}, {"source": "two"}])[ + "documents" + ] assert [doc.meta["source"] for doc in documents] == ["one", "two"] - def test_rejects_a_metadata_list_when_paths_contain_a_directory(self, converter, tmp_path): + def test_rejects_a_metadata_list_when_paths_contain_a_directory(self, local_converter, tmp_path): (tmp_path / "first.txt").write_text("first") with pytest.raises(ValueError, match="`meta` can only be a dictionary"): - converter.run(paths=[tmp_path], meta=[{"source": "one"}]) + local_converter.run(paths=[tmp_path], meta=[{"source": "one"}]) - def test_rejects_a_metadata_list_whose_length_does_not_match_the_paths(self, converter, tmp_path): + def test_rejects_a_metadata_list_whose_length_does_not_match_the_paths(self, local_converter, tmp_path): first = tmp_path / "first.txt" first.write_text("first") with pytest.raises(ValueError, match="length of the metadata list"): - converter.run(paths=[first], meta=[{"source": "one"}, {"source": "two"}]) + local_converter.run(paths=[first], meta=[{"source": "one"}, {"source": "two"}]) + + +@pytest.mark.integration +class TestRunIntegration: + """These tests need an Unstructured API running locally, see the README.""" + + def test_run_one_doc_per_file(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-file") + + documents = converter.run([pdf_path])["documents"] + + assert len(documents) == 1 + assert documents[0].meta == {"file_path": str(pdf_path)} + + def test_run_one_doc_per_page(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-page") + + documents = converter.run([pdf_path])["documents"] + + assert len(documents) == 4 + for i, doc in enumerate(documents, start=1): + assert doc.meta["file_path"] == str(pdf_path) + assert doc.meta["page_number"] == i + + def test_run_one_doc_per_element(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-element") + + documents = converter.run([pdf_path])["documents"] + + assert len(documents) > 4 + for doc in documents: + assert doc.meta["file_path"] == str(pdf_path) + assert "page_number" in doc.meta + + # elements have a category attribute that is saved in the document meta + assert "category" in doc.meta + + def test_run_one_doc_per_file_with_meta(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + meta = {"custom_meta": "foobar"} + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-file") + + documents = converter.run(paths=[pdf_path], meta=meta)["documents"] + + assert len(documents) == 1 + assert documents[0].meta["file_path"] == str(pdf_path) + assert "custom_meta" in documents[0].meta + assert documents[0].meta["custom_meta"] == "foobar" + assert documents[0].meta == {"file_path": str(pdf_path), "custom_meta": "foobar"} + + def test_run_one_doc_per_page_with_meta(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + meta = {"custom_meta": "foobar"} + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-page") + + documents = converter.run(paths=[pdf_path], meta=meta)["documents"] + assert len(documents) == 4 + for i, doc in enumerate(documents, start=1): + assert doc.meta["file_path"] == str(pdf_path) + assert doc.meta["page_number"] == i + assert "custom_meta" in doc.meta + assert doc.meta["custom_meta"] == "foobar" + + def test_run_one_doc_per_element_with_meta(self, samples_path, local_api_url): + pdf_path = samples_path / "sample_pdf.pdf" + meta = {"custom_meta": "foobar"} + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-element") + + documents = converter.run(paths=[pdf_path], meta=meta)["documents"] + + assert len(documents) > 4 + first_element_index = 0 + for doc in documents: + assert doc.meta["file_path"] == str(pdf_path) + assert "page_number" in doc.meta + + # elements have a category attribute that is saved in the document meta + assert "category" in doc.meta + assert "custom_meta" in doc.meta + assert doc.meta["custom_meta"] == "foobar" + assert doc.meta["element_index"] == first_element_index + first_element_index += 1 + + def test_run_one_doc_per_element_with_meta_list_two_files(self, samples_path, local_api_url): + pdf_path = [samples_path / "sample_pdf.pdf", samples_path / "sample_pdf2.pdf"] + meta = [ + {"custom_meta": "sample_pdf.pdf", "common_meta": "common"}, + {"custom_meta": "sample_pdf2.pdf", "common_meta": "common"}, + ] + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-element") + + documents = converter.run(paths=pdf_path, meta=meta)["documents"] + + assert len(documents) > 4 + for doc in documents: + assert doc.meta["custom_meta"] == doc.meta["filename"] + assert "file_path" in doc.meta + assert "page_number" in doc.meta + # elements have a category attribute that is saved in the document meta + assert "category" in doc.meta + assert "common_meta" in doc.meta + assert doc.meta["common_meta"] == "common" + + def test_run_one_doc_per_element_with_meta_list_folder_fail(self, samples_path, local_api_url): + pdf_path = [samples_path] + meta = [{"custom_meta": "foobar", "common_meta": "common"}, {"other_meta": "barfoo", "common_meta": "common"}] + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-element") + with pytest.raises(ValueError, match="`meta` can only be a dictionary"): + converter.run(paths=pdf_path, meta=meta)["documents"] + + def test_run_one_doc_per_element_with_meta_list_folder(self, samples_path, local_api_url): + pdf_path = [samples_path] + meta = {"common_meta": "common"} + + converter = UnstructuredFileConverter(api_url=local_api_url, document_creation_mode="one-doc-per-element") + + documents = converter.run(paths=pdf_path, meta=meta)["documents"] + + assert len(documents) > 4 + for doc in documents: + assert "file_path" in doc.meta + assert "page_number" in doc.meta + # elements have a category attribute that is saved in the document meta + assert "category" in doc.meta + assert "common_meta" in doc.meta + assert doc.meta["common_meta"] == "common"