chore: import upstream snapshot with attribution
This commit is contained in:
@@ -0,0 +1,23 @@
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def input_pdf():
|
||||
return Path(__file__).resolve().parents[3] / "samples" / "pdf" / "1901.03003.pdf"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def output_dir():
|
||||
path = (
|
||||
Path(__file__).resolve().parents[3]
|
||||
/ "python"
|
||||
/ "opendataloader-pdf"
|
||||
/ "tests"
|
||||
/ "temp"
|
||||
)
|
||||
path.mkdir(exist_ok=True)
|
||||
yield path
|
||||
shutil.rmtree(path, ignore_errors=True)
|
||||
@@ -0,0 +1,142 @@
|
||||
"""Unit tests for auto-generated cli_options module"""
|
||||
|
||||
import pytest
|
||||
from opendataloader_pdf.cli_options_generated import CLI_OPTIONS, add_options_to_parser
|
||||
|
||||
|
||||
class TestCLIOptions:
|
||||
"""Tests for CLI_OPTIONS metadata list"""
|
||||
|
||||
def test_cli_options_is_list(self):
|
||||
"""CLI_OPTIONS should be a list"""
|
||||
assert isinstance(CLI_OPTIONS, list)
|
||||
|
||||
def test_cli_options_not_empty(self):
|
||||
"""CLI_OPTIONS should not be empty"""
|
||||
assert len(CLI_OPTIONS) > 0
|
||||
|
||||
def test_each_option_has_required_fields(self):
|
||||
"""Each option should have all required fields"""
|
||||
required_fields = [
|
||||
"name",
|
||||
"python_name",
|
||||
"short_name",
|
||||
"type",
|
||||
"required",
|
||||
"default",
|
||||
"description",
|
||||
]
|
||||
for opt in CLI_OPTIONS:
|
||||
for field in required_fields:
|
||||
assert field in opt, f"Option {opt.get('name', 'unknown')} missing field: {field}"
|
||||
|
||||
def test_option_types_are_valid(self):
|
||||
"""Option types should be 'string' or 'boolean'"""
|
||||
valid_types = {"string", "boolean"}
|
||||
for opt in CLI_OPTIONS:
|
||||
assert opt["type"] in valid_types, f"Invalid type for {opt['name']}: {opt['type']}"
|
||||
|
||||
def test_python_name_is_snake_case(self):
|
||||
"""Python names should be snake_case (no hyphens)"""
|
||||
for opt in CLI_OPTIONS:
|
||||
assert "-" not in opt["python_name"], f"Python name should not contain hyphen: {opt['python_name']}"
|
||||
|
||||
def test_known_options_exist(self):
|
||||
"""Known options should exist in the list"""
|
||||
option_names = {opt["name"] for opt in CLI_OPTIONS}
|
||||
expected_options = {
|
||||
"output-dir",
|
||||
"password",
|
||||
"format",
|
||||
"quiet",
|
||||
"content-safety-off",
|
||||
"keep-line-breaks",
|
||||
"image-output",
|
||||
"image-format",
|
||||
}
|
||||
for expected in expected_options:
|
||||
assert expected in option_names, f"Expected option not found: {expected}"
|
||||
|
||||
def test_sanitize_option_exists(self):
|
||||
option_names = [opt["name"] for opt in CLI_OPTIONS]
|
||||
assert "sanitize" in option_names
|
||||
sanitize_opt = next(opt for opt in CLI_OPTIONS if opt["name"] == "sanitize")
|
||||
assert sanitize_opt["type"] == "boolean"
|
||||
assert sanitize_opt["default"] == False
|
||||
|
||||
|
||||
|
||||
class TestAddOptionsToParser:
|
||||
"""Tests for add_options_to_parser function"""
|
||||
|
||||
def test_adds_all_options(self):
|
||||
"""Should add all options to argparse parser"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
add_options_to_parser(parser)
|
||||
|
||||
# Parse empty args to get defaults
|
||||
args = parser.parse_args([])
|
||||
|
||||
# Check that all options are added
|
||||
for opt in CLI_OPTIONS:
|
||||
python_name = opt["python_name"]
|
||||
assert hasattr(args, python_name.replace("-", "_")), f"Option {python_name} not added to parser"
|
||||
|
||||
def test_boolean_options_default_to_false(self):
|
||||
"""Boolean options should default to False"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
add_options_to_parser(parser)
|
||||
args = parser.parse_args([])
|
||||
|
||||
for opt in CLI_OPTIONS:
|
||||
if opt["type"] == "boolean":
|
||||
python_name = opt["python_name"].replace("-", "_")
|
||||
assert getattr(args, python_name) is False, f"Boolean option {python_name} should default to False"
|
||||
|
||||
def test_string_options_default_to_none(self):
|
||||
"""String options should default to None"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
add_options_to_parser(parser)
|
||||
args = parser.parse_args([])
|
||||
|
||||
for opt in CLI_OPTIONS:
|
||||
if opt["type"] == "string":
|
||||
python_name = opt["python_name"].replace("-", "_")
|
||||
assert getattr(args, python_name) is None, f"String option {python_name} should default to None"
|
||||
|
||||
def test_short_options_work(self):
|
||||
"""Short option flags should work"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
add_options_to_parser(parser)
|
||||
|
||||
# Test with -o (short for --output-dir)
|
||||
args = parser.parse_args(["-o", "/output"])
|
||||
assert args.output_dir == "/output"
|
||||
|
||||
# Test with -f (short for --format)
|
||||
args = parser.parse_args(["-f", "json"])
|
||||
assert args.format == "json"
|
||||
|
||||
# Test with -q (short for --quiet)
|
||||
args = parser.parse_args(["-q"])
|
||||
assert args.quiet is True
|
||||
|
||||
def test_long_options_work(self):
|
||||
"""Long option flags should work"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
add_options_to_parser(parser)
|
||||
|
||||
args = parser.parse_args(["--output-dir", "/output", "--format", "json,markdown", "--quiet"])
|
||||
assert args.output_dir == "/output"
|
||||
assert args.format == "json,markdown"
|
||||
assert args.quiet is True
|
||||
@@ -0,0 +1,16 @@
|
||||
"""Integration tests that actually run the JAR (slow)"""
|
||||
|
||||
import opendataloader_pdf
|
||||
|
||||
|
||||
def test_convert_generates_output(input_pdf, output_dir):
|
||||
"""Verify that convert() actually generates output files"""
|
||||
opendataloader_pdf.convert(
|
||||
input_path=str(input_pdf),
|
||||
output_dir=str(output_dir),
|
||||
format="json",
|
||||
quiet=True,
|
||||
)
|
||||
output = output_dir / "1901.03003.json"
|
||||
assert output.exists(), f"Output file not found at {output}"
|
||||
assert output.stat().st_size > 0, "Output file is empty"
|
||||
@@ -0,0 +1,91 @@
|
||||
"""Tests for hybrid_server."""
|
||||
|
||||
import logging
|
||||
import sys
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
||||
def test_gpu_detected_logging(caplog):
|
||||
"""GPU detection should log GPU name and CUDA version when available."""
|
||||
mock_torch = MagicMock()
|
||||
mock_torch.cuda.is_available.return_value = True
|
||||
mock_torch.cuda.get_device_name.return_value = "NVIDIA A100"
|
||||
mock_torch.version.cuda = "12.1"
|
||||
|
||||
with patch.dict("sys.modules", {"torch": mock_torch}):
|
||||
# Re-import to pick up the mock
|
||||
import importlib
|
||||
from opendataloader_pdf import hybrid_server
|
||||
|
||||
importlib.reload(hybrid_server)
|
||||
|
||||
with caplog.at_level(logging.INFO):
|
||||
# Simulate the GPU detection block from main()
|
||||
try:
|
||||
import torch
|
||||
if torch.cuda.is_available():
|
||||
gpu_name = torch.cuda.get_device_name(0)
|
||||
cuda_version = torch.version.cuda
|
||||
logging.getLogger(__name__).info(
|
||||
f"GPU detected: {gpu_name} (CUDA {cuda_version})"
|
||||
)
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
assert "GPU detected: NVIDIA A100 (CUDA 12.1)" in caplog.text
|
||||
|
||||
|
||||
def test_no_gpu_logging(caplog):
|
||||
"""Should log CPU fallback when no GPU is available."""
|
||||
mock_torch = MagicMock()
|
||||
mock_torch.cuda.is_available.return_value = False
|
||||
|
||||
with patch.dict("sys.modules", {"torch": mock_torch}):
|
||||
with caplog.at_level(logging.INFO):
|
||||
try:
|
||||
import torch
|
||||
if torch.cuda.is_available():
|
||||
pass
|
||||
else:
|
||||
logging.getLogger(__name__).info("No GPU detected, using CPU.")
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
assert "No GPU detected, using CPU." in caplog.text
|
||||
|
||||
|
||||
def test_no_pytorch_logging(caplog):
|
||||
"""Should log CPU fallback when PyTorch is not installed."""
|
||||
with patch.dict("sys.modules", {"torch": None}):
|
||||
with caplog.at_level(logging.INFO):
|
||||
try:
|
||||
import torch # noqa: F811
|
||||
if torch.cuda.is_available():
|
||||
pass
|
||||
else:
|
||||
logging.getLogger(__name__).info("No GPU detected, using CPU.")
|
||||
except (ImportError, TypeError):
|
||||
logging.getLogger(__name__).info(
|
||||
"No GPU detected, using CPU. (PyTorch not installed)"
|
||||
)
|
||||
|
||||
assert "No GPU detected, using CPU. (PyTorch not installed)" in caplog.text
|
||||
|
||||
|
||||
def test_get_loop_setting_returns_asyncio_on_windows():
|
||||
"""On Windows, should return 'asyncio' to avoid uvloop errors (#323)."""
|
||||
from opendataloader_pdf.hybrid_server import _get_loop_setting
|
||||
|
||||
with patch("sys.platform", "win32"):
|
||||
assert _get_loop_setting() == "asyncio"
|
||||
|
||||
|
||||
def test_get_loop_setting_returns_auto_on_non_windows():
|
||||
"""On non-Windows platforms, should return 'auto' (uvloop if available)."""
|
||||
from opendataloader_pdf.hybrid_server import _get_loop_setting
|
||||
|
||||
with patch("sys.platform", "darwin"):
|
||||
assert _get_loop_setting() == "auto"
|
||||
|
||||
with patch("sys.platform", "linux"):
|
||||
assert _get_loop_setting() == "auto"
|
||||
@@ -0,0 +1,139 @@
|
||||
"""Tests for hybrid_server non-blocking conversion.
|
||||
|
||||
Verifies that converter.convert() runs in a thread pool rather than blocking
|
||||
the event loop. This is the root cause of issue #301: the Java client's
|
||||
3-second health check times out when the server is busy with a synchronous
|
||||
conversion call inside an async endpoint.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import threading
|
||||
import time
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_docling():
|
||||
"""Mock docling modules so tests don't need the actual dependency."""
|
||||
mock_converter = MagicMock()
|
||||
mock_result = MagicMock()
|
||||
mock_result.status.value = "success"
|
||||
mock_result.errors = []
|
||||
mock_result.input.page_count = 1
|
||||
mock_result.document.export_to_dict.return_value = {
|
||||
"pages": {"1": {}},
|
||||
"body": {},
|
||||
}
|
||||
|
||||
# Track which thread the conversion runs on
|
||||
convert_thread_name = {}
|
||||
|
||||
def tracking_convert(path, page_range=None):
|
||||
convert_thread_name["thread"] = threading.current_thread().name
|
||||
time.sleep(2)
|
||||
return mock_result
|
||||
|
||||
mock_converter.convert = tracking_convert
|
||||
mock_converter._convert_thread = convert_thread_name
|
||||
|
||||
mock_conversion_status = MagicMock()
|
||||
mock_conversion_status.PARTIAL_SUCCESS = "partial_success"
|
||||
|
||||
with patch.dict("sys.modules", {
|
||||
"docling": MagicMock(),
|
||||
"docling.datamodel.base_models": MagicMock(
|
||||
InputFormat=MagicMock(PDF="pdf"),
|
||||
ConversionStatus=mock_conversion_status,
|
||||
),
|
||||
"docling.datamodel.pipeline_options": MagicMock(),
|
||||
"docling.document_converter": MagicMock(),
|
||||
"uvicorn": MagicMock(),
|
||||
}):
|
||||
yield mock_converter
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_with_converter(mock_docling):
|
||||
"""Create a FastAPI app with the mock converter."""
|
||||
import importlib
|
||||
from opendataloader_pdf import hybrid_server
|
||||
|
||||
importlib.reload(hybrid_server)
|
||||
|
||||
app = hybrid_server.create_app()
|
||||
hybrid_server.converter = mock_docling
|
||||
return app
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_convert_runs_in_thread_pool(app_with_converter, mock_docling):
|
||||
"""converter.convert() must run in a worker thread, not on the event loop.
|
||||
|
||||
When converter.convert() runs directly on the async event loop thread,
|
||||
it blocks all concurrent request handling — including /health checks.
|
||||
The fix wraps converter.convert() with asyncio.to_thread() so it runs
|
||||
in a separate thread.
|
||||
|
||||
Reproduces issue #301.
|
||||
"""
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
|
||||
transport = ASGITransport(app=app_with_converter)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
event_loop_thread = threading.current_thread().name
|
||||
|
||||
response = await client.post(
|
||||
"/v1/convert/file",
|
||||
files={"files": ("test.pdf", b"%PDF-1.4 minimal", "application/pdf")},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
convert_thread = mock_docling._convert_thread.get("thread")
|
||||
assert convert_thread is not None, "converter.convert() was not called"
|
||||
assert convert_thread != event_loop_thread, (
|
||||
f"converter.convert() ran on the event loop thread '{event_loop_thread}'. "
|
||||
f"It must run in a worker thread via asyncio.to_thread() to avoid "
|
||||
f"blocking /health and other endpoints during long conversions."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_health_responds_during_conversion(app_with_converter):
|
||||
"""Health endpoint must respond quickly even during active conversion.
|
||||
|
||||
This is the user-facing symptom of issue #301: the Java CLI gets
|
||||
SocketTimeoutException when the hybrid server is busy processing
|
||||
another document. Mock sleep is 2s; health must respond under 0.2s.
|
||||
"""
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
|
||||
transport = ASGITransport(app=app_with_converter)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
# Start conversion (takes 2s in mock)
|
||||
convert_task = asyncio.create_task(
|
||||
client.post(
|
||||
"/v1/convert/file",
|
||||
files={"files": ("test.pdf", b"%PDF-1.4 minimal", "application/pdf")},
|
||||
)
|
||||
)
|
||||
|
||||
# Wait for conversion to start in the worker thread
|
||||
await asyncio.sleep(0.3)
|
||||
|
||||
# Health check should respond quickly — well under the 2s conversion
|
||||
start = time.monotonic()
|
||||
health_response = await client.get("/health")
|
||||
health_time = time.monotonic() - start
|
||||
|
||||
assert health_response.status_code == 200
|
||||
assert health_response.json() == {"status": "ok"}
|
||||
assert health_time < 0.2, (
|
||||
f"Health endpoint took {health_time:.2f}s during conversion. "
|
||||
f"Expected < 0.2s. The event loop is likely blocked."
|
||||
)
|
||||
|
||||
convert_response = await convert_task
|
||||
assert convert_response.status_code == 200
|
||||
@@ -0,0 +1,447 @@
|
||||
"""Tests for the OCR option surface added to the hybrid server.
|
||||
|
||||
Covers:
|
||||
* `create_converter` delegation to docling's `get_ocr_factory`
|
||||
* Defaults preserve prior behavior (do_ocr=True, engine=easyocr)
|
||||
* `disable_ocr=True` -> do_ocr=False (#387)
|
||||
* Engine selection produces the matching OcrOptions subclass (#436, #439)
|
||||
* `psm` is applied only for Tesseract engines (silently ignored otherwise)
|
||||
* Unknown / denylisted engines exit with a clear message
|
||||
* argparse `--no-ocr` and `--force-ocr` are mutually exclusive
|
||||
* argparse `--ocr-engine` choices exclude `kserve_v2_ocr`
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import io
|
||||
from contextlib import redirect_stderr
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from opendataloader_pdf import hybrid_server
|
||||
|
||||
|
||||
# ---------- create_converter: factory delegation ----------
|
||||
|
||||
def _capture_pipeline_options(**kwargs):
|
||||
"""Call create_converter while mocking the docling document_converter so we
|
||||
can introspect the PdfPipelineOptions instance that would be passed in.
|
||||
"""
|
||||
captured = {}
|
||||
|
||||
def fake_pdf_format_option(*, pipeline_options):
|
||||
captured["pipeline_options"] = pipeline_options
|
||||
return object()
|
||||
|
||||
with patch(
|
||||
"docling.document_converter.DocumentConverter"
|
||||
) as mock_dc, patch(
|
||||
"docling.document_converter.PdfFormatOption", side_effect=fake_pdf_format_option
|
||||
):
|
||||
mock_dc.return_value = object()
|
||||
hybrid_server.create_converter(**kwargs)
|
||||
|
||||
return captured["pipeline_options"]
|
||||
|
||||
|
||||
def test_defaults_preserve_prior_behavior():
|
||||
"""No flags -> do_ocr=True, EasyOcrOptions (matches pre-change behavior)."""
|
||||
from docling.datamodel.pipeline_options import EasyOcrOptions
|
||||
|
||||
opts = _capture_pipeline_options()
|
||||
assert opts.do_ocr is True
|
||||
assert isinstance(opts.ocr_options, EasyOcrOptions)
|
||||
|
||||
|
||||
def test_disable_ocr_sets_do_ocr_false():
|
||||
"""`disable_ocr=True` -> do_ocr=False (#387)."""
|
||||
opts = _capture_pipeline_options(disable_ocr=True)
|
||||
assert opts.do_ocr is False
|
||||
|
||||
|
||||
def test_ocr_engine_tesseract_yields_tesseract_options():
|
||||
"""`ocr_engine='tesseract'` -> TesseractCliOcrOptions (#439 path)."""
|
||||
from docling.datamodel.pipeline_options import TesseractCliOcrOptions
|
||||
|
||||
opts = _capture_pipeline_options(ocr_engine="tesseract")
|
||||
assert isinstance(opts.ocr_options, TesseractCliOcrOptions)
|
||||
|
||||
|
||||
def test_ocr_engine_rapidocr_yields_rapidocr_options():
|
||||
"""`ocr_engine='rapidocr'` -> RapidOcrOptions (#436 path)."""
|
||||
from docling.datamodel.pipeline_options import RapidOcrOptions
|
||||
|
||||
opts = _capture_pipeline_options(ocr_engine="rapidocr")
|
||||
assert isinstance(opts.ocr_options, RapidOcrOptions)
|
||||
|
||||
|
||||
def test_force_full_page_ocr_propagates_to_engine_options():
|
||||
"""`force_full_page_ocr=True` flows to the engine's options instance."""
|
||||
opts = _capture_pipeline_options(
|
||||
ocr_engine="rapidocr", force_full_page_ocr=True
|
||||
)
|
||||
assert opts.ocr_options.force_full_page_ocr is True
|
||||
|
||||
|
||||
def test_ocr_lang_overrides_engine_default():
|
||||
"""A non-empty ocr_lang list replaces the engine's default lang."""
|
||||
opts = _capture_pipeline_options(
|
||||
ocr_engine="tesseract", ocr_lang=["mal", "eng"]
|
||||
)
|
||||
assert opts.ocr_options.lang == ["mal", "eng"]
|
||||
|
||||
|
||||
def test_psm_is_applied_to_tesseract():
|
||||
"""`psm` is applied when the engine is Tesseract."""
|
||||
opts = _capture_pipeline_options(ocr_engine="tesseract", psm=6)
|
||||
assert opts.ocr_options.psm == 6
|
||||
|
||||
|
||||
def test_psm_is_ignored_for_non_tesseract_engines():
|
||||
"""`psm` is silently ignored for engines that do not expose it."""
|
||||
# EasyOcrOptions has no `psm` field; passing psm should not raise.
|
||||
opts = _capture_pipeline_options(ocr_engine="easyocr", psm=6)
|
||||
assert not hasattr(opts.ocr_options, "psm")
|
||||
|
||||
|
||||
def test_ocr_engine_auto_yields_auto_options():
|
||||
"""`ocr_engine='auto'` -> OcrAutoOptions; engine choice is deferred to docling."""
|
||||
from docling.datamodel.pipeline_options import OcrAutoOptions
|
||||
|
||||
opts = _capture_pipeline_options(ocr_engine="auto")
|
||||
assert isinstance(opts.ocr_options, OcrAutoOptions)
|
||||
|
||||
|
||||
def test_unknown_engine_raises_value_error_with_clear_message():
|
||||
"""An unknown engine kind raises ValueError listing valid engines.
|
||||
|
||||
ValueError (not SystemExit) keeps the function library-friendly: programmatic
|
||||
callers can catch and retry with a different engine. main() relies on
|
||||
argparse `choices` to gate invalid CLI input separately.
|
||||
"""
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
_capture_pipeline_options(ocr_engine="bogus-engine")
|
||||
msg = str(excinfo.value)
|
||||
assert "bogus-engine" in msg
|
||||
assert "Available engines" in msg
|
||||
|
||||
|
||||
def test_create_converter_rejects_denylisted_engine():
|
||||
"""Programmatic callers cannot bypass `_OCR_ENGINE_DENYLIST` via create_converter().
|
||||
|
||||
The CLI layer rejects denylisted engines via argparse `choices`. This test
|
||||
locks in the same enforcement at the function-call layer, so importing the
|
||||
module and passing `ocr_engine="kserve_v2_ocr"` directly fails with a clear
|
||||
ValueError instead of building a converter that fails opaquely on the first
|
||||
request.
|
||||
"""
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
_capture_pipeline_options(ocr_engine="kserve_v2_ocr")
|
||||
msg = str(excinfo.value)
|
||||
assert "kserve_v2_ocr" in msg
|
||||
assert "hybrid local mode" in msg
|
||||
assert "Available engines" in msg
|
||||
# Must also mention the denylist explicitly so a future maintainer searching
|
||||
# for the constant lands here.
|
||||
assert "_OCR_ENGINE_DENYLIST" in msg
|
||||
|
||||
|
||||
# ---------- argparse: --no-ocr / --force-ocr / --ocr-engine / --psm ----------
|
||||
|
||||
def _build_parser_subset():
|
||||
"""Reconstruct the OCR-relevant argparse subset from main().
|
||||
|
||||
Mirrors the layout in hybrid_server.main() so changes to the CLI surface
|
||||
are caught by these tests. Uses the same `_OCR_ENGINE_DENYLIST` module
|
||||
constant as production code so tests stay in sync with main() automatically.
|
||||
"""
|
||||
from docling.models.factories import get_ocr_factory
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
ocr_mode = parser.add_mutually_exclusive_group()
|
||||
ocr_mode.add_argument("--force-ocr", action="store_true")
|
||||
ocr_mode.add_argument("--no-ocr", action="store_true")
|
||||
|
||||
choices = sorted(
|
||||
set(get_ocr_factory(allow_external_plugins=False).registered_kind)
|
||||
- hybrid_server._OCR_ENGINE_DENYLIST
|
||||
)
|
||||
parser.add_argument(
|
||||
"--ocr-engine", default="easyocr", choices=choices
|
||||
)
|
||||
parser.add_argument("--psm", type=int, default=None)
|
||||
parser.add_argument("--ocr-lang", default=None)
|
||||
return parser
|
||||
|
||||
|
||||
def test_argparse_defaults():
|
||||
"""No flags -> no_ocr=False, force_ocr=False, engine=easyocr, psm=None."""
|
||||
args = _build_parser_subset().parse_args([])
|
||||
assert args.no_ocr is False
|
||||
assert args.force_ocr is False
|
||||
assert args.ocr_engine == "easyocr"
|
||||
assert args.psm is None
|
||||
|
||||
|
||||
def test_argparse_no_ocr_and_force_ocr_are_mutually_exclusive():
|
||||
"""Passing both --no-ocr and --force-ocr exits with an argparse error."""
|
||||
err = io.StringIO()
|
||||
with pytest.raises(SystemExit), redirect_stderr(err):
|
||||
_build_parser_subset().parse_args(["--no-ocr", "--force-ocr"])
|
||||
assert "not allowed with argument" in err.getvalue()
|
||||
|
||||
|
||||
def test_argparse_kserve_engine_is_rejected():
|
||||
"""`--ocr-engine kserve_v2_ocr` is rejected (denylist filter)."""
|
||||
err = io.StringIO()
|
||||
with pytest.raises(SystemExit), redirect_stderr(err):
|
||||
_build_parser_subset().parse_args(["--ocr-engine", "kserve_v2_ocr"])
|
||||
assert "invalid choice" in err.getvalue()
|
||||
|
||||
|
||||
def test_argparse_tesseract_path_for_issue_439():
|
||||
"""The CLI invocation that resolves #439 parses cleanly."""
|
||||
args = _build_parser_subset().parse_args(
|
||||
["--ocr-engine", "tesseract", "--ocr-lang", "mal", "--force-ocr"]
|
||||
)
|
||||
assert args.ocr_engine == "tesseract"
|
||||
assert args.ocr_lang == "mal"
|
||||
assert args.force_ocr is True
|
||||
assert args.no_ocr is False
|
||||
|
||||
|
||||
def test_argparse_psm_accepted_as_integer():
|
||||
"""`--psm 6` is parsed as an integer.
|
||||
|
||||
Range and semantics belong to Tesseract / docling; this server passes the
|
||||
integer through unchanged. Out-of-range values surface from docling /
|
||||
Tesseract at conversion time, not here.
|
||||
"""
|
||||
args = _build_parser_subset().parse_args(
|
||||
["--ocr-engine", "tesseract", "--psm", "6"]
|
||||
)
|
||||
assert args.psm == 6
|
||||
|
||||
|
||||
# ---------- _check_ocr_engine_available ----------
|
||||
|
||||
def test_engine_check_easyocr_always_ok():
|
||||
"""easyocr ships with the `[hybrid]` extra; check is a no-op."""
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("easyocr")
|
||||
assert ok is True
|
||||
assert msg == ""
|
||||
|
||||
|
||||
def test_engine_check_auto_always_ok():
|
||||
"""`auto` defers engine choice to docling; check is a no-op."""
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("auto")
|
||||
assert ok is True
|
||||
assert msg == ""
|
||||
|
||||
|
||||
def test_engine_check_unknown_kind_returns_false():
|
||||
"""Unknown engine kinds fail closed instead of silently passing.
|
||||
|
||||
Before this contract was tightened, the helper returned `(True, "")` for any
|
||||
engine name it did not recognize, so a docling release that registered a
|
||||
new engine kind would slip past the probe and fail at first conversion.
|
||||
Locks in the fail-closed behavior.
|
||||
"""
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("hypothetical_new_engine")
|
||||
assert ok is False
|
||||
assert "hypothetical_new_engine" in msg
|
||||
# Maintainer-targeted hint: where to add the probe branch.
|
||||
assert "_check_ocr_engine_available" in msg
|
||||
|
||||
|
||||
def test_engine_check_tesseract_missing_binary():
|
||||
"""`tesseract` engine without the binary on PATH fails with an actionable message."""
|
||||
with patch("shutil.which", return_value=None):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("tesseract")
|
||||
assert ok is False
|
||||
assert "tesseract" in msg.lower()
|
||||
assert "PATH" in msg
|
||||
|
||||
|
||||
def test_engine_check_tesseract_present():
|
||||
"""`tesseract` engine passes when the binary resolves on PATH."""
|
||||
with patch("shutil.which", return_value="/usr/bin/tesseract"):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("tesseract")
|
||||
assert ok is True
|
||||
assert msg == ""
|
||||
|
||||
|
||||
def test_engine_check_tesserocr_missing_package():
|
||||
"""`tesserocr` engine without the Python package fails with install hint."""
|
||||
with patch("importlib.util.find_spec", return_value=None):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("tesserocr")
|
||||
assert ok is False
|
||||
assert "tesserocr" in msg
|
||||
assert "pip install" in msg
|
||||
|
||||
|
||||
def test_engine_check_rapidocr_missing_package():
|
||||
"""`rapidocr` engine without the Python package fails with install hint."""
|
||||
with patch("importlib.util.find_spec", return_value=None):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("rapidocr")
|
||||
assert ok is False
|
||||
assert "rapidocr" in msg
|
||||
assert "onnxruntime" in msg
|
||||
|
||||
|
||||
def test_engine_check_rapidocr_missing_onnxruntime():
|
||||
"""`rapidocr` installed without `onnxruntime` fails with a backend-specific hint."""
|
||||
# Return a truthy spec for rapidocr, None for onnxruntime, None for anything else.
|
||||
def fake_find_spec(name):
|
||||
return object() if name == "rapidocr" else None
|
||||
|
||||
with patch("importlib.util.find_spec", side_effect=fake_find_spec):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("rapidocr")
|
||||
assert ok is False
|
||||
assert "onnxruntime" in msg
|
||||
# The message should not duplicate the "rapidocr is not installed" wording.
|
||||
assert "rapidocr` Python package" not in msg
|
||||
|
||||
|
||||
def test_engine_check_ocrmac_off_macos():
|
||||
"""`ocrmac` selected on a non-Darwin platform fails with a clear platform message."""
|
||||
with patch("sys.platform", "linux"):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("ocrmac")
|
||||
assert ok is False
|
||||
assert "macOS" in msg
|
||||
|
||||
|
||||
def test_engine_check_ocrmac_on_macos_missing_package():
|
||||
"""`ocrmac` on macOS without the `ocrmac` Python package fails with install hint."""
|
||||
with patch("sys.platform", "darwin"), patch(
|
||||
"importlib.util.find_spec", return_value=None
|
||||
):
|
||||
ok, msg = hybrid_server._check_ocr_engine_available("ocrmac")
|
||||
assert ok is False
|
||||
assert "ocrmac" in msg
|
||||
assert "pip install" in msg
|
||||
# Should NOT be the platform-mismatch message.
|
||||
assert "macOS only" not in msg and "not macOS" not in msg
|
||||
|
||||
|
||||
# ---------- --no-ocr ignored-flag warning ----------
|
||||
|
||||
def _run_main_to_warning(argv, monkeypatch, caplog):
|
||||
"""Drive `main()` far enough to capture the --no-ocr warning, then short-circuit.
|
||||
|
||||
`main()` ends in `uvicorn.run(...)`; we patch `create_app` and `uvicorn.run`
|
||||
so the function returns cleanly after argparse + the warning logic without
|
||||
starting a server.
|
||||
"""
|
||||
monkeypatch.setattr("sys.argv", ["opendataloader-pdf-hybrid", *argv])
|
||||
|
||||
# Skip dep import checks — pytest already runs inside the [hybrid] env.
|
||||
monkeypatch.setattr(hybrid_server, "_check_dependencies", lambda: None)
|
||||
|
||||
# Avoid building a real DocumentConverter / FastAPI app.
|
||||
monkeypatch.setattr(hybrid_server, "create_app", lambda **kwargs: object())
|
||||
|
||||
# Stub uvicorn so main() returns instead of starting a server.
|
||||
fake_uvicorn = type("FakeUvicorn", (), {"run": staticmethod(lambda *a, **k: None)})
|
||||
monkeypatch.setitem(__import__("sys").modules, "uvicorn", fake_uvicorn)
|
||||
|
||||
caplog.set_level("WARNING", logger=hybrid_server.logger.name)
|
||||
hybrid_server.main()
|
||||
|
||||
|
||||
def test_no_ocr_warns_when_engine_explicitly_set(monkeypatch, caplog):
|
||||
"""`--no-ocr --ocr-engine tesseract` warns that --ocr-engine has no effect."""
|
||||
_run_main_to_warning(
|
||||
["--no-ocr", "--ocr-engine", "tesseract"], monkeypatch, caplog
|
||||
)
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("--ocr-engine tesseract" in w for w in warnings), warnings
|
||||
|
||||
|
||||
def test_no_ocr_warns_when_engine_explicitly_set_to_easyocr(monkeypatch, caplog):
|
||||
"""Explicit `--ocr-engine easyocr` under --no-ocr is still inert and must warn.
|
||||
|
||||
argparse cannot distinguish "user typed easyocr" from "default was used", so
|
||||
main() inspects argv directly. This regression test locks the path in.
|
||||
"""
|
||||
_run_main_to_warning(
|
||||
["--no-ocr", "--ocr-engine", "easyocr"], monkeypatch, caplog
|
||||
)
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("--ocr-engine easyocr" in w for w in warnings), warnings
|
||||
|
||||
|
||||
def test_no_ocr_no_warning_when_engine_left_default(monkeypatch, caplog):
|
||||
"""`--no-ocr` without --ocr-engine does not falsely report easyocr as inert."""
|
||||
_run_main_to_warning(["--no-ocr"], monkeypatch, caplog)
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
# No "--ocr-engine" mention because the user did not type it.
|
||||
assert not any("--ocr-engine" in w for w in warnings), warnings
|
||||
|
||||
|
||||
def test_no_ocr_warns_when_ocr_lang_set(monkeypatch, caplog):
|
||||
"""`--no-ocr --ocr-lang ko` warns that --ocr-lang has no effect."""
|
||||
_run_main_to_warning(["--no-ocr", "--ocr-lang", "ko"], monkeypatch, caplog)
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("--ocr-lang" in w for w in warnings), warnings
|
||||
|
||||
|
||||
def test_no_ocr_warns_when_psm_set(monkeypatch, caplog):
|
||||
"""`--no-ocr --psm 6` warns that --psm has no effect."""
|
||||
_run_main_to_warning(["--no-ocr", "--psm", "6"], monkeypatch, caplog)
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("--psm 6" in w for w in warnings), warnings
|
||||
|
||||
|
||||
def test_no_ocr_alone_emits_no_warning(monkeypatch, caplog):
|
||||
"""`--no-ocr` on its own does not warn — there are no inert flags to call out."""
|
||||
_run_main_to_warning(["--no-ocr"], monkeypatch, caplog)
|
||||
warnings = [
|
||||
r.message
|
||||
for r in caplog.records
|
||||
if r.levelname == "WARNING" and "no effect" in r.message
|
||||
]
|
||||
assert warnings == []
|
||||
|
||||
|
||||
# ---------- main() exits when engine prerequisites are missing ----------
|
||||
|
||||
def test_main_exits_when_tesseract_binary_missing(monkeypatch, caplog):
|
||||
"""Selecting `--ocr-engine tesseract` without the binary on PATH exits at startup."""
|
||||
monkeypatch.setattr(
|
||||
"sys.argv", ["opendataloader-pdf-hybrid", "--ocr-engine", "tesseract"]
|
||||
)
|
||||
monkeypatch.setattr(hybrid_server, "_check_dependencies", lambda: None)
|
||||
monkeypatch.setattr(hybrid_server, "create_app", lambda **kwargs: object())
|
||||
monkeypatch.setattr("shutil.which", lambda _name: None)
|
||||
fake_uvicorn = type("FakeUvicorn", (), {"run": staticmethod(lambda *a, **k: None)})
|
||||
monkeypatch.setitem(__import__("sys").modules, "uvicorn", fake_uvicorn)
|
||||
|
||||
caplog.set_level("ERROR", logger=hybrid_server.logger.name)
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
hybrid_server.main()
|
||||
assert excinfo.value.code == 2
|
||||
errors = [r.message for r in caplog.records if r.levelname == "ERROR"]
|
||||
assert any("tesseract" in e.lower() for e in errors), errors
|
||||
|
||||
|
||||
def test_main_skips_engine_check_when_no_ocr(monkeypatch, caplog):
|
||||
"""`--no-ocr --ocr-engine tesseract` skips the binary check entirely."""
|
||||
called = {"which": False}
|
||||
|
||||
def spy_which(_name):
|
||||
called["which"] = True
|
||||
return None # Would fail the check if it were called.
|
||||
|
||||
monkeypatch.setattr(
|
||||
"sys.argv",
|
||||
["opendataloader-pdf-hybrid", "--no-ocr", "--ocr-engine", "tesseract"],
|
||||
)
|
||||
monkeypatch.setattr(hybrid_server, "_check_dependencies", lambda: None)
|
||||
monkeypatch.setattr(hybrid_server, "create_app", lambda **kwargs: object())
|
||||
monkeypatch.setattr("shutil.which", spy_which)
|
||||
fake_uvicorn = type("FakeUvicorn", (), {"run": staticmethod(lambda *a, **k: None)})
|
||||
monkeypatch.setitem(__import__("sys").modules, "uvicorn", fake_uvicorn)
|
||||
|
||||
hybrid_server.main() # must not raise SystemExit
|
||||
assert called["which"] is False
|
||||
@@ -0,0 +1,299 @@
|
||||
"""Tests for PARTIAL_SUCCESS handling in hybrid server responses.
|
||||
|
||||
Validates that when Docling encounters errors during PDF preprocessing
|
||||
(e.g., Invalid code point), the hybrid server correctly reports:
|
||||
- partial_success status instead of success
|
||||
- list of failed page numbers
|
||||
- error messages from Docling
|
||||
"""
|
||||
|
||||
from opendataloader_pdf.hybrid_server import (
|
||||
_extract_failed_pages_from_errors,
|
||||
build_conversion_response,
|
||||
)
|
||||
|
||||
|
||||
class TestBuildConversionResponse:
|
||||
"""Tests for the build_conversion_response function."""
|
||||
|
||||
def test_success_status(self):
|
||||
"""Fully successful conversion should return status=success."""
|
||||
response = build_conversion_response(
|
||||
status_value="success",
|
||||
json_content={"pages": {"1": {}, "2": {}, "3": {}}},
|
||||
processing_time=1.5,
|
||||
errors=[],
|
||||
requested_pages=None,
|
||||
)
|
||||
assert response["status"] == "success"
|
||||
assert response["failed_pages"] == []
|
||||
assert response["processing_time"] == 1.5
|
||||
|
||||
def test_partial_success_status(self):
|
||||
"""PARTIAL_SUCCESS should return status=partial_success with failed pages."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "2": {}, "4": {}, "5": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["Unknown page: pipeline terminated early"],
|
||||
requested_pages=(1, 5),
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [3]
|
||||
assert response["errors"] == ["Unknown page: pipeline terminated early"]
|
||||
|
||||
def test_partial_success_multiple_failed_pages(self):
|
||||
"""Multiple failed pages should all be reported."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "3": {}, "5": {}}},
|
||||
processing_time=3.0,
|
||||
errors=[
|
||||
"Unknown page: pipeline terminated early",
|
||||
"Unknown page: pipeline terminated early",
|
||||
],
|
||||
requested_pages=(1, 5),
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert sorted(response["failed_pages"]) == [2, 4]
|
||||
|
||||
def test_partial_success_no_page_range_with_total_pages(self):
|
||||
"""When total_pages is provided, boundary page failures are detected."""
|
||||
# 5-page document, page 1 (first) and page 5 (last) failed
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"2": {}, "3": {}, "4": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["error1", "error2"],
|
||||
requested_pages=None,
|
||||
total_pages=5,
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [1, 5]
|
||||
|
||||
def test_partial_success_no_page_range_fallback(self):
|
||||
"""When no page range or total_pages, interior gaps are still detected."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "2": {}, "4": {}, "5": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["Unknown page: pipeline terminated early"],
|
||||
requested_pages=None,
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [3]
|
||||
|
||||
def test_success_no_errors_field(self):
|
||||
"""Successful conversion should have empty errors list."""
|
||||
response = build_conversion_response(
|
||||
status_value="success",
|
||||
json_content={"pages": {"1": {}, "2": {}}},
|
||||
processing_time=1.0,
|
||||
errors=[],
|
||||
requested_pages=None,
|
||||
)
|
||||
assert response["errors"] == []
|
||||
|
||||
def test_document_field_present(self):
|
||||
"""Response should contain document.json_content."""
|
||||
json_content = {"pages": {"1": {}}, "body": {"text": "hello"}}
|
||||
response = build_conversion_response(
|
||||
status_value="success",
|
||||
json_content=json_content,
|
||||
processing_time=1.0,
|
||||
errors=[],
|
||||
requested_pages=None,
|
||||
)
|
||||
assert response["document"]["json_content"] == json_content
|
||||
|
||||
def test_partial_success_first_page_failed_with_page_range(self):
|
||||
"""First page failure should be detected when page range is specified."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"2": {}, "3": {}}},
|
||||
processing_time=1.0,
|
||||
errors=["error"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["failed_pages"] == [1]
|
||||
|
||||
def test_partial_success_last_page_failed_with_page_range(self):
|
||||
"""Last page failure should be detected when page range is specified."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "2": {}}},
|
||||
processing_time=1.0,
|
||||
errors=["error"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["failed_pages"] == [3]
|
||||
|
||||
def test_partial_success_all_pages_failed(self):
|
||||
"""All pages failing should report every page in failed_pages."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {}},
|
||||
processing_time=2.0,
|
||||
errors=["error1", "error2", "error3"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [1, 2, 3]
|
||||
|
||||
def test_partial_success_all_pages_failed_with_total_pages(self):
|
||||
"""All pages failing with total_pages should report every page."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {}},
|
||||
processing_time=2.0,
|
||||
errors=["error1", "error2"],
|
||||
requested_pages=None,
|
||||
total_pages=3,
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [1, 2, 3]
|
||||
|
||||
def test_failure_status_no_failed_pages_detection(self):
|
||||
"""Failure status should not trigger failed page detection."""
|
||||
response = build_conversion_response(
|
||||
status_value="failure",
|
||||
json_content={"pages": {"1": {}}},
|
||||
processing_time=1.0,
|
||||
errors=["PDF conversion failed"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["status"] == "failure"
|
||||
assert response["failed_pages"] == []
|
||||
|
||||
def test_partial_success_missing_pages_key(self):
|
||||
"""json_content without 'pages' key should mark all requested pages as failed."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"body": {"text": "hello"}},
|
||||
processing_time=1.0,
|
||||
errors=["error"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["status"] == "partial_success"
|
||||
assert response["failed_pages"] == [1, 2, 3]
|
||||
|
||||
|
||||
class TestExtractFailedPagesFromErrors:
|
||||
"""Tests for error message-based failed page detection."""
|
||||
|
||||
def test_std_bad_alloc_errors(self):
|
||||
"""Page numbers should be extracted from 'Page N: std::bad_alloc' messages."""
|
||||
errors = [
|
||||
"Page 26: std::bad_alloc",
|
||||
"Page 27: std::bad_alloc",
|
||||
"Page 28: std::bad_alloc",
|
||||
]
|
||||
assert _extract_failed_pages_from_errors(errors) == [26, 27, 28]
|
||||
|
||||
def test_mixed_error_formats(self):
|
||||
"""Only 'Page N:' prefixed messages should be matched."""
|
||||
errors = [
|
||||
"Page 5: Invalid code point",
|
||||
"Unknown page: pipeline terminated early",
|
||||
"Page 10: std::bad_alloc",
|
||||
]
|
||||
assert _extract_failed_pages_from_errors(errors) == [5, 10]
|
||||
|
||||
def test_no_page_errors(self):
|
||||
"""Non-page errors should return empty list."""
|
||||
errors = [
|
||||
"Unknown page: pipeline terminated early",
|
||||
"General error occurred",
|
||||
]
|
||||
assert _extract_failed_pages_from_errors(errors) == []
|
||||
|
||||
def test_bare_page_no_colon(self):
|
||||
"""Bare 'Page N' (no colon) should be matched when error_msg is empty."""
|
||||
assert _extract_failed_pages_from_errors(["Page 26"]) == [26]
|
||||
|
||||
|
||||
class TestBuildConversionResponseErrorParsing:
|
||||
"""Tests for failed page detection via error message parsing.
|
||||
|
||||
When docling includes failed pages as empty entries in the pages dict,
|
||||
gap detection fails. Error message parsing handles this case.
|
||||
"""
|
||||
|
||||
def test_failed_pages_with_empty_entries_in_pages_dict(self):
|
||||
"""Failed pages present as empty entries should still be detected via errors."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "2": {}, "3": {}, "4": {}, "5": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["Page 4: std::bad_alloc", "Page 5: std::bad_alloc"],
|
||||
requested_pages=(1, 5),
|
||||
)
|
||||
assert response["failed_pages"] == [4, 5]
|
||||
|
||||
def test_boundary_pages_detected_via_errors(self):
|
||||
"""Boundary page failures should be detected even without page range."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "2": {}, "3": {}, "4": {}, "5": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["Page 4: std::bad_alloc", "Page 5: std::bad_alloc"],
|
||||
requested_pages=None,
|
||||
total_pages=None,
|
||||
)
|
||||
assert response["failed_pages"] == [4, 5]
|
||||
|
||||
def test_both_strategies_combined(self):
|
||||
"""Union of error-parsed and gap-detected pages should be reported.
|
||||
|
||||
Page 2 is missing from dict (gap), pages 4-5 have error messages
|
||||
but are present as empty entries. All three must appear.
|
||||
"""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "3": {}, "4": {}, "5": {}}},
|
||||
processing_time=2.0,
|
||||
errors=["Page 4: std::bad_alloc", "Page 5: std::bad_alloc"],
|
||||
requested_pages=(1, 5),
|
||||
)
|
||||
assert response["failed_pages"] == [2, 4, 5]
|
||||
|
||||
def test_overlap_between_gap_and_error_is_deduplicated(self):
|
||||
"""Same page from gap and error parsing should appear once."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "3": {}}},
|
||||
processing_time=1.0,
|
||||
errors=["Page 2: std::bad_alloc"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["failed_pages"] == [2]
|
||||
|
||||
def test_duplicate_page_in_errors(self):
|
||||
"""Duplicate page numbers in error messages should be deduplicated."""
|
||||
errors = [
|
||||
"Page 3: std::bad_alloc",
|
||||
"Page 3: Invalid code point",
|
||||
]
|
||||
assert _extract_failed_pages_from_errors(errors) == [3]
|
||||
|
||||
def test_no_page_pattern_errors_falls_back_to_gap(self):
|
||||
"""When errors lack 'Page N:' pattern, gap detection should still work."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "3": {}}},
|
||||
processing_time=1.0,
|
||||
errors=["Unknown page: pipeline terminated early"],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["failed_pages"] == [2]
|
||||
|
||||
def test_empty_errors_with_partial_success(self):
|
||||
"""Partial success with no error messages should still detect gaps."""
|
||||
response = build_conversion_response(
|
||||
status_value="partial_success",
|
||||
json_content={"pages": {"1": {}, "3": {}}},
|
||||
processing_time=1.0,
|
||||
errors=[],
|
||||
requested_pages=(1, 3),
|
||||
)
|
||||
assert response["failed_pages"] == [2]
|
||||
@@ -0,0 +1,101 @@
|
||||
"""Tests for `--picture-description-prompt` propagation in the hybrid server.
|
||||
|
||||
Covers PDFDLOSP-20: the CLI accepted `--picture-description-prompt` without
|
||||
error but the prompt was never written into `PictureDescriptionVlmOptions`,
|
||||
so output was byte-identical regardless of the user's prompt. These tests
|
||||
lock in both halves of the contract:
|
||||
|
||||
* A custom prompt reaches `PictureDescriptionVlmOptions.prompt`.
|
||||
* Omitting the prompt preserves docling's built-in default.
|
||||
* Blank / whitespace-only prompts are treated as "not provided" so they
|
||||
never silently inject an empty prompt into the VLM.
|
||||
* The whole feature is gated by `enrich_picture_description=True`.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from opendataloader_pdf import hybrid_server
|
||||
|
||||
|
||||
def _capture_pipeline_options(**kwargs):
|
||||
"""Build a converter while mocking out docling's heavy bits so we can
|
||||
introspect the PdfPipelineOptions instance.
|
||||
"""
|
||||
captured = {}
|
||||
|
||||
def fake_pdf_format_option(*, pipeline_options):
|
||||
captured["pipeline_options"] = pipeline_options
|
||||
return object()
|
||||
|
||||
with patch(
|
||||
"docling.document_converter.DocumentConverter"
|
||||
) as mock_dc, patch(
|
||||
"docling.document_converter.PdfFormatOption", side_effect=fake_pdf_format_option
|
||||
):
|
||||
mock_dc.return_value = object()
|
||||
hybrid_server.create_converter(**kwargs)
|
||||
|
||||
return captured["pipeline_options"]
|
||||
|
||||
|
||||
def test_custom_prompt_is_forwarded_to_vlm_options():
|
||||
"""The user-supplied prompt must end up on PictureDescriptionVlmOptions.
|
||||
|
||||
Regression for PDFDLOSP-20.
|
||||
"""
|
||||
opts = _capture_pipeline_options(
|
||||
enrich_picture_description=True,
|
||||
picture_description_prompt="HELLO WORLD",
|
||||
)
|
||||
assert opts.picture_description_options is not None
|
||||
assert opts.picture_description_options.prompt == "HELLO WORLD"
|
||||
|
||||
|
||||
def test_default_prompt_is_preserved_when_user_omits_flag():
|
||||
"""When no prompt is given, docling's built-in default must survive.
|
||||
|
||||
Locks the current docling default. If a docling upgrade changes the
|
||||
phrase, this canary fires — at which point we decide whether to track
|
||||
the new default or pin our own.
|
||||
"""
|
||||
opts = _capture_pipeline_options(enrich_picture_description=True)
|
||||
assert opts.picture_description_options is not None
|
||||
assert (
|
||||
opts.picture_description_options.prompt
|
||||
== "Describe this image in a few sentences."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("blank", ["", " ", "\t\n"])
|
||||
def test_blank_prompt_falls_back_to_default(blank):
|
||||
"""Empty / whitespace-only prompts must not inject an empty prompt.
|
||||
|
||||
Otherwise we recreate the same class of silent-flag misbehavior that
|
||||
PDFDLOSP-20 reported.
|
||||
"""
|
||||
opts = _capture_pipeline_options(
|
||||
enrich_picture_description=True,
|
||||
picture_description_prompt=blank,
|
||||
)
|
||||
assert opts.picture_description_options is not None
|
||||
assert (
|
||||
opts.picture_description_options.prompt
|
||||
== "Describe this image in a few sentences."
|
||||
)
|
||||
|
||||
|
||||
def test_prompt_ignored_when_enrichment_disabled():
|
||||
"""Without --enrich-picture-description, the prompt has no destination.
|
||||
|
||||
Docling populates a default `picture_description_options` object on
|
||||
`PdfPipelineOptions` even when do_picture_description=False, so we
|
||||
assert on the gate (`do_picture_description`) rather than identity of
|
||||
the options object.
|
||||
"""
|
||||
opts = _capture_pipeline_options(
|
||||
enrich_picture_description=False,
|
||||
picture_description_prompt="HELLO WORLD",
|
||||
)
|
||||
assert opts.do_picture_description is False
|
||||
@@ -0,0 +1,86 @@
|
||||
"""Tests for Unicode sanitization in hybrid server responses.
|
||||
|
||||
Validates that lone surrogates and null characters from Docling OCR output
|
||||
are sanitized before JSON serialization to prevent UnicodeEncodeError in
|
||||
Starlette's JSONResponse.render().
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from opendataloader_pdf.hybrid_server import sanitize_unicode
|
||||
|
||||
|
||||
class TestSanitizeUnicode:
|
||||
"""Tests for the sanitize_unicode function."""
|
||||
|
||||
def test_lone_surrogate_replaced(self):
|
||||
"""Lone surrogates should be replaced with U+FFFD."""
|
||||
data = {"text": "Hello \ud800 World"}
|
||||
result = sanitize_unicode(data)
|
||||
assert "\ud800" not in result["text"]
|
||||
assert "\ufffd" in result["text"]
|
||||
|
||||
def test_all_surrogate_range_replaced(self):
|
||||
"""All surrogate code points (U+D800 to U+DFFF) should be replaced."""
|
||||
data = {"text": "\ud800\udbff\udc00\udfff"}
|
||||
result = sanitize_unicode(data)
|
||||
assert result["text"] == "\ufffd" * 4
|
||||
|
||||
def test_null_character_replaced(self):
|
||||
"""Null characters should be replaced with U+FFFD."""
|
||||
data = {"text": "Hello\x00World"}
|
||||
result = sanitize_unicode(data)
|
||||
assert "\x00" not in result["text"]
|
||||
assert result["text"] == "Hello\ufffdWorld"
|
||||
|
||||
def test_nested_dict_sanitized(self):
|
||||
"""Nested dictionaries should be sanitized recursively."""
|
||||
data = {"level1": {"level2": {"text": "bad\ud800char"}}}
|
||||
result = sanitize_unicode(data)
|
||||
assert "\ud800" not in result["level1"]["level2"]["text"]
|
||||
assert "\ufffd" in result["level1"]["level2"]["text"]
|
||||
|
||||
def test_list_sanitized(self):
|
||||
"""Lists within the data should be sanitized."""
|
||||
data = {"items": ["good", "bad\ud800text", "also\x00bad"]}
|
||||
result = sanitize_unicode(data)
|
||||
assert result["items"][0] == "good"
|
||||
assert "\ud800" not in result["items"][1]
|
||||
assert "\x00" not in result["items"][2]
|
||||
|
||||
def test_clean_data_unchanged(self):
|
||||
"""Clean data without problematic characters should pass through unchanged."""
|
||||
data = {"text": "Hello World", "number": 42, "flag": True, "nothing": None}
|
||||
result = sanitize_unicode(data)
|
||||
assert result == data
|
||||
|
||||
def test_non_string_values_preserved(self):
|
||||
"""Non-string values (int, float, bool, None) should be preserved as-is."""
|
||||
data = {"int": 42, "float": 3.14, "bool": True, "none": None}
|
||||
result = sanitize_unicode(data)
|
||||
assert result == data
|
||||
|
||||
def test_sanitized_output_json_serializable(self):
|
||||
"""Sanitized output must survive json.dumps + encode('utf-8') without error."""
|
||||
data = {
|
||||
"status": "success",
|
||||
"document": {
|
||||
"json_content": {
|
||||
"body": {"text": "OCR text with \ud800 lone surrogate and \x00 null"}
|
||||
}
|
||||
},
|
||||
}
|
||||
result = sanitize_unicode(data)
|
||||
# This is the exact operation that Starlette's JSONResponse.render() performs
|
||||
json_bytes = json.dumps(result, ensure_ascii=False).encode("utf-8")
|
||||
assert isinstance(json_bytes, bytes)
|
||||
|
||||
def test_mixed_valid_and_invalid_unicode(self):
|
||||
"""Valid Unicode (including CJK, emoji) should be preserved alongside sanitization."""
|
||||
data = {"text": "Valid \u4e16\u754c \ud800 text"}
|
||||
result = sanitize_unicode(data)
|
||||
assert "\u4e16\u754c" in result["text"] # CJK preserved
|
||||
assert "\ud800" not in result["text"] # surrogate removed
|
||||
assert "\ufffd" in result["text"] # replacement added
|
||||
@@ -0,0 +1,153 @@
|
||||
"""Unit tests for runner.py error-handling behaviour.
|
||||
|
||||
Regression: when the JAR fails, the streaming branch already wrote the
|
||||
JAR's stdout to the console live, so the except handler must not re-emit
|
||||
the captured copy. The quiet branch, conversely, has not surfaced anything
|
||||
yet and is allowed to print the captured streams — but only once
|
||||
(``CalledProcessError.output`` and ``.stdout`` are the same attribute).
|
||||
"""
|
||||
|
||||
import io
|
||||
import subprocess
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from opendataloader_pdf import runner
|
||||
|
||||
|
||||
class _FakeAsFile:
|
||||
def __init__(self, path):
|
||||
self._path = path
|
||||
|
||||
def __enter__(self):
|
||||
return self._path
|
||||
|
||||
def __exit__(self, *_args):
|
||||
return False
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def patched_jar(monkeypatch, tmp_path):
|
||||
"""Bypass the real resources lookup so run_jar reaches subprocess."""
|
||||
fake_jar = tmp_path / "opendataloader-pdf-cli.jar"
|
||||
fake_jar.write_bytes(b"")
|
||||
fake_traversable = MagicMock()
|
||||
fake_traversable.joinpath = lambda *_a, **_kw: fake_jar
|
||||
monkeypatch.setattr(runner.resources, "files", lambda _pkg: fake_traversable)
|
||||
monkeypatch.setattr(runner.resources, "as_file", lambda p: _FakeAsFile(p))
|
||||
|
||||
|
||||
def test_streaming_failure_does_not_duplicate_output(monkeypatch, capsys, patched_jar):
|
||||
"""Streaming mode prints JAR output live; the except handler must not
|
||||
re-emit the captured copy on stderr."""
|
||||
jar_output = "Invalid page range format: '-10'\nusage: [options] ...\n"
|
||||
|
||||
fake_process = MagicMock()
|
||||
fake_process.stdout = iter([jar_output])
|
||||
fake_process.wait.return_value = 2
|
||||
fake_process.__enter__ = lambda self: self
|
||||
fake_process.__exit__ = lambda self, *_a: False
|
||||
|
||||
monkeypatch.setattr(runner.subprocess, "Popen", lambda *_a, **_kw: fake_process)
|
||||
|
||||
with pytest.raises(subprocess.CalledProcessError):
|
||||
runner.run_jar(["--bogus"], quiet=False)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
# JAR text appears exactly once: the live streaming write.
|
||||
assert "Invalid page range format" in captured.out
|
||||
assert captured.out.count("usage: [options]") == 1
|
||||
# The except handler did NOT re-emit the captured copy on stderr.
|
||||
assert "Invalid page range format" not in captured.err
|
||||
assert "usage: [options]" not in captured.err
|
||||
# Meta info is still surfaced.
|
||||
assert "Error running opendataloader-pdf CLI." in captured.err
|
||||
assert "Return code: 2" in captured.err
|
||||
|
||||
|
||||
def test_quiet_success_relays_stdout_but_not_stderr(monkeypatch, capsys, patched_jar):
|
||||
"""Quiet mode must relay the JAR's stdout (--to-stdout payload, folder
|
||||
summary line) to the caller while still suppressing the JAR's log
|
||||
stream (stderr). Regression: --quiet + --to-stdout produced no output,
|
||||
breaking pipe consumers."""
|
||||
result = subprocess.CompletedProcess(
|
||||
args=["java", "-jar", "fake.jar"],
|
||||
returncode=0,
|
||||
stdout="extracted text payload\n",
|
||||
stderr="[INFO] java log noise\n",
|
||||
)
|
||||
monkeypatch.setattr(runner.subprocess, "run", MagicMock(return_value=result))
|
||||
|
||||
returned = runner.run_jar(["doc.pdf", "--quiet", "--to-stdout"], quiet=True)
|
||||
|
||||
captured = capsys.readouterr()
|
||||
# Payload reaches the caller's stdout exactly once.
|
||||
assert captured.out.count("extracted text payload") == 1
|
||||
# The JAR's log stream stays suppressed (that is what quiet means).
|
||||
assert "java log noise" not in captured.out
|
||||
assert "java log noise" not in captured.err
|
||||
# Return value is unchanged for library callers.
|
||||
assert returned == "extracted text payload\n"
|
||||
|
||||
|
||||
def test_quiet_relays_through_stdout_buffer_byte_path(monkeypatch, patched_jar):
|
||||
"""The production CLI writes the relayed payload through
|
||||
``sys.stdout.buffer`` (bytes), not the ``sys.stdout.write`` (str)
|
||||
fallback. ``capsys`` replaces ``sys.stdout`` with an object whose
|
||||
``.buffer`` semantics differ, so the sibling test only exercises the
|
||||
str branch. This drives the real byte-relay path and asserts the
|
||||
payload is encoded to UTF-8 and written exactly once, intact — covering
|
||||
the ``encode("utf-8", "replace")`` step that the str branch skips.
|
||||
"""
|
||||
payload = "héllo 한글 payload\n" # non-ASCII: exercises the utf-8 encode
|
||||
result = subprocess.CompletedProcess(
|
||||
args=["java", "-jar", "fake.jar"],
|
||||
returncode=0,
|
||||
stdout=payload,
|
||||
stderr="[INFO] java log noise\n",
|
||||
)
|
||||
monkeypatch.setattr(runner.subprocess, "run", MagicMock(return_value=result))
|
||||
|
||||
# Fake stdout with a real binary buffer, mirroring a genuine TextIOWrapper
|
||||
# (hasattr(sys.stdout, "buffer") is True), so run_jar takes the byte path.
|
||||
fake_stdout = MagicMock()
|
||||
fake_stdout.buffer = io.BytesIO()
|
||||
monkeypatch.setattr(runner.sys, "stdout", fake_stdout)
|
||||
|
||||
returned = runner.run_jar(["doc.pdf", "--quiet", "--to-stdout"], quiet=True)
|
||||
|
||||
written = fake_stdout.buffer.getvalue()
|
||||
# Byte path was taken: payload written as UTF-8 exactly once, intact.
|
||||
assert written == payload.encode("utf-8")
|
||||
assert written.decode("utf-8") == payload
|
||||
# The str fallback branch was NOT used.
|
||||
fake_stdout.write.assert_not_called()
|
||||
fake_stdout.buffer.flush() # buffer is flushed by run_jar; no error
|
||||
# Return value is unchanged for library callers.
|
||||
assert returned == payload
|
||||
|
||||
|
||||
def test_quiet_failure_prints_captured_streams_once(monkeypatch, capsys, patched_jar):
|
||||
"""Quiet mode captures output, so the except handler surfaces it — but
|
||||
must avoid the old bug where Output and Stdout (aliases) both printed."""
|
||||
error = subprocess.CalledProcessError(
|
||||
returncode=2,
|
||||
cmd=["java", "-jar", "fake.jar"],
|
||||
output="captured stdout text",
|
||||
stderr="captured stderr text",
|
||||
)
|
||||
monkeypatch.setattr(runner.subprocess, "run", MagicMock(side_effect=error))
|
||||
|
||||
with pytest.raises(subprocess.CalledProcessError):
|
||||
runner.run_jar(["--bogus"], quiet=True)
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert err.count("captured stdout text") == 1
|
||||
assert err.count("captured stderr text") == 1
|
||||
# The pre-fix code printed both "Output:" and "Stdout:" with the same text.
|
||||
assert "Output:" not in err
|
||||
assert "Stdout: captured stdout text" in err
|
||||
assert "Stderr: captured stderr text" in err
|
||||
assert "Error running opendataloader-pdf CLI." in err
|
||||
assert "Return code: 2" in err
|
||||
Reference in New Issue
Block a user