From ff3b7eca1e7530f8c1a85dfff8ecacdf6014b9c9 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Sun, 18 Oct 2026 19:53:19 -0700 Subject: [PATCH 1/5] Give each pytest process its own config dir Every test run shared ./config: the developer's real codex.toml, secret keys and, above all, the file-based Django caches that cachalot fills. A -p plugin now points CODEX_CONFIG_DIR at a per-process directory under the run's tmp root before pytest-django imports the settings, so serial runs, parallel worktrees and (next) xdist workers never share a cache. An explicit CODEX_CONFIG_DIR still wins in a serial run. With that isolation the SerializeMixin file lock on BaseTestImporter only serialized importer classes across processes, so it goes. --durations=25 --durations-min=1.0 makes slow tests visible in every run's output. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 4 ++++ tests/importer/test_basic.py | 5 +---- tests/pytest_config_dir.py | 25 +++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 tests/pytest_config_dir.py diff --git a/pyproject.toml b/pyproject.toml index c05a47cd8..ff2d0e941 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -243,10 +243,14 @@ omit = [ [tool.pytest] minversion = "9.0" addopts = [ + "-p", + "tests.pytest_config_dir", "--cov", "--cov-append", "--cov-report=html", "--cov-report=term", + "--durations=25", + "--durations-min=1.0", "--junit-xml=test-results/pytest/results.xml", "--strict-config", "--strict-markers", diff --git a/tests/importer/test_basic.py b/tests/importer/test_basic.py index 15d3b1723..acbafd697 100644 --- a/tests/importer/test_basic.py +++ b/tests/importer/test_basic.py @@ -16,7 +16,6 @@ from deepdiff import DeepDiff from django.core.cache import cache from django.test import TestCase -from django.test.testcases import SerializeMixin from loguru import logger from codex.librarian.mp_queue import LIBRARIAN_QUEUE @@ -946,9 +945,7 @@ def run_full_import(importer) -> None: importer.full_text_search() -class BaseTestImporter(SerializeMixin, TestCase, ABC): - lockfile = __file__ - +class BaseTestImporter(TestCase, ABC): @override @classmethod def setUpTestData(cls): diff --git a/tests/pytest_config_dir.py b/tests/pytest_config_dir.py new file mode 100644 index 000000000..a89374921 --- /dev/null +++ b/tests/pytest_config_dir.py @@ -0,0 +1,25 @@ +"""Give every pytest process its own CODEX_CONFIG_DIR before Django loads.""" + +import os + +from tests.tmp_dirs import tmp_dir + +# Registered with ``-p`` in ``addopts`` because pytest-django imports the +# settings in ``pytest_load_initial_conftests``, before ``conftest.py`` loads; +# ``-p`` modules import earlier, during option pre-parsing. +# +# Without this every run shares ``./config``: the developer's real config, +# and above all its file-based Django caches. Under xdist, cachalot then +# serves one worker's cached queries to the others, each of which has its +# own in-memory test database, and test-DB setup fails with ``UNIQUE +# constraint failed: django_content_type...``. ``cache.clear()`` in one +# worker would also wipe the others' entries. +# +# xdist workers inherit the controller's environment, so a worker always +# replaces the inherited value. A serial run keeps an explicit +# CODEX_CONFIG_DIR. +if "PYTEST_XDIST_WORKER" in os.environ or "CODEX_CONFIG_DIR" not in os.environ: + _config_dir = tmp_dir("config") + # Settings write the secret key files straight into it at import. + _config_dir.mkdir(exist_ok=True) + os.environ["CODEX_CONFIG_DIR"] = str(_config_dir) From 490dbe1811d50035f20e02727593aaf96372d8a2 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Sun, 18 Oct 2026 20:12:42 -0700 Subject: [PATCH 2/5] Mock only the abort event's wait in second-look tests The delete-backstop and custom-cover cleanup tests patched threading.Event.wait for every Event in the process. Thread.start() waits on an Event too, and the first multiprocessing.Queue put in a process starts the queue's feeder thread, so a stray wait() call landed on the mock whenever that test was the first in its process to put on LIBRARIAN_QUEUE. TestSecondLookBeforeDeleting then failed its assert_called_once_with(5), alone or on a fresh xdist worker, and the janitor test's assert_called() could pass without the second look ever waiting. Both now patch wait on the abort event the importer or janitor is built with, which is the wait the code under test calls. Co-Authored-By: Claude Opus 5.5 --- tests/importer/test_delete_backstop.py | 31 +++++++++++++-------- tests/test_janitor_cleanup_custom_covers.py | 14 ++++++---- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/tests/importer/test_delete_backstop.py b/tests/importer/test_delete_backstop.py index b789f646e..0ec0aaddc 100644 --- a/tests/importer/test_delete_backstop.py +++ b/tests/importer/test_delete_backstop.py @@ -23,7 +23,6 @@ import os import shutil -import threading from pathlib import Path from threading import Event, Lock from typing import Final, override @@ -68,10 +67,14 @@ class _DeleteTestBase(BaseTestImporter): @override def setUp(self) -> None: super().setUp() - # Every delete-bearing batch waits between its two probes. Patch - # the wait rather than shortening it, so the tests assert *that* - # it waited without any of them paying for it. - wait_patcher = patch.object(threading.Event, "wait") + # Every delete-bearing batch waits between its two probes, on the + # importer's abort event. Patch that event's wait rather than + # shortening it, so the tests assert *that* it waited without any + # of them paying for it. Not ``Event.wait`` for every event: + # ``Thread.start()`` waits on one too, and the first queue ``put`` + # in a process starts the queue's feeder thread. + self.abort_event = Event() + wait_patcher = patch.object(self.abort_event, "wait") self.wait_mock = wait_patcher.start() self.addCleanup(wait_patcher.stop) self.library = Library.objects.get(pk=self.task.library_id) @@ -116,17 +119,21 @@ def _make_subdir_folder(self) -> Folder: library=self.library, path=str(_SUBDIR), name=_SUBDIR.name ) + def _new_importer(self, task: ImportTask) -> ComicImporter: + """Build an importer that waits on the test's patched abort event.""" + return ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), self.abort_event) + def _delete(self, **task_kwargs) -> ComicImporter: """Run the delete phase with a fresh importer, as a scan would.""" task = ImportTask(library_id=self.library.pk, **task_kwargs) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) importer.delete() return importer def _revive(self, **task_kwargs) -> ComicImporter: """Run the revival pre-phase with a fresh importer.""" task = ImportTask(library_id=self.library.pk, **task_kwargs) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) importer.unstamp_revived() return importer @@ -198,7 +205,7 @@ class TestMassDeleteWarning(_DeleteTestBase): def _importer(self) -> tuple[ComicImporter, MagicMock]: """Build an importer whose log is captured for assertions.""" task = ImportTask(library_id=self.library.pk) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) mock_log = MagicMock() importer.log = mock_log return importer, mock_log @@ -303,7 +310,7 @@ def test_unreadable_paths_are_reported(self) -> None: task = ImportTask( library_id=self.library.pk, files_deleted=frozenset({_EXTANT}) ) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) mock_log = MagicMock() importer.log = mock_log @@ -392,7 +399,7 @@ def test_cascade_is_announced(self) -> None: task = ImportTask( library_id=self.library.pk, dirs_deleted=frozenset({str(_SUBDIR)}) ) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) mock_log = MagicMock() importer.log = mock_log @@ -418,7 +425,7 @@ def _poll_and_delete(self) -> None: # Nothing under the unreadable directory may even be proposed. assert not [path for path in task.files_deleted if str(_SUBDIR) in path] assert not [path for path in task.dirs_deleted if str(_SUBDIR) in path] - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) importer.delete() def test_bookmarks_survive_an_unreadable_publisher_directory(self) -> None: @@ -536,7 +543,7 @@ def test_revival_is_reported(self) -> None: task = ImportTask( library_id=self.library.pk, files_deleted=frozenset({_EXTANT}) ) - importer = ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()) + importer = self._new_importer(task) mock_log = MagicMock() importer.log = mock_log diff --git a/tests/test_janitor_cleanup_custom_covers.py b/tests/test_janitor_cleanup_custom_covers.py index ace1d0439..f8857c907 100644 --- a/tests/test_janitor_cleanup_custom_covers.py +++ b/tests/test_janitor_cleanup_custom_covers.py @@ -108,8 +108,11 @@ def setUp(self) -> None: path.write_bytes(b"not really a png") self.covers.append(CustomCover.objects.create(path=str(path))) # Patch the second look's wait so the tests assert *that* it - # waited without any of them paying for it. - wait_patcher = patch.object(Event, "wait") + # waited without any of them paying for it. Only the janitor's own + # abort event: ``Thread.start()`` waits on an ``Event`` too, and + # would satisfy ``assert_called()`` on its own. + self.abort_event = Event() # pyright: ignore[reportUninitializedInstanceVariable] + wait_patcher = patch.object(self.abort_event, "wait") self.wait_mock = wait_patcher.start() # pyright: ignore[reportUninitializedInstanceVariable] self.addCleanup(wait_patcher.stop) @@ -118,9 +121,10 @@ def tearDown(self) -> None: """Remove the temp cover tree.""" shutil.rmtree(TMP_DIR, ignore_errors=True) - @staticmethod - def _cleanup() -> None: - Janitor(logger, LIBRARIAN_QUEUE, Lock(), Event()).cleanup_custom_covers() + def _cleanup(self) -> None: + Janitor( + logger, LIBRARIAN_QUEUE, Lock(), self.abort_event + ).cleanup_custom_covers() def _surviving_pks(self) -> set[int]: return set(CustomCover.objects.values_list("pk", flat=True)) From c6bc6a916ca8a10f3cfe800aeadd900583c7437c Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Sun, 18 Oct 2026 20:12:43 -0700 Subject: [PATCH 3/5] Run the Python suite in parallel with pytest-xdist --numprocesses=auto --dist=loadscope spreads test classes across every core and keeps each class on one worker, so setUpTestData runs once per class. Each worker gets its own in-memory test database and, from the previous commit's plugin, its own config dir and file caches. --no-loadscope-reorder keeps pytest-django's TestCase-before- TransactionTestCase order on every worker. loadscope's default sorts scopes by size, which put BookmarkDedupeMigrationTests mid-run; its flush then stripped the migration-seeded rows (ComicboxTaggingDefaults, AgeRatingMetron) and left the comic sequence at 14 for the TestCases that followed on that worker: 13 failures. Locally on 12 cores: 239 s serial -> 75 s; -n 4 (the CI runner's vCPUs) 87 s. Same 1668 passed, 1 xfailed and the same coverage TOTAL as the serial run. -n0 runs serially. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 8 ++++++++ uv.lock | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index ff2d0e941..2ef9e821d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ test = [ "pytest-asyncio~=1.0", "pytest-cov~=7.0", "pytest-django~=4.1", + "pytest-xdist~=3.8", "rnc2rng~=2.7", ] docs = ["mkdocs-minify-plugin~=0.8", "mkdocs~=1.6"] @@ -245,6 +246,13 @@ minversion = "9.0" addopts = [ "-p", "tests.pytest_config_dir", + "--numprocesses=auto", + "--dist=loadscope", + # Keep pytest-django's order, TestCase before TransactionTestCase, on every + # worker. loadscope otherwise sorts scopes by size, and a + # TransactionTestCase's flush then strips the migration-seeded rows from + # the TestCases that follow it on that worker. + "--no-loadscope-reorder", "--cov", "--cov-append", "--cov-report=html", diff --git a/uv.lock b/uv.lock index 7b015ef49..ef58458ec 100644 --- a/uv.lock +++ b/uv.lock @@ -754,6 +754,7 @@ test = [ { name = "pytest-asyncio" }, { name = "pytest-cov" }, { name = "pytest-django" }, + { name = "pytest-xdist" }, { name = "rnc2rng" }, ] @@ -834,6 +835,7 @@ test = [ { name = "pytest-asyncio", specifier = "~=1.0" }, { name = "pytest-cov", specifier = "~=7.0" }, { name = "pytest-django", specifier = "~=4.1" }, + { name = "pytest-xdist", specifier = "~=3.8" }, { name = "rnc2rng", specifier = "~=2.7" }, ] @@ -1502,6 +1504,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/42/b0/b709cc33ccb0288f41f2d92dfd754d2ccf925cb9277a66effbc132cd004c/elementpath-5.1.4-py3-none-any.whl", hash = "sha256:5ec8d7bb44b16c98414a6a95e20cb150f3cbfc3cac65edc14773288c7a937376", size = 261881, upload-time = "2026-08-08T20:29:20.649Z" }, ] +[[package]] +name = "execnet" +version = "2.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/bf/89/780e11f9588d9e7128a3f87788354c7946a9cbb1401ad38a48c4db9a4f07/execnet-2.1.2.tar.gz", hash = "sha256:63d83bfdd9a23e35b9c6a3261412324f964c2ec8dcd8d3c6916ee9373e0befcd", size = 166622, upload-time = "2025-11-12T09:56:37.75Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ab/84/02fc1827e8cdded4aa65baef11296a9bbe595c474f0d6d758af082d849fd/execnet-2.1.2-py3-none-any.whl", hash = "sha256:67fba928dd5a544b783f6056f449e5e3931a5c378b128bc18501f7ea79e296ec", size = 40708, upload-time = "2025-11-12T09:56:36.333Z" }, +] + [[package]] name = "executing" version = "2.3.0" @@ -3386,6 +3397,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9c/03/850bffad2b581c440ca51c039d74504d5a422c94bda0bdb8a8ba5068d48b/pytest_django-4.14.0-py3-none-any.whl", hash = "sha256:c533b08d89cc675efcd5398eea270b34547e35f9a3608e2c9748dd88428ea187", size = 27067, upload-time = "2026-08-10T14:13:06.998Z" }, ] +[[package]] +name = "pytest-xdist" +version = "3.8.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "execnet" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/78/b4/439b179d1ff526791eb921115fca8e44e596a13efeda518b9d845a619450/pytest_xdist-3.8.0.tar.gz", hash = "sha256:7e578125ec9bc6050861aa93f2d59f1d8d085595d6551c2c90b6f4fad8d3a9f1", size = 88069, upload-time = "2025-07-01T13:30:59.346Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ca/31/d4e37e9e550c2b92a9cbc2e4d0b7420a27224968580b5a447f420847c975/pytest_xdist-3.8.0-py3-none-any.whl", hash = "sha256:202ca578cfeb7370784a8c33d6d05bc6e13b4f25b5053c30a152269fd10f0b88", size = 46396, upload-time = "2025-07-01T13:30:56.632Z" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From dd3a9b2990969dc4807d801f1ae56ce7fba20ea1 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Sun, 18 Oct 2026 20:12:43 -0700 Subject: [PATCH 4/5] Stop vitest linting and rewriting its snapshot vite-plugin-checker ran a full eslint_d pass inside every vitest run. The Lint job already lints the frontend, so the checker now loads only outside the test environment. reader-nav-button's only assertion on its initial markup was toMatchSnapshot() against a gitignored file that --update rewrote on every run, so it could never fail. It now asserts the link's tag, href, title, disabled state and classes. --update leaves test and test:ci; test:update regenerates snapshots on purpose. Co-Authored-By: Claude Opus 5.5 --- frontend/package.json | 5 +++-- frontend/tests/unit/reader-nav-button.test.js | 7 ++++++- frontend/vite.config.js | 14 ++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index b61c6937b..192b667df 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -12,8 +12,9 @@ "devBuild": "vite build --mode development", "fix": "eslint_d --cache --fix . && prettier --write .", "lint": "eslint_d --cache . && prettier --check .", - "test": "vitest --reporter verbose --update", - "test:ci": "vitest run --reporter verbose --update" + "test": "vitest --reporter verbose", + "test:ci": "vitest run --reporter verbose", + "test:update": "vitest run --update" }, "dependencies": { "@unhead/vue": "^3.4.3", diff --git a/frontend/tests/unit/reader-nav-button.test.js b/frontend/tests/unit/reader-nav-button.test.js index c65fe8423..9268ffe2b 100644 --- a/frontend/tests/unit/reader-nav-button.test.js +++ b/frontend/tests/unit/reader-nav-button.test.js @@ -40,7 +40,12 @@ test("reader-nav-button", async () => { const readerStore = useReaderStore(); // test initial state - expect(wrapper.html()).toMatchSnapshot(); + expect(wrapper.element.tagName).toBe("A"); + expect(wrapper.attributes("href")).toBe("/read/0?page=0"); + expect(wrapper.attributes("title")).toBe("Page 0"); + expect(wrapper.attributes("aria-disabled")).toBe("true"); + expect(wrapper.classes()).toContain("readerNavButton"); + expect(wrapper.classes()).toContain("readerNavButtonLeft"); expect(wrapper.text()).toContain("0"); // push new route diff --git a/frontend/vite.config.js b/frontend/vite.config.js index 8a5eb95fb..9743dea60 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -207,12 +207,14 @@ const config = defineConfig(({ mode }) => { plugins: [ vue(), vuetify({ autoImport: true }), - checker({ - eslint: { - lintCommand: "eslint_d --cache .", // "./src/**/*.{js,vue}"', - useFlatConfig: true, - }, - }), + // Lint runs ESLint over the frontend; vitest needn't run it again. + !IS_TEST_ENV && + checker({ + eslint: { + lintCommand: "eslint_d --cache .", // "./src/**/*.{js,vue}"', + useFlatConfig: true, + }, + }), dynamicBase({ publicPath: PUBLIC_PATH, }), From e9ec1a7471d4219a85b8c7f1652ec94931538805 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Sun, 18 Oct 2026 21:20:21 -0700 Subject: [PATCH 5/5] update devevnv --- .devenv-version | 2 +- .github/workflows/devenv-check.yml | 13 ++++++++++++- .gitignore | 1 - package.json | 4 ++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.devenv-version b/.devenv-version index 13dead7eb..d3b5ba4bf 100644 --- a/.devenv-version +++ b/.devenv-version @@ -1 +1 @@ -0.2.10 +0.2.11 diff --git a/.github/workflows/devenv-check.yml b/.github/workflows/devenv-check.yml index 42e2c0c1b..9023b988b 100644 --- a/.github/workflows/devenv-check.yml +++ b/.github/workflows/devenv-check.yml @@ -54,7 +54,7 @@ on: type: boolean default: true retention-days: - description: Days to keep python-dist and the ci-passed marker. + description: Days to keep python-dist, the ci-passed marker and coverage. type: number default: 7 outputs: @@ -334,6 +334,17 @@ jobs: name: test-results-${{ strategy.job-index }} path: "**/test-results/pytest/*.xml" + # pytest-cov's HTML report. A project whose tests write none, such as + # one with pytest addopts of its own, has nothing to upload. + - name: Upload Coverage + if: matrix.junit && !cancelled() && steps.work.outcome != 'skipped' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: coverage-${{ strategy.job-index }} + path: "**/test-results/coverage/" + retention-days: ${{ inputs.retention-days }} + if-no-files-found: ignore + - name: Publish Test Report if: matrix.junit && !cancelled() && steps.upload-tests.outcome == 'success' uses: mikepenz/action-junit-report@b7b80d7ac5b1da46d6aaff9fb57ad50d8453bcb3 # v6.6.0 diff --git a/.gitignore b/.gitignore index 96489413e..ecdb1a231 100644 --- a/.gitignore +++ b/.gitignore @@ -50,7 +50,6 @@ MANIFEST TODO.md __pycache__/ __pypackages__/ -__snapshots__ build celerybeat-schedule celerybeat.pid diff --git a/package.json b/package.json index a9edd2c5c..ea567fdbb 100644 --- a/package.json +++ b/package.json @@ -2,8 +2,8 @@ "description": "linting for top level codex project & django backend", "type": "module", "scripts": { - "fix": "eslint_d --cache --fix . && prettier --write . && bin/prettier-nginx.sh --write", - "lint": "eslint_d --cache . && prettier --check ." + "fix": "eslint_d --cache --ignore-pattern frontend/ --fix . && prettier --write . && bin/prettier-nginx.sh --write", + "lint": "eslint_d --cache --ignore-pattern frontend/ . && prettier --check ." }, "browserslist": [ "> 1%",