diff --git a/skills/visual/scripts/visual.py b/skills/visual/scripts/visual.py index eae7f02..41822dd 100644 --- a/skills/visual/scripts/visual.py +++ b/skills/visual/scripts/visual.py @@ -758,7 +758,8 @@ def chrome(binary: str, *args: str, done=None, timeout: int = 45) -> str: """Run headless Chrome and return stdout. Headless Chrome on macOS can linger after it has written its output, so poll for `done(stdout_text)` and stop it ourselves.""" import tempfile - with tempfile.TemporaryDirectory() as prof, tempfile.TemporaryFile("w+") as out: + # Chrome subprocesses can still write to the profile after the parent exits. + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as prof, tempfile.TemporaryFile("w+") as out: proc = subprocess.Popen([binary, "--headless", "--disable-gpu", "--hide-scrollbars", "--no-first-run", "--no-default-browser-check", "--mute-audio", f"--user-data-dir={prof}", *args], diff --git a/tests/test_visual_chrome_cleanup.py b/tests/test_visual_chrome_cleanup.py new file mode 100644 index 0000000..04d4ebb --- /dev/null +++ b/tests/test_visual_chrome_cleanup.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +"""Late Chrome profile writes must not discard a successful DOM result.""" +import errno +import os +import shutil +import subprocess +import sys +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "skills/visual/scripts")) +import visual + + +class ChromeCleanupTests(unittest.TestCase): + def test_profile_cleanup_race_preserves_output(self): + rmtree = shutil.rmtree + popen = subprocess.Popen + + def late_profile_write(path, *, onerror=None, onexc=None, **kwargs): + try: + raise OSError(errno.ENOTEMPTY, "Directory not empty", path) + except OSError as error: + try: + if onexc: + onexc(os.rmdir, path, error) + else: + onerror(os.rmdir, path, sys.exc_info()) + finally: + rmtree(path) + + def emit_dom(command, **kwargs): + return popen([sys.executable, "-c", "print('ready')"], **kwargs) + + with patch("tempfile._shutil.rmtree", side_effect=late_profile_write), \ + patch.object(visual.subprocess, "Popen", side_effect=emit_dom): + # A real process exits before its profile is cleaned up, just as Chrome does. + result = visual.chrome("chrome") + self.assertEqual(result.strip(), "ready") + + def test_launch_errors_are_still_reported(self): + with self.assertRaises(FileNotFoundError): + visual.chrome("/no-such-chrome-binary") + + +if __name__ == "__main__": + unittest.main()