From 2fe9686ffc010bc2f65a3a28867fa77c02dd73f5 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 27 Oct 2026 07:04:57 -0700 Subject: [PATCH] test(browser): empty media folders by a rename, delete them in the background (#384) With the database commits gone, throttled resets still stalled in rmtree: unlinking the last fixture's files waits for their pending writeback. Each reseed now moves the old output and input folders into /reset-trash and makes empty ones; a background thread deletes the trash, and the server's cleanup stops it and removes the rest before the temp root goes. --- tests/browser/folder_trash.py | 70 +++++++++++++++++++ tests/browser/server.py | 6 +- tests/browser/workspace_concept_fixture.py | 5 +- .../workspace_library_editor_fixture.py | 5 +- tests/browser/workspace_review_fixture.py | 4 +- tests/browser/workspace_tagging_fixture.py | 4 +- tests/browser/workspace_visual_fixture.py | 5 +- tests/browser/workspace_vocabulary_fixture.py | 5 +- tests/test_browser_folder_trash.py | 52 ++++++++++++++ 9 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 tests/browser/folder_trash.py create mode 100644 tests/test_browser_folder_trash.py diff --git a/tests/browser/folder_trash.py b/tests/browser/folder_trash.py new file mode 100644 index 00000000..e1456e05 --- /dev/null +++ b/tests/browser/folder_trash.py @@ -0,0 +1,70 @@ +"""Empty a browser-test media folder without waiting for the disk (#384). + +A reset used to ``shutil.rmtree`` the output and input folders the last +fixture wrote. Unlinking a file whose pages are still being written back +waits for that write, so on a slow CI disk a reset stalled in ``rmtree``. +``replace_folder`` renames the old folder into ``/reset-trash`` (one +metadata change) and makes a new empty one; a background thread deletes the +trash, and ``close_trash`` stops that thread and removes what is left before +the server's temp root goes. +""" + +from __future__ import annotations + +import queue +import shutil +import threading +import uuid +from pathlib import Path + +TRASH_NAME = "reset-trash" + + +class _Trash: + def __init__(self, directory: Path) -> None: + self.directory = directory + self._queue: "queue.SimpleQueue[Path | None]" = queue.SimpleQueue() + self._thread = threading.Thread( + target=self._work, name="pm-reset-trash", daemon=True + ) + self._thread.start() + + def _work(self) -> None: + while (folder := self._queue.get()) is not None: + shutil.rmtree(folder, ignore_errors=True) + + def discard(self, folder: Path) -> None: + self.directory.mkdir(exist_ok=True) + moved = self.directory / f"{folder.name}-{uuid.uuid4().hex}" + folder.rename(moved) + self._queue.put(moved) + + def close(self) -> None: + self._queue.put(None) + self._thread.join() + shutil.rmtree(self.directory, ignore_errors=True) + + +_trashes: dict[Path, _Trash] = {} +_lock = threading.Lock() + + +def replace_folder(folder: Path) -> None: + """Make ``folder`` an empty folder; its old content is deleted later.""" + folder = Path(folder) + if folder.exists(): + directory = folder.parent / TRASH_NAME + with _lock: + trash = _trashes.get(directory) + if trash is None: + trash = _trashes[directory] = _Trash(directory) + trash.discard(folder) + folder.mkdir() + + +def close_trash(parent: Path) -> None: + """Stop deleting under ``parent``'s trash and remove it (blocking).""" + with _lock: + trash = _trashes.pop(Path(parent) / TRASH_NAME, None) + if trash is not None: + trash.close() diff --git a/tests/browser/server.py b/tests/browser/server.py index 3c31b707..95f32ce2 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -19,6 +19,7 @@ from aiohttp.web_runner import GracefulExit from PIL import Image from PIL.PngImagePlugin import PngInfo +from tests.browser.folder_trash import close_trash, replace_folder from tests.browser.request_hold import register_request_holds from tests.browser import slow_reset_watch from tests.browser.fast_reset import SEED_ON_LIVE_ENV, ScratchSeeder @@ -270,8 +271,7 @@ def create_app(root: Path) -> web.Application: ): conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) for index, (directory, text, tag) in enumerate( [ ("output", "Cat portrait", "cats"), @@ -647,6 +647,8 @@ def create_app(root: Path) -> web.Application: api.close_annotation_captures() seeder.close() api.db.close_all() + # Old media folders still waiting for deletion go before the temp root. + close_trash(root) seed(api) # Open Needs review change streams end first, as on the real host. diff --git a/tests/browser/workspace_concept_fixture.py b/tests/browser/workspace_concept_fixture.py index a5dd23e8..1c86dbbd 100644 --- a/tests/browser/workspace_concept_fixture.py +++ b/tests/browser/workspace_concept_fixture.py @@ -16,7 +16,6 @@ import json import math import os from pathlib import Path -import shutil import time from types import SimpleNamespace from typing import TYPE_CHECKING @@ -34,6 +33,7 @@ from utils.execution_contexts import ( from utils.execution_history_events import make_finish, make_start from utils.execution_outputs import OutputBundle, OutputReference, snapshot_output_roots from utils.pair_execution import EXECUTION_KEY +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -300,8 +300,7 @@ def _clear(api: PromptManagerAPI, root: Path) -> None: for table in _TABLES: conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) def _record_run( diff --git a/tests/browser/workspace_library_editor_fixture.py b/tests/browser/workspace_library_editor_fixture.py index ab375c83..8c0aed9e 100644 --- a/tests/browser/workspace_library_editor_fixture.py +++ b/tests/browser/workspace_library_editor_fixture.py @@ -3,11 +3,11 @@ from __future__ import annotations from pathlib import Path -import shutil from typing import TYPE_CHECKING from aiohttp import web from PIL import Image +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -51,8 +51,7 @@ def seed_library_editor(api: PromptManagerAPI, root: Path) -> dict: for table in _TABLES: conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) epoch = db.get_execution_epoch() receipt = db.create_library_recipe( EDITOR_POSITIVE, diff --git a/tests/browser/workspace_review_fixture.py b/tests/browser/workspace_review_fixture.py index 1023d1fc..45365dd7 100644 --- a/tests/browser/workspace_review_fixture.py +++ b/tests/browser/workspace_review_fixture.py @@ -52,6 +52,7 @@ from typing import TYPE_CHECKING from aiohttp import web from PIL import Image from PIL.PngImagePlugin import PngInfo +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -225,8 +226,7 @@ def seed_review( conn.execute(f"DELETE FROM {table}") api.close_annotation_captures() for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) shutil.rmtree(root / "temp" / "review", ignore_errors=True) output = root / "output" files = [] diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index 843e2684..01667ee8 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -19,6 +19,7 @@ from typing import TYPE_CHECKING from aiohttp import web from PIL import Image from PIL.PngImagePlugin import PngInfo +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -98,8 +99,7 @@ def seed_tagging(api: PromptManagerAPI, root: Path, extra_images: int = 0) -> di for table in _TABLES: conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) output = root / "output" _png(output / FILES[0][0], (196, 140, 92)) _png(output / FILES[1][0], (150, 110, 80)) diff --git a/tests/browser/workspace_visual_fixture.py b/tests/browser/workspace_visual_fixture.py index edb8dc01..f10cf457 100644 --- a/tests/browser/workspace_visual_fixture.py +++ b/tests/browser/workspace_visual_fixture.py @@ -5,13 +5,13 @@ from __future__ import annotations import json import os from pathlib import Path -import shutil from typing import TYPE_CHECKING from PIL import Image, ImageDraw from PIL.PngImagePlugin import PngInfo from utils.hashing import generate_prompt_hash +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -129,8 +129,7 @@ def seed_workspace_visual(api: PromptManagerAPI, root: Path) -> None: ): conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) for parent_index, (positive, negatives) in enumerate(zip(_POSITIVES, _NEGATIVES)): category = ("Architecture", "Landscape", "Abstract")[parent_index % 3] tags = ["geometric", ("warm", "cool")[parent_index % 2]] diff --git a/tests/browser/workspace_vocabulary_fixture.py b/tests/browser/workspace_vocabulary_fixture.py index 10c5a8c7..17901872 100644 --- a/tests/browser/workspace_vocabulary_fixture.py +++ b/tests/browser/workspace_vocabulary_fixture.py @@ -7,10 +7,10 @@ in concept order, so tag creation order and category first use match the mock. from __future__ import annotations from pathlib import Path -import shutil from typing import TYPE_CHECKING from aiohttp import web +from tests.browser.folder_trash import replace_folder if TYPE_CHECKING: from py.api import PromptManagerAPI @@ -120,8 +120,7 @@ def seed_vocabulary( for table in _TABLES: conn.execute(f"DELETE FROM {table}") for name in ("output", "input"): - shutil.rmtree(root / name) - (root / name).mkdir() + replace_folder(root / name) epoch = db.get_execution_epoch() recipes = [] for recipe in CONCEPT_RECIPES: diff --git a/tests/test_browser_folder_trash.py b/tests/test_browser_folder_trash.py new file mode 100644 index 00000000..5550a635 --- /dev/null +++ b/tests/test_browser_folder_trash.py @@ -0,0 +1,52 @@ +"""Browser-test media folders are emptied by a rename, deleted later (#384).""" + +import tempfile +import threading +import unittest +from pathlib import Path + +from tests.browser.folder_trash import TRASH_NAME, close_trash, replace_folder + + +class TestFolderTrash(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + self.addCleanup(close_trash, self.root) + + def trash_threads(self): + return [t for t in threading.enumerate() if t.name == "pm-reset-trash"] + + def test_replaces_the_folder_with_an_empty_one(self): + folder = self.root / "output" + (folder / "nested").mkdir(parents=True) + (folder / "nested" / "image.png").write_bytes(b"png") + replace_folder(folder) + self.assertTrue(folder.is_dir()) + self.assertEqual(list(folder.iterdir()), []) + close_trash(self.root) + self.assertFalse((self.root / TRASH_NAME).exists()) + self.assertEqual(self.trash_threads(), []) + + def test_a_missing_folder_is_created_without_a_trash(self): + replace_folder(self.root / "input") + self.assertTrue((self.root / "input").is_dir()) + self.assertFalse((self.root / TRASH_NAME).exists()) + self.assertEqual(self.trash_threads(), []) + + def test_repeated_replacements_share_one_thread_and_all_are_removed(self): + folder = self.root / "output" + for number in range(5): + folder.mkdir(exist_ok=True) + (folder / f"{number}.png").write_bytes(b"png") + replace_folder(folder) + self.assertEqual(len(self.trash_threads()), 1) + close_trash(self.root) + self.assertEqual(list(folder.iterdir()), []) + self.assertFalse((self.root / TRASH_NAME).exists()) + close_trash(self.root) # idempotent + + +if __name__ == "__main__": + unittest.main()