diff --git a/packtools/sps/formats/pdf/utils/file_utils.py b/packtools/sps/formats/pdf/utils/file_utils.py index 1e47c4813..941c5ecb4 100644 --- a/packtools/sps/formats/pdf/utils/file_utils.py +++ b/packtools/sps/formats/pdf/utils/file_utils.py @@ -9,25 +9,35 @@ class DirectoryRemovalError(Exception): ... -def convert_docx_to_pdf(docx_path, libreoffice_binary="libreoffice"): +def convert_docx_to_pdf(docx_path, libreoffice_binary=None): """ Converts a DOCX file to PDF format using LibreOffice in headless mode. The function runs a subprocess to call LibreOffice, specifying the input DOCX file and the output directory for the generated PDF file. Args: docx_path (str): The path to the DOCX file to be converted. - libreoffice_binary (str): The path to the LibreOffice binary. Defaults to "libreoffice". + libreoffice_binary (str): The path to the LibreOffice binary. If not provided, + it is autodetected on PATH (tries "libreoffice", then "soffice"). Raises: + FileNotFoundError: If no LibreOffice binary is found. RuntimeError: If the PDF file was not created successfully. Returns: str: The path to the generated PDF file. """ - - output_dir = os.path.dirname(docx_path) - os.makedirs(output_dir, exist_ok=True) + binary = libreoffice_binary or shutil.which("libreoffice") or shutil.which("soffice") + if not binary: + raise FileNotFoundError( + "LibreOffice binary ('libreoffice' or 'soffice') was not found on PATH. " + "Install LibreOffice, or pass libreoffice_binary (CLI: --libreoffice-binary) " + "with the path to the executable." + ) + + output_dir = os.path.dirname(docx_path) or "." + if output_dir != ".": + os.makedirs(output_dir, exist_ok=True) subprocess.run([ - libreoffice_binary, + binary, '--headless', '--convert-to', 'pdf', diff --git a/tests/sps/formats/pdf/utils/test_file_utils.py b/tests/sps/formats/pdf/utils/test_file_utils.py index e69de29bb..a8d86974d 100644 --- a/tests/sps/formats/pdf/utils/test_file_utils.py +++ b/tests/sps/formats/pdf/utils/test_file_utils.py @@ -0,0 +1,90 @@ +import os +import tempfile +import unittest +from unittest.mock import patch + +from packtools.sps.formats.pdf.utils.file_utils import convert_docx_to_pdf + + +class TestConvertDocxToPdfBinaryResolution(unittest.TestCase): + """ + Regression test for a bug where the CLI always passed an explicit + libreoffice_binary argument (None, when --libreoffice-binary was + omitted) that shadowed this function's own default, breaking the + subprocess call with a TypeError instead of converting the file. + """ + + def setUp(self): + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + self.docx_path = os.path.join(self.tmpdir.name, "doc.docx") + with open(self.docx_path, "wb") as f: + f.write(b"") + self.pdf_path = os.path.join(self.tmpdir.name, "doc.pdf") + + def _touch_pdf(self, *args, **kwargs): + with open(self.pdf_path, "wb") as f: + f.write(b"") + + @patch("packtools.sps.formats.pdf.utils.file_utils.subprocess.run") + def test_explicit_binary_is_respected(self, mock_run): + mock_run.side_effect = self._touch_pdf + convert_docx_to_pdf(self.docx_path, libreoffice_binary="/custom/soffice") + self.assertEqual(mock_run.call_args[0][0][0], "/custom/soffice") + + @patch("packtools.sps.formats.pdf.utils.file_utils.subprocess.run") + @patch("packtools.sps.formats.pdf.utils.file_utils.shutil.which") + def test_autodetects_libreoffice_when_binary_omitted(self, mock_which, mock_run): + mock_which.side_effect = lambda name: "/usr/bin/libreoffice" if name == "libreoffice" else None + mock_run.side_effect = self._touch_pdf + convert_docx_to_pdf(self.docx_path, libreoffice_binary=None) + self.assertEqual(mock_run.call_args[0][0][0], "/usr/bin/libreoffice") + + @patch("packtools.sps.formats.pdf.utils.file_utils.subprocess.run") + @patch("packtools.sps.formats.pdf.utils.file_utils.shutil.which") + def test_falls_back_to_soffice_when_libreoffice_missing(self, mock_which, mock_run): + mock_which.side_effect = lambda name: "/usr/bin/soffice" if name == "soffice" else None + mock_run.side_effect = self._touch_pdf + convert_docx_to_pdf(self.docx_path, libreoffice_binary=None) + self.assertEqual(mock_run.call_args[0][0][0], "/usr/bin/soffice") + + @patch("packtools.sps.formats.pdf.utils.file_utils.subprocess.run") + @patch("packtools.sps.formats.pdf.utils.file_utils.shutil.which", return_value=None) + def test_raises_clear_error_when_no_binary_found(self, mock_which, mock_run): + with self.assertRaises(FileNotFoundError): + convert_docx_to_pdf(self.docx_path, libreoffice_binary=None) + mock_run.assert_not_called() + + +class TestConvertDocxToPdfRelativeOutputPath(unittest.TestCase): + """ + Regression test for issue #773: os.path.dirname("doc.docx") is "" when + docx_path has no directory component (e.g. CLI -o doc.pdf), and + os.makedirs("") raises FileNotFoundError. + """ + + def setUp(self): + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + self._cwd = os.getcwd() + os.chdir(self.tmpdir.name) + self.addCleanup(os.chdir, self._cwd) + + self.docx_path = "doc.docx" + with open(self.docx_path, "wb") as f: + f.write(b"") + self.pdf_path = "doc.pdf" + + def _touch_pdf(self, *args, **kwargs): + with open(self.pdf_path, "wb") as f: + f.write(b"") + + @patch("packtools.sps.formats.pdf.utils.file_utils.subprocess.run") + def test_docx_path_without_directory_component_does_not_raise(self, mock_run): + mock_run.side_effect = self._touch_pdf + result = convert_docx_to_pdf(self.docx_path, libreoffice_binary="/usr/bin/soffice") + self.assertEqual(os.path.normpath(result), self.pdf_path) + + +if __name__ == "__main__": + unittest.main()