From c0520223fda4b900a67c752794bacde246d13a83 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Sun, 8 Mar 2026 17:22:27 -0700 Subject: [PATCH] fix: clipboard BMP conversion file loss and broken test Source code (hermes_cli/clipboard.py): - _convert_to_png() lost the file when both Pillow and ImageMagick were unavailable: path.rename(tmp) moved the file to .bmp, then subprocess.run raised FileNotFoundError, but the file was never renamed back. The final fallback 'return path.exists()' returned False. - Fix: restore the original file in both except handlers by renaming tmp back to path when the original is missing. Test (tests/tools/test_clipboard.py): - test_file_still_usable_when_no_converter expected 'from PIL import Image' to raise an Exception, but Pillow is installed so pytest.raises fired 'DID NOT RAISE'. The test also never called _convert_to_png(). - Fix: properly mock PIL unavailability via patch.dict(sys.modules), actually call _convert_to_png(), and assert the correct result. --- hermes_cli/clipboard.py | 6 +++++- tests/tools/test_clipboard.py | 15 +++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/hermes_cli/clipboard.py b/hermes_cli/clipboard.py index fa750d85..6373cfc8 100644 --- a/hermes_cli/clipboard.py +++ b/hermes_cli/clipboard.py @@ -285,8 +285,8 @@ def _convert_to_png(path: Path) -> bool: logger.debug("Pillow BMP→PNG conversion failed: %s", e) # Fall back to ImageMagick convert + tmp = path.with_suffix(".bmp") try: - tmp = path.with_suffix(".bmp") path.rename(tmp) r = subprocess.run( ["convert", str(tmp), "png:" + str(path)], @@ -297,8 +297,12 @@ def _convert_to_png(path: Path) -> bool: return True except FileNotFoundError: logger.debug("ImageMagick not installed — cannot convert BMP to PNG") + if tmp.exists() and not path.exists(): + tmp.rename(path) except Exception as e: logger.debug("ImageMagick BMP→PNG conversion failed: %s", e) + if tmp.exists() and not path.exists(): + tmp.rename(path) # Can't convert — BMP is still usable as-is for most APIs return path.exists() and path.stat().st_size > 0 diff --git a/tests/tools/test_clipboard.py b/tests/tools/test_clipboard.py index 9b759741..dca3d3d2 100644 --- a/tests/tools/test_clipboard.py +++ b/tests/tools/test_clipboard.py @@ -550,14 +550,13 @@ class TestConvertToPng: """BMP file should still be reported as success if no converter available.""" dest = tmp_path / "img.png" dest.write_bytes(FAKE_BMP) # it's a BMP but named .png - # Both Pillow and ImageMagick fail - with patch("hermes_cli.clipboard.subprocess.run", side_effect=FileNotFoundError): - # Pillow import fails - with pytest.raises(Exception): - from PIL import Image # noqa — this may or may not work - # The function should still return True if file exists and has content - # (raw BMP is better than nothing) - assert dest.exists() and dest.stat().st_size > 0 + # Both Pillow and ImageMagick unavailable + with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}): + with patch("hermes_cli.clipboard.subprocess.run", side_effect=FileNotFoundError): + result = _convert_to_png(dest) + # Raw BMP is better than nothing — function should return True + assert result is True + assert dest.exists() and dest.stat().st_size > 0 # ── has_clipboard_image dispatch ─────────────────────────────────────────