test: Unstructured - increase unit tests coverage - #3794
Merged
Conversation
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) <noreply@anthropic.com>
github-actions
Bot
added
integration:unstructured-fileconverter
type:documentation
Contributor
Coverage report (unstructured)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
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) <noreply@anthropic.com>
anakin87
reviewed
Aug 20, 2026
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) <noreply@anthropic.com>
julian-risch
added a commit
that referenced
this pull request
Aug 20, 2026
Drop the separate test_converter_unit.py and merge everything into test_converter.py, split into focused classes rather than one big class: TestInit, TestSerde, TestGetConversionArgs, TestValidateArgs, TestRun, TestRunAsync, TestResolveMimeType and TestRunIntegration. This follows the review on #3794, down to the class naming, and hoists the integration and asyncio markers onto the classes instead of repeating them per method. Cover the behaviour the to_dict() fix protects, which was only asserted at the attribute level before: - run and run_async pick up output_file_type from __init__ when the argument is omitted, and the argument wins when both are set - a converter that has been through to_dict() -> from_dict() still converts without an output_file_type argument, so reverting the fix fails the test - from_dict() still accepts dictionaries serialized before output_file_type was included - both ValueError paths when output_file_type is set nowhere, and the _validate_args branch for a type outside OUTPUT_FILE_TYPE entirely Rename the `converter` fixture to `real_converter`, since next to `mock_converter` the bare name did not convey that it needs an actual LibreOffice installation, and give the remaining fixtures docstrings. Unit coverage: 93% -> 100%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Proposed Changes:
_create_documents(was 0%), the path/directory expansion inrunand both of itsValueErrorpaths, and_partition_file_into_elementsincluding its swallow-and-warn failure path, by patchingpartition_via_api.unstructuredElementobjects rather than mocks, so they stay honest against the SDK.--strict-markers, which this integration was missing.How did you test it?
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.