diff --git a/CLAUDE.md b/CLAUDE.md index aadb8405c..b455682da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,7 +98,7 @@ Meaningful codes when reading `make doctor` output: `SM001` missing meta (error) ## Tests & fixtures -Root `conftest.py` provides app-level fixtures available to every test directory: +The `simple_module_test` plugin provides app-level fixtures available to every test directory — auto-loaded via its `pytest11` entry point (defined in `framework/testing/simple_module_test/fixtures.py`), so the root `conftest.py` is intentionally thin: - `settings` — in-memory SQLite `Settings` with `multi_tenant=True`. - `db_state`, `engine`, `db_session` — fresh in-memory `DatabaseState` per test; `db_session` also creates all module tables and stamps `alembic_version` at head so the boot-time migration check passes. - `app` — `create_app(settings)` with lifespan started/stopped. diff --git a/README.md b/README.md index 5386a1c85..412851567 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,6 @@ Historical, point-in-time design docs live under [`docs/plans/`](docs/plans/) an ## Contributing -- Write tests with the fixtures in `conftest.py` (`db_session`, `authenticated_client`). +- Write tests with the fixtures from the `simple_module_test` plugin (`db_session`, `authenticated_client`). - Lint with `make lint` before pushing; CI runs all four checks in parallel. - Stick to the conventions in `docs/framework-conventions.md` — they're what diagnostics enforce. diff --git a/conftest.py b/conftest.py index e95e67dca..e4ecfb533 100644 --- a/conftest.py +++ b/conftest.py @@ -1,175 +1,15 @@ -"""Root conftest — shared fixtures available to all test directories.""" +"""Root conftest — intentionally thin. -from __future__ import annotations - -import contextlib -import importlib -from collections.abc import AsyncGenerator -from functools import lru_cache - -import httpx -import pytest -from simple_module_core.discovery import discover_modules -from simple_module_db.base import all_module_bases -from simple_module_db.session import DatabaseState, init_db -from simple_module_hosting.settings import Settings -from simple_module_test import forge_session_cookie -from sqlalchemy.ext.asyncio import ( - AsyncEngine, - AsyncSession, -) - - -@pytest.fixture -def settings() -> Settings: - """Settings configured for testing with in-memory SQLite. - - Multi-tenancy stays on so the existing ``TenantMiddleware`` tests - (and the ``X-Tenant-ID`` header paths they rely on) keep working. - Individual tests that want the tenant middleware absent construct - their own ``Settings(multi_tenant=False, ...)`` in the test body. - """ - return Settings( - database_url="sqlite+aiosqlite:///:memory:", - environment="testing", - secret_key="test-secret-key", - multi_tenant=True, - tenant_header="X-Tenant-ID", - ) - - -@pytest.fixture -async def db_state() -> AsyncGenerator[DatabaseState, None]: - """Create a fresh in-memory DatabaseState with listeners registered.""" - from simple_module_db.listeners import register_listeners - - state = init_db("sqlite+aiosqlite:///:memory:") - register_listeners(state) - yield state - await state.engine.dispose() - - -@pytest.fixture -async def engine(db_state: DatabaseState) -> AsyncEngine: - """Return the engine from the test DatabaseState.""" - return db_state.engine - - -@lru_cache(maxsize=1) -def _ensure_models_imported() -> list: - """Import all module models so all_module_bases is populated (cached).""" - for mod in discover_modules(): - pkg = type(mod).__module__.split(".")[0] - with contextlib.suppress(ModuleNotFoundError): - importlib.import_module(f"{pkg}.models") - return list(all_module_bases) - - -@lru_cache(maxsize=1) -def _alembic_head() -> str | None: - """Cached head revision — cannot change within a pytest run.""" - from simple_module_hosting.migrations import resolve_head_revision - - return resolve_head_revision() - - -async def _create_all_tables(engine) -> None: - """Create all module tables in a single connection. - - Also stamps the alembic_version table at head so the app's startup - migration check (``check_migrations``) treats the test DB as current. - Without the stamp the check would raise because ``create_all`` doesn't - touch alembic_version. - """ - from sqlalchemy import text - - bases = _ensure_models_imported() - head = _alembic_head() +The shared app/db/client fixtures (``settings``, ``db_state``, ``engine``, +``db_session``, ``app``, ``client``, ``authenticated_client``) live in the +``simple_module_test`` package and are auto-registered via its ``pytest11`` +entry point — installing the package is enough, no conftest import needed. They +used to be duplicated here; they were moved so the *published* plugin actually +ships what its README advertises (GH #200), and this repo now dogfoods that +plugin like any consumer would. - async with engine.begin() as conn: +Add genuinely repo-local fixtures here if the need arises; shared ones belong in +``framework/testing/simple_module_test/fixtures.py``. +""" - def _sync_create_all(sync_conn): - for base in bases: - base.metadata.create_all(sync_conn) - - await conn.run_sync(_sync_create_all) - - if head: - await conn.execute( - text( - "CREATE TABLE IF NOT EXISTS alembic_version " - "(version_num VARCHAR(32) NOT NULL PRIMARY KEY)" - ) - ) - await conn.execute(text("DELETE FROM alembic_version")) - await conn.execute( - text("INSERT INTO alembic_version (version_num) VALUES (:v)"), - {"v": head}, - ) - - -@pytest.fixture -async def db_session(db_state: DatabaseState) -> AsyncGenerator[AsyncSession, None]: - """Yield an async session backed by in-memory SQLite.""" - await _create_all_tables(db_state.engine) - - async with db_state.session_factory() as session: - yield session - - -@pytest.fixture -async def app(settings: Settings): - """Create a FastAPI app with tables pre-created and lifespan triggered.""" - from simple_module_hosting.app_builder import create_app - - application = create_app(settings) - - await _create_all_tables(application.state.sm.db.engine) - - # Trigger lifespan startup so app.state.migration is populated - ctx = application.router.lifespan_context(application) - await ctx.__aenter__() - - yield application - - # Lifespan shutdown disposes the engine - await ctx.__aexit__(None, None, None) - - -@pytest.fixture -async def client(app) -> AsyncGenerator[httpx.AsyncClient, None]: - """Unauthenticated async HTTP client.""" - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient( - transport=transport, - base_url="http://testserver", - ) as c: - yield c - - -@pytest.fixture -async def authenticated_client(app) -> AsyncGenerator[httpx.AsyncClient, None]: - """HTTPX client with a signed session cookie carrying a seeded admin user's id.""" - from users.bootstrap import create_admin - - async with app.state.sm.db.session_factory() as session: - result = await create_admin( - session, - email="admin@test", - password="test-password", - full_name="Test Admin", - ) - user_id = str(result.user.id) - - signed = forge_session_cookie( - app.state.sm.settings.secret_key, - {"user_id": user_id}, - ) - - transport = httpx.ASGITransport(app=app) - async with httpx.AsyncClient( - transport=transport, - base_url="http://testserver", - cookies={"session": signed}, - ) as c: - yield c +from __future__ import annotations diff --git a/docs/database/sessions.md b/docs/database/sessions.md index ca8941c42..9d261a7ad 100644 --- a/docs/database/sessions.md +++ b/docs/database/sessions.md @@ -110,7 +110,7 @@ Cross-tenant admin work runs outside a tenant-scoped request (a CLI or worker wh ## Sessions in tests -The `db_session` fixture in `conftest.py` creates a fresh in-memory SQLite DB, creates every module's tables, stamps `alembic_version` at head, and yields an `AsyncSession`. Each test gets a fresh one — no shared state, no transaction rollback hacks. +The `db_session` fixture from the `simple_module_test` plugin creates a fresh in-memory SQLite DB, creates every module's tables, stamps `alembic_version` at head, and yields an `AsyncSession`. Each test gets a fresh one — no shared state, no transaction rollback hacks. ```python @pytest.mark.asyncio diff --git a/docs/framework/events.md b/docs/framework/events.md index f8c49de01..1bf5c0bf9 100644 --- a/docs/framework/events.md +++ b/docs/framework/events.md @@ -146,7 +146,7 @@ async def test_place_order_publishes_event(db_session, app): assert received[0].customer_email == "a@b.c" ``` -The `app` fixture from `conftest.py` provides a fresh app with a fresh `EventBus` per test. +The `app` fixture from the `simple_module_test` plugin provides a fresh app with a fresh `EventBus` per test. ## Design guidelines diff --git a/docs/framework/permissions.md b/docs/framework/permissions.md index eb2fced29..c04e2b8c5 100644 --- a/docs/framework/permissions.md +++ b/docs/framework/permissions.md @@ -123,7 +123,7 @@ export default function OrdersToolbar() { ## Testing with permissions -The `authenticated_client` fixture in `conftest.py` seeds an admin user (who has `*`). For tests that need a less-privileged user, build one from the `users` module fixtures or flip the principal temporarily: +The `authenticated_client` fixture from the `simple_module_test` plugin seeds an admin user (who has `*`). For tests that need a less-privileged user, build one from the `users` module fixtures or flip the principal temporarily: ```python @pytest.mark.asyncio diff --git a/docs/guide/first-module.md b/docs/guide/first-module.md index 990a0d9a9..e5abb49b3 100644 --- a/docs/guide/first-module.md +++ b/docs/guide/first-module.md @@ -287,7 +287,7 @@ async def test_create_and_list_orders(authenticated_client): assert any(o["id"] == created["id"] for o in r.json()) ``` -The `authenticated_client` fixture from `conftest.py` seeds an admin user and carries a signed session cookie. The `db_session` fixture creates all module tables and stamps the Alembic head so the boot-time migration check passes. See [Fixtures](/testing/fixtures). +The `authenticated_client` fixture from the `simple_module_test` plugin seeds an admin user and carries a signed session cookie. The `db_session` fixture creates all module tables and stamps the Alembic head so the boot-time migration check passes. See [Fixtures](/testing/fixtures). Run: diff --git a/docs/index.md b/docs/index.md index 1a09a3c47..164799f26 100644 --- a/docs/index.md +++ b/docs/index.md @@ -54,7 +54,7 @@ features:

Framework

Discovery, lifecycle hooks, middleware, permissions, events, i18n.

Database

SQLModel conventions, per-module Base, mixins, sessions, Alembic.

Frontend

Inertia page keys, shared props, page discovery, React layout.

 -

Testing

The fixtures in conftest.py, unit tests, end-to-end tests.

 +

Testing

The simple_module_test plugin fixtures, unit tests, end-to-end tests.

Modules

Reference for each bundled module: routes, contracts, settings.

Reference

CLI commands, env vars, diagnostic codes, deployment.

 diff --git a/docs/module-authoring.md b/docs/module-authoring.md index 59575de45..f28fc8929 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -277,6 +277,11 @@ The package registers pytest fixtures via a `pytest11` entry_point — no |---|---| | `build_test_app` | Callable `(ModuleCls) -> FastAPI` — wraps a single module in a minimal FastAPI app with its routes registered. | | `fake_event_bus` | A `FakeEventBus` that records every `publish`/`publish_nowait` call so tests can assert emitted events. | +| `settings` | In-memory-SQLite `Settings` (`multi_tenant=True`) for the test app. | +| `db_state` / `engine` / `db_session` | Fresh in-memory `DatabaseState` per test; `db_session` creates every installed module's tables and stamps `alembic_version` at head. | +| `app` | A full `create_app(settings)` with lifespan started/stopped. | +| `client` | `httpx.AsyncClient` bound to the test app (anonymous). | +| `authenticated_client` | Same, with a seeded admin + signed session cookie. Requires the `users` module installed (seeds via `users.bootstrap`). | Example test: diff --git a/docs/modules/background_tasks.md b/docs/modules/background_tasks.md index 49dab57dd..01cbfb898 100644 --- a/docs/modules/background_tasks.md +++ b/docs/modules/background_tasks.md @@ -32,6 +32,32 @@ send_receipt.delay(order_id=42) Every dispatch / start / completion / failure / retry / revoke is recorded in the `background_tasks_task_execution` table by Celery signal handlers, so you can see history even after the Celery result backend has expired the result. +## Scheduling periodic work (beat) + +A module schedules recurring work the same way it registers tasks — by shipping a `tasks.py` — and additionally exporting a module-level `BEAT_SCHEDULE` dict. `build_celery` merges every installed module's `BEAT_SCHEDULE` into the beat schedule at boot (identically in the web and worker processes), so `make beat` runs them alongside the built-ins: + +```python +# modules/invoices/invoices/tasks.py +from celery import shared_task +from celery.schedules import crontab + +@shared_task(name="invoices.generate_recurring") +def generate_recurring() -> int: + ... + +# Discovered by build_celery and merged into the beat schedule. +BEAT_SCHEDULE = { + "invoices-generate-recurring-daily": { + "task": "invoices.generate_recurring", + "schedule": crontab(hour=6, minute=0), # 0 6 * * * + }, +} +``` + +Entry values are plain Celery [beat entries](https://docs.celeryq.dev/en/stable/userguide/periodic-tasks.html#entries): `schedule` accepts a number of seconds, a `crontab(...)`, or a `solar(...)`. The two built-in entry names (`background-tasks-sweep-stuck`, `background-tasks-purge-old`) are authoritative — a module reusing one is ignored with a warning. + +> **Don't reach for `from celery.signals import on_after_configure`.** It's an *app-instance* signal (not a member of `celery.signals`, so the import raises), and `build_celery` runs `conf.update(...)` — which fires it — *before* `autodiscover_tasks` imports your `tasks.py`, so a handler would miss the window. The declarative `BEAT_SCHEDULE` dict above is the supported mechanism. If you need entries computed at runtime, Celery's `@app.on_after_finalize.connect` + `sender.add_periodic_task(...)` also works. + ## Running workers | Command | Use case | @@ -145,6 +171,8 @@ Bootstrap env-var equivalents (`SM_BG_TASKS_*`) only seed pydantic defaults at f A `background_tasks.demo_echo` task is also registered for smoke tests; not scheduled. +Other modules contribute their own periodic entries via `tasks.BEAT_SCHEDULE` — see [Scheduling periodic work (beat)](#scheduling-periodic-work-beat). + ## Events Published (and consumable from any other module): diff --git a/docs/modules/dashboard.md b/docs/modules/dashboard.md index 52dce4cfc..3fe04b25c 100644 --- a/docs/modules/dashboard.md +++ b/docs/modules/dashboard.md @@ -67,6 +67,18 @@ Top-level keys in `dashboard/locales/en.json`: - `home.system_info_title`, `home.system_info.python_version`, `home.system_info.health_checks`, `home.system_info.modules` - `home.welcome_card_title`, `home.description_body` +## Extending it + +The bundled dashboard renders a **fixed** set of cards — `dashboard/stats.py` returns a hardcoded shape and the module exposes only `register_routes` + `register_menu_items`. This is intentional: there is **no** `register_dashboard_cards` hook or card/widget registry, and the module deliberately keeps no extension surface so it stays a small, predictable landing template rather than a framework subsystem to maintain. + +To show app-specific tiles, **build your own dashboard page in your module** rather than contributing to this one: + +- Add an Inertia view route + page (`register_routes` → `pages/Home.tsx`) and a sidebar entry (`register_menu_items`), exactly like any other module page. +- Make it the post-login landing page by pointing `users.login_redirect_url` at your route (see [Replacing it](#replacing-it)). You can drop the bundled dashboard entirely via `SM_MODULES_ENABLED` minus `dashboard`. +- Your page can still reuse this module's data — call `GET /api/dashboard/stats` for the system overview alongside your own module's endpoints. + +> Resolved as **by design** (GH #203): consumer modules build their own dashboard page; the bundled one is not a contribution point. + ## Replacing it If you want a different post-login landing page, set `users.login_redirect_url` in the [admin settings UI](/modules/settings) (or via `smpy settings import-from-env` from `SM_USERS_LOGIN_REDIRECT_URL`) to your route. You can keep the dashboard module installed for the menu entry, or set `SM_MODULES_ENABLED` without `dashboard` to drop it entirely. The `users` module auto-detects whether `dashboard` is installed; if not, it redirects to the first other module that exposes view routes (e.g. the GIS module on apps like smpy_gis), falling back to `/` only as a last resort. diff --git a/docs/testing/fixtures.md b/docs/testing/fixtures.md index d939a1f16..dfa274c52 100644 --- a/docs/testing/fixtures.md +++ b/docs/testing/fixtures.md @@ -1,6 +1,6 @@ # Fixtures -The root `conftest.py` provides app-level fixtures that every test directory inherits. Everything you need for a typical integration test — app, DB session, HTTP client, authenticated client — is already wired up. +The `simple_module_test` plugin provides app-level fixtures that every test directory inherits — auto-loaded via its `pytest11` entry point, so installing the package is enough (no `conftest.py` import needed). Everything you need for a typical integration test — app, DB session, HTTP client, authenticated client — is already wired up. ## Available fixtures @@ -90,6 +90,8 @@ Same as `client`, but the fixture also: 2. Forges a signed session cookie for that user. 3. Attaches the cookie to the client. +> Because it seeds via `users.bootstrap`, this fixture requires the `users` module to be installed (the import is deferred to the fixture body, so the rest of the plugin loads without it). Apps scaffolded by `smpy` include `users`. + ```python @pytest.mark.asyncio async def test_create_order_as_admin(authenticated_client): diff --git a/docs/testing/overview.md b/docs/testing/overview.md index 9e3f81e5a..6c6506861 100644 --- a/docs/testing/overview.md +++ b/docs/testing/overview.md @@ -29,7 +29,8 @@ The root `pyproject.toml` sets `asyncio_mode = "auto"`. Async tests don't need ` ## File layout ```text -conftest.py # root fixtures: app, db_session, client, authenticated_client +conftest.py # intentionally thin — shared fixtures ship in the simple_module_test plugin +framework/testing/ # simple_module_test plugin: app, db_session, client, authenticated_client fixtures framework//tests/ # tests against each framework package (core, db, hosting, cli, testing) host/tests/ # host-level tests modules//tests/ # per-module pytest tests @@ -93,5 +94,5 @@ Before marking a test flaky, check: ## Next steps -- [Fixtures](/testing/fixtures) — the shared fixtures in `conftest.py` and how to extend them. +- [Fixtures](/testing/fixtures) — the shared fixtures from the `simple_module_test` plugin and how to extend them. - [E2E tests](/e2e-testing) — the Playwright suite. diff --git a/framework/cli/simple_module_cli/cli.py b/framework/cli/simple_module_cli/cli.py index 8b6674063..cec0998cc 100644 --- a/framework/cli/simple_module_cli/cli.py +++ b/framework/cli/simple_module_cli/cli.py @@ -56,7 +56,17 @@ def create_host( target = dest or Path.cwd() / name selected = [m.strip() for m in modules.split(",") if m.strip()] try: - _create_host(target, name=name, modules=selected) + # Pin the host's framework + module deps to the installed framework + # version so the generated host's first `uv sync` resolves (the + # template's >=1.0,<2.0 / >=0.1,<1.0 ranges don't exist pre-1.0). The + # workspace `smpy new` path rewrites these via _rewrite_pyproject, but + # standalone create-host never did — see GH #206. + _create_host( + target, + name=name, + modules=selected, + framework_version=resolve_framework_version(), + ) except FileExistsError as exc: typer.echo(f"ERROR: {exc}", err=True) raise typer.Exit(code=1) from exc diff --git a/framework/cli/simple_module_cli/pins.py b/framework/cli/simple_module_cli/pins.py new file mode 100644 index 000000000..f711b9a72 --- /dev/null +++ b/framework/cli/simple_module_cli/pins.py @@ -0,0 +1,66 @@ +"""Resolve and pin the framework version that scaffolded apps depend on. + +Kept separate from :mod:`simple_module_cli.scaffolding` (template +materialization) because version resolution + dependency pinning is its own +concern: the scaffolders, ``cli`` commands, and ``app_project`` all reach for +it independently. The module templates ship forward-looking ``simple_module_*`` +ranges (``>=1.0,<2.0`` / ``>=0.1,<1.0``) against the framework's eventual stable +line, but the published distributions are pre-1.0 (``0.0.x``), so those ranges +resolve to nothing on PyPI. Rewriting them to an exact ``==`` pin lets a freshly +scaffolded app/module resolve against the framework version that created it. +See GH #195 / #206. +""" + +from __future__ import annotations + +from pathlib import Path + + +def resolve_framework_version() -> str: + """Resolve the framework version that scaffolded apps should pin against. + + The CLI ships in lockstep with the rest of the framework (one + ``bump_version.py`` rewrites every ``pyproject.toml``), so its own + installed distribution version is the source of truth. Falls back to a + placeholder for editable installs lacking dist-info — never reached from a + release wheel. + """ + from importlib.metadata import PackageNotFoundError + from importlib.metadata import version as pkg_version + + try: + return pkg_version("simple_module_cli") + except PackageNotFoundError: + return "0.0.0" + + +def _pin_one(dep: str, version: str) -> str: + """Pin a single ``simple_module_*`` requirement to ``==version``; else pass through.""" + pkg = dep.split(">=", 1)[0].split("==", 1)[0].split("<", 1)[0].strip() + if pkg.startswith(("simple_module_", "simple-module-")): + return f"{pkg}=={version}" + return dep + + +def pin_framework_deps(pyproject_path: Path, version: str) -> None: + """Pin every ``simple_module_*`` requirement in a pyproject to ``==version``. + + Rewrites both ``dependencies`` and every ``optional-dependencies`` extra + (the module template's ``dev`` extra pins ``simple_module_test``). Used by + ``create_module`` and ``create_host`` so a freshly scaffolded package + resolves against the framework version that created it. See GH #195 / #206. + """ + import tomlkit + + doc = tomlkit.parse(pyproject_path.read_text(encoding="utf-8")) + project = doc.get("project") + if project is None: + return + deps = project.get("dependencies") + if deps is not None: + project["dependencies"] = [_pin_one(dep, version) for dep in deps] + optional = project.get("optional-dependencies") + if optional is not None: + for extra, items in list(optional.items()): + optional[extra] = [_pin_one(dep, version) for dep in items] + pyproject_path.write_text(tomlkit.dumps(doc), encoding="utf-8") diff --git a/framework/cli/simple_module_cli/scaffolding.py b/framework/cli/simple_module_cli/scaffolding.py index 22652829c..fad870307 100644 --- a/framework/cli/simple_module_cli/scaffolding.py +++ b/framework/cli/simple_module_cli/scaffolding.py @@ -28,6 +28,10 @@ validate_scaffold_name, ) +# Re-exported so the scaffolders and their long-standing callers keep importing +# version pinning from one place; the implementations live in pins.py. +from simple_module_cli.pins import pin_framework_deps, resolve_framework_version + __all__ = [ "SAFE_PRESERVED_NAMES", "create_host", @@ -57,57 +61,15 @@ def _module_to_pypi_name(name: str) -> str: return f"simple_module_{name.lower()}" -def resolve_framework_version() -> str: - """Resolve the framework version that scaffolded apps should pin against. - - The CLI ships in lockstep with the rest of the framework (one - ``bump_version.py`` rewrites every ``pyproject.toml``), so its own - installed distribution version is the source of truth. Falls back to a - placeholder for editable installs lacking dist-info — never reached from a - release wheel. - """ - from importlib.metadata import PackageNotFoundError - from importlib.metadata import version as pkg_version - - try: - return pkg_version("simple_module_cli") - except PackageNotFoundError: - return "0.0.0" - - -def _pin_one(dep: str, version: str) -> str: - """Pin a single ``simple_module_*`` requirement to ``==version``; else pass through.""" - pkg = dep.split(">=", 1)[0].split("==", 1)[0].split("<", 1)[0].strip() - if pkg.startswith(("simple_module_", "simple-module-")): - return f"{pkg}=={version}" - return dep - - -def pin_framework_deps(pyproject_path: Path, version: str) -> None: - """Pin every ``simple_module_*`` requirement in a pyproject to ``==version``. +def _should_pin_framework_version(version: str | None) -> bool: + """Whether ``version`` is a concrete pin rather than a skip sentinel. - The module template ships forward-looking ranges (``>=1.0,<2.0``) against - the framework's eventual stable line, but the published distributions are - pre-1.0 (``0.0.x``), so those ranges resolve to nothing on PyPI. Rewriting - to an exact pin lets a freshly created module resolve against the framework - version that created it — e.g. ``uv add ./modules/`` into the same - workspace. Both ``dependencies`` and every ``optional-dependencies`` extra - (the ``dev`` extra pins ``simple_module_test``) are rewritten. See GH #195. + Both scaffolders skip pinning for ``None`` (a library caller that wants the + template's ranges kept verbatim) and ``"*"`` (the npm-wildcard default, + which would otherwise render the invalid PEP 508 specifier ``==*``). One + rule, so the two paths can't drift. See GH #195 / #206. """ - import tomlkit - - doc = tomlkit.parse(pyproject_path.read_text(encoding="utf-8")) - project = doc.get("project") - if project is None: - return - deps = project.get("dependencies") - if deps is not None: - project["dependencies"] = [_pin_one(dep, version) for dep in deps] - optional = project.get("optional-dependencies") - if optional is not None: - for extra, items in list(optional.items()): - optional[extra] = [_pin_one(dep, version) for dep in items] - pyproject_path.write_text(tomlkit.dumps(doc), encoding="utf-8") + return bool(version) and version != "*" def _iter_template_files(template_root: Path): @@ -242,6 +204,12 @@ def create_host( }, preserve_existing=preserve_existing, ) + # Pin every simple_module_* host dep (framework packages *and* selected + # bundled modules) to the lockstep version so the host's first `uv sync` + # resolves — the template's >=1.0,<2.0 / >=0.1,<1.0 ranges match nothing + # against pre-1.0 dists. See GH #206. + if _should_pin_framework_version(framework_version): + pin_framework_deps(dest / "pyproject.toml", framework_version) logger.info( "Scaffolded host '%s' at %s (modules: %s)", name, dest, ", ".join(modules) or "" ) @@ -257,11 +225,11 @@ def create_module( ) -> Path: """Scaffold a module package at ``dest``. - When ``framework_version`` is given, the template's forward-looking - ``simple_module_*`` ranges are rewritten to an exact pin so the module - resolves against that framework version (e.g. ``uv add`` into the workspace - that created it). Left as ``None``, the template's ranges are kept verbatim. - See GH #195. + When ``framework_version`` is a concrete version, the template's + forward-looking ``simple_module_*`` ranges are rewritten to an exact pin so + the module resolves against that framework version (e.g. ``uv add`` into the + workspace that created it). Left as ``None`` (or the ``"*"`` sentinel), the + template's ranges are kept verbatim. See GH #195. """ dest = Path(dest) existed_before = dest.exists() @@ -281,7 +249,7 @@ def create_module( }, path_rewrites={_PACKAGE_PATH_TOKEN: package_name}, ) - if framework_version is not None: + if _should_pin_framework_version(framework_version): pin_framework_deps(dest / "pyproject.toml", framework_version) except Exception: # Rollback so a half-scaffolded directory doesn't leave the user diff --git a/framework/cli/tests/test_module_pages_manifest.py b/framework/cli/tests/test_module_pages_manifest.py new file mode 100644 index 000000000..23079665b --- /dev/null +++ b/framework/cli/tests/test_module_pages_manifest.py @@ -0,0 +1,70 @@ +"""Tests for the module-pages manifest emitted by `smpy gen-pages`.""" + +from __future__ import annotations + +import re + + +class TestModulePagesManifest: + async def test_compute_returns_existing_page_dirs(self): + """Returns {ModuleName: Path} for installed modules that ship a pages/ dir.""" + from simple_module_core import discover_modules + from simple_module_hosting.manifest import compute_module_pages + + modules = discover_modules() + result = compute_module_pages(modules) + + # Dashboard ships pages/; Auth is API-only (no frontend pages). + assert "Dashboard" in result + assert "Auth" not in result + for name, path in result.items(): + assert path.is_dir(), f"{name} -> {path} should exist" + assert path.name == "pages" + + async def test_compute_skips_modules_without_pages_dir(self, tmp_path, monkeypatch): + """A module whose package has no pages/ dir is omitted (not an error).""" + from simple_module_core import ModuleBase, ModuleMeta + from simple_module_hosting.manifest import compute_module_pages + + class HeadlessMod(ModuleBase): + meta = ModuleMeta(name="Headless") + + result = compute_module_pages([HeadlessMod()]) + assert "Headless" not in result + + async def test_write_manifest_emits_json_and_ts(self, tmp_path): + """write_module_pages_manifest emits the JSON manifest, TS glob, and Tailwind CSS files.""" + import json + + from simple_module_core import discover_modules + from simple_module_hosting.manifest import write_module_pages_manifest + + modules = discover_modules() + written = write_module_pages_manifest(modules, tmp_path) + + manifest = tmp_path / "modules.manifest.json" + generated = tmp_path / "modules.generated.ts" + css = tmp_path / "modules.generated.css" + assert manifest.is_file() + assert generated.is_file() + assert css.is_file() + assert written == {"manifest": manifest, "generated": generated, "css": css} + + data = json.loads(manifest.read_text(encoding="utf-8")) + assert "Dashboard" in data + assert data["Dashboard"].endswith("pages") or data["Dashboard"].endswith("pages/") + + ts = generated.read_text(encoding="utf-8") + assert "import.meta.glob" in ts + assert "Dashboard" in ts + assert "AUTO-GENERATED" in ts or "auto-generated" in ts.lower() + # Glob patterns must be relative to output_dir — Vite treats + # leading-slash paths as project-root-relative and silently matches + # nothing for FS-absolute paths. + for match in re.findall(r'import\.meta\.glob\("([^"]+)"\)', ts): + assert match.startswith(("./", "../")), ( + f"glob pattern {match!r} must be relative, not absolute" + ) + + css_text = css.read_text(encoding="utf-8") + assert "AUTO-GENERATED" in css_text or "auto-generated" in css_text.lower() diff --git a/framework/cli/tests/test_scaffolding_host.py b/framework/cli/tests/test_scaffolding_host.py index b2e4c3da6..edad462dd 100644 --- a/framework/cli/tests/test_scaffolding_host.py +++ b/framework/cli/tests/test_scaffolding_host.py @@ -1,77 +1,13 @@ -"""Tests for the module-pages manifest and `smpy create-host` scaffolding.""" +"""Tests for `smpy create-host` scaffolding. -from __future__ import annotations +Module-pages manifest tests live in ``test_module_pages_manifest.py``. +""" -import re +from __future__ import annotations import pytest -class TestModulePagesManifest: - async def test_compute_returns_existing_page_dirs(self): - """Returns {ModuleName: Path} for installed modules that ship a pages/ dir.""" - from simple_module_core import discover_modules - from simple_module_hosting.manifest import compute_module_pages - - modules = discover_modules() - result = compute_module_pages(modules) - - # Dashboard ships pages/; Auth is API-only (no frontend pages). - assert "Dashboard" in result - assert "Auth" not in result - for name, path in result.items(): - assert path.is_dir(), f"{name} -> {path} should exist" - assert path.name == "pages" - - async def test_compute_skips_modules_without_pages_dir(self, tmp_path, monkeypatch): - """A module whose package has no pages/ dir is omitted (not an error).""" - from simple_module_core import ModuleBase, ModuleMeta - from simple_module_hosting.manifest import compute_module_pages - - class HeadlessMod(ModuleBase): - meta = ModuleMeta(name="Headless") - - result = compute_module_pages([HeadlessMod()]) - assert "Headless" not in result - - async def test_write_manifest_emits_json_and_ts(self, tmp_path): - """write_module_pages_manifest emits the JSON manifest, TS glob, and Tailwind CSS files.""" - import json - - from simple_module_core import discover_modules - from simple_module_hosting.manifest import write_module_pages_manifest - - modules = discover_modules() - written = write_module_pages_manifest(modules, tmp_path) - - manifest = tmp_path / "modules.manifest.json" - generated = tmp_path / "modules.generated.ts" - css = tmp_path / "modules.generated.css" - assert manifest.is_file() - assert generated.is_file() - assert css.is_file() - assert written == {"manifest": manifest, "generated": generated, "css": css} - - data = json.loads(manifest.read_text(encoding="utf-8")) - assert "Dashboard" in data - assert data["Dashboard"].endswith("pages") or data["Dashboard"].endswith("pages/") - - ts = generated.read_text(encoding="utf-8") - assert "import.meta.glob" in ts - assert "Dashboard" in ts - assert "AUTO-GENERATED" in ts or "auto-generated" in ts.lower() - # Glob patterns must be relative to output_dir — Vite treats - # leading-slash paths as project-root-relative and silently matches - # nothing for FS-absolute paths. - for match in re.findall(r'import\.meta\.glob\("([^"]+)"\)', ts): - assert match.startswith(("./", "../")), ( - f"glob pattern {match!r} must be relative, not absolute" - ) - - css_text = css.read_text(encoding="utf-8") - assert "AUTO-GENERATED" in css_text or "auto-generated" in css_text.lower() - - class TestCreateHost: async def test_creates_expected_backend_files(self, tmp_path): """create_host writes the full backend + frontend scaffold.""" @@ -144,6 +80,65 @@ async def test_refuses_existing_non_empty_dir(self, tmp_path): with pytest.raises(FileExistsError): create_host(dest, name="demo", modules=[]) + async def test_pins_framework_deps_when_version_given(self, tmp_path): + """Regression #206: create_host(..., framework_version=X) rewrites the + template's >=1.0,<2.0 framework ranges (and the >=0.1,<1.0 selected-module + ranges) to ==X so the generated host's first `uv sync` resolves against the + lockstep-published framework version — pre-1.0 dists satisfy neither range.""" + from simple_module_cli.scaffolding import create_host + + dest = tmp_path / "demo" + create_host(dest, name="demo", modules=["Dashboard"], framework_version="0.0.18") + pyproject = (dest / "pyproject.toml").read_text(encoding="utf-8") + + for pkg in ( + "simple_module_core", + "simple_module_db", + "simple_module_hosting", + "simple_module_settings", + ): + assert f"{pkg}==0.0.18" in pyproject + assert f"{pkg}>=1.0,<2.0" not in pyproject + # Selected module deps are pinned to the same lockstep version too — + # the template's >=0.1,<1.0 range is itself unsatisfiable at 0.0.x. + assert "simple_module_dashboard==0.0.18" in pyproject + assert "simple_module_dashboard>=0.1,<1.0" not in pyproject + # Non-framework deps are left untouched. + assert "uvicorn[standard]>=0.34" in pyproject + + async def test_keeps_ranges_without_version(self, tmp_path): + """Without framework_version the template ranges are kept verbatim, so + direct-library callers that don't want an exact pin (and the existing + scaffolding tests) are unaffected.""" + from simple_module_cli.scaffolding import create_host + + dest = tmp_path / "demo" + create_host(dest, name="demo", modules=["Dashboard"]) + pyproject = (dest / "pyproject.toml").read_text(encoding="utf-8") + assert "simple_module_core>=1.0,<2.0" in pyproject + assert "simple_module_dashboard>=0.1,<1.0" in pyproject + + async def test_cli_create_host_pins_to_framework_version(self, tmp_path): + """The `smpy create-host` command pins framework + module deps so the + generated host's first `uv sync` resolves against the lockstep release + that scaffolded it (#206).""" + from simple_module_cli.cli import app + from simple_module_cli.scaffolding import resolve_framework_version + from typer.testing import CliRunner + + runner = CliRunner() + dest = tmp_path / "out" + result = runner.invoke( + app, ["create-host", "demo", "--dest", str(dest), "--with", "Dashboard"] + ) + assert result.exit_code == 0, result.output + + pyproject = (dest / "pyproject.toml").read_text(encoding="utf-8") + version = resolve_framework_version() + assert f"simple_module_core=={version}" in pyproject + assert "simple_module_core>=1.0,<2.0" not in pyproject + assert f"simple_module_dashboard=={version}" in pyproject + async def test_env_py_uses_shared_helper(self, tmp_path): """Scaffolded migrations/env.py delegates to the shared helper, not inline logic.""" from simple_module_cli.scaffolding import create_host diff --git a/framework/cli/tests/test_scaffolding_module.py b/framework/cli/tests/test_scaffolding_module.py index c17651385..445385858 100644 --- a/framework/cli/tests/test_scaffolding_module.py +++ b/framework/cli/tests/test_scaffolding_module.py @@ -118,6 +118,18 @@ async def test_create_module_keeps_ranges_without_version(self, tmp_path): pyproject = (dest / "pyproject.toml").read_text(encoding="utf-8") assert "simple_module_core>=1.0,<2.0" in pyproject + async def test_create_module_treats_star_as_skip_not_a_pin(self, tmp_path): + """The ``"*"`` sentinel keeps the template ranges (same as ``None``) — it + must NOT render the invalid PEP 508 specifier ``==*``. Guards the + harmonized pin rule shared with create_host.""" + from simple_module_cli.scaffolding import create_module + + dest = tmp_path / "simple-module-orders" + create_module(dest, name="Orders", framework_version="*") + pyproject = (dest / "pyproject.toml").read_text(encoding="utf-8") + assert "simple_module_core>=1.0,<2.0" in pyproject + assert "==*" not in pyproject + async def test_cli_create_module_pins_to_framework_version(self, tmp_path): """The `smpy create-module` command pins framework deps so `uv add` resolves into the app that created it (#195).""" diff --git a/framework/testing/README.md b/framework/testing/README.md index 90c0b6d23..9425f5a8e 100644 --- a/framework/testing/README.md +++ b/framework/testing/README.md @@ -14,11 +14,15 @@ pip install "simple_module_hosting[dev]" ## What it provides +All fixtures below are auto-registered via the `pytest11` entry point — installing the package is enough. + +- `fake_event_bus` — a fresh recording `EventBus` per test that captures every `publish`/`publish_nowait` call. +- `build_test_app` — callable returning a minimal FastAPI app that loads exactly one module. - `settings` — a ready-to-use `Settings` instance with an in-memory SQLite database and multi-tenancy enabled. - `db_state`, `engine`, `db_session` — fresh `DatabaseState` per test; `db_session` also creates all module tables and stamps `alembic_version` at head so the boot-time migration check passes. - `app` — a `create_app(settings)` instance with `lifespan` started and stopped. - `client` — an `httpx.AsyncClient` bound to the test app. -- `authenticated_client` — same but with an admin user seeded and a forged session cookie attached. +- `authenticated_client` — same but with an admin user seeded and a forged session cookie attached. **Requires the `users` module** to be installed (it seeds the admin via `users.bootstrap`); apps scaffolded by `smpy` include it. ## Usage diff --git a/framework/testing/simple_module_test/__init__.py b/framework/testing/simple_module_test/__init__.py index 6535bc993..a658960c5 100644 --- a/framework/testing/simple_module_test/__init__.py +++ b/framework/testing/simple_module_test/__init__.py @@ -12,9 +12,12 @@ * :func:`build_test_app` — return a minimal FastAPI app loading exactly one module, with an in-memory SQLite DB. -The corresponding pytest fixtures are ``fake_event_bus``, ``test_app``, -``test_client``, and ``test_db_session``. See ``simple_module_test.plugin`` -for their definitions. +The pytest plugin auto-registers these fixtures (no imports needed): +``fake_event_bus``, ``build_test_app``, ``settings``, ``db_state``, +``engine``, ``db_session``, ``app``, ``client``, and ``authenticated_client``. +See ``simple_module_test.plugin`` (and ``simple_module_test.fixtures``) for +their definitions. ``authenticated_client`` additionally requires the ``users`` +module to be installed — it seeds an admin via ``users.bootstrap``. """ from simple_module_test.app_factory import build_test_app diff --git a/framework/testing/simple_module_test/fixtures.py b/framework/testing/simple_module_test/fixtures.py new file mode 100644 index 000000000..c827adc98 --- /dev/null +++ b/framework/testing/simple_module_test/fixtures.py @@ -0,0 +1,197 @@ +"""Headline app/db/client fixtures shipped by the pytest plugin. + +These are the fixtures the README advertises — ``settings``, ``db_state``, +``engine``, ``db_session``, ``app``, ``client``, ``authenticated_client`` — +made available to any test run in an environment that installs +``simple_module_test`` (the ``pytest11`` entry point re-exports them from +:mod:`simple_module_test.plugin`). They build a real ``create_app`` against an +in-memory SQLite DB with every installed module's tables created and the +``alembic_version`` row stamped at head, so the boot-time migration check +passes. + +Module-scope imports here reference only framework packages +(``simple_module_core`` / ``simple_module_db`` / ``simple_module_hosting``), +which are this package's declared dependencies. The one plugin-module coupling +— ``authenticated_client`` seeding an admin via ``users.bootstrap`` — is a lazy +import inside the fixture body, so importing this module never requires the +``users`` module to be installed (mirroring ``plugin._bootstrap_eager_celery``, +which soft-imports ``background_tasks``). +""" + +from __future__ import annotations + +import contextlib +import importlib +from collections.abc import AsyncGenerator +from functools import lru_cache + +import httpx +import pytest +from simple_module_core.discovery import discover_modules +from simple_module_db.base import all_module_bases +from simple_module_db.session import DatabaseState, init_db +from simple_module_hosting.settings import Settings +from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession + +from simple_module_test.session_cookie import forge_session_cookie + + +@pytest.fixture +def settings() -> Settings: + """Settings configured for testing with in-memory SQLite. + + Multi-tenancy stays on so the existing ``TenantMiddleware`` tests + (and the ``X-Tenant-ID`` header paths they rely on) keep working. + Individual tests that want the tenant middleware absent construct + their own ``Settings(multi_tenant=False, ...)`` in the test body. + """ + return Settings( + database_url="sqlite+aiosqlite:///:memory:", + environment="testing", + secret_key="test-secret-key", + multi_tenant=True, + tenant_header="X-Tenant-ID", + ) + + +@pytest.fixture +async def db_state() -> AsyncGenerator[DatabaseState, None]: + """Create a fresh in-memory DatabaseState with listeners registered.""" + from simple_module_db.listeners import register_listeners + + state = init_db("sqlite+aiosqlite:///:memory:") + register_listeners(state) + yield state + await state.engine.dispose() + + +@pytest.fixture +async def engine(db_state: DatabaseState) -> AsyncEngine: + """Return the engine from the test DatabaseState.""" + return db_state.engine + + +@lru_cache(maxsize=1) +def _ensure_models_imported() -> list: + """Import all module models so all_module_bases is populated (cached).""" + for mod in discover_modules(): + pkg = type(mod).__module__.split(".")[0] + with contextlib.suppress(ModuleNotFoundError): + importlib.import_module(f"{pkg}.models") + return list(all_module_bases) + + +@lru_cache(maxsize=1) +def _alembic_head() -> str | None: + """Cached head revision — cannot change within a pytest run.""" + from simple_module_hosting.migrations import resolve_head_revision + + return resolve_head_revision() + + +async def _create_all_tables(engine) -> None: + """Create all module tables in a single connection. + + Also stamps the alembic_version table at head so the app's startup + migration check (``check_migrations``) treats the test DB as current. + Without the stamp the check would raise because ``create_all`` doesn't + touch alembic_version. + """ + from sqlalchemy import text + + bases = _ensure_models_imported() + head = _alembic_head() + + async with engine.begin() as conn: + + def _sync_create_all(sync_conn): + for base in bases: + base.metadata.create_all(sync_conn) + + await conn.run_sync(_sync_create_all) + + if head: + await conn.execute( + text( + "CREATE TABLE IF NOT EXISTS alembic_version " + "(version_num VARCHAR(32) NOT NULL PRIMARY KEY)" + ) + ) + await conn.execute(text("DELETE FROM alembic_version")) + await conn.execute( + text("INSERT INTO alembic_version (version_num) VALUES (:v)"), + {"v": head}, + ) + + +@pytest.fixture +async def db_session(db_state: DatabaseState) -> AsyncGenerator[AsyncSession, None]: + """Yield an async session backed by in-memory SQLite.""" + await _create_all_tables(db_state.engine) + + async with db_state.session_factory() as session: + yield session + + +@pytest.fixture +async def app(settings: Settings): + """Create a FastAPI app with tables pre-created and lifespan triggered.""" + from simple_module_hosting.app_builder import create_app + + application = create_app(settings) + + await _create_all_tables(application.state.sm.db.engine) + + # Trigger lifespan startup so app.state.migration is populated + ctx = application.router.lifespan_context(application) + await ctx.__aenter__() + + yield application + + # Lifespan shutdown disposes the engine + await ctx.__aexit__(None, None, None) + + +@pytest.fixture +async def client(app) -> AsyncGenerator[httpx.AsyncClient, None]: + """Unauthenticated async HTTP client.""" + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + ) as c: + yield c + + +@pytest.fixture +async def authenticated_client(app) -> AsyncGenerator[httpx.AsyncClient, None]: + """HTTPX client with a signed session cookie carrying a seeded admin user's id. + + Requires the ``users`` module to be installed — it seeds the admin via + ``users.bootstrap.create_admin``. The import is deferred to here (not module + scope) so this plugin imports cleanly without ``users``; a consumer app that + uses this fixture without ``users`` gets a clear ImportError naming it. + """ + from users.bootstrap import create_admin + + async with app.state.sm.db.session_factory() as session: + result = await create_admin( + session, + email="admin@test", + password="test-password", + full_name="Test Admin", + ) + user_id = str(result.user.id) + + signed = forge_session_cookie( + app.state.sm.settings.secret_key, + {"user_id": user_id}, + ) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + cookies={"session": signed}, + ) as c: + yield c diff --git a/framework/testing/simple_module_test/plugin.py b/framework/testing/simple_module_test/plugin.py index 8276909f3..85a9328f9 100644 --- a/framework/testing/simple_module_test/plugin.py +++ b/framework/testing/simple_module_test/plugin.py @@ -15,6 +15,21 @@ from simple_module_test.app_factory import build_test_app as _build_test_app from simple_module_test.fake_events import FakeEventBus +# Re-export the headline app/db/client fixtures the README advertises. pytest +# collects fixture-decorated attributes of the plugin module, so importing them +# here registers them for every test run that installs simple_module_test — +# without a per-consumer conftest. Treat this surface like a public API: deleting +# one breaks external modules' suites. See GH #200. +from simple_module_test.fixtures import ( # noqa: F401 + app, + authenticated_client, + client, + db_session, + db_state, + engine, + settings, +) + def _bootstrap_eager_celery() -> None: """Register a process-wide eager Celery app before any test runs. diff --git a/framework/testing/tests/test_plugin_fixtures.py b/framework/testing/tests/test_plugin_fixtures.py new file mode 100644 index 000000000..91d001f23 --- /dev/null +++ b/framework/testing/tests/test_plugin_fixtures.py @@ -0,0 +1,62 @@ +"""GH #200: the published ``simple_module_test`` plugin must register the +headline fixtures its README/``__init__`` advertise — not just ``fake_event_bus`` ++ ``build_test_app``. Before the fix those fixtures lived only in the framework +repo's root ``conftest.py``, so a consumer that installed the plugin (but wasn't +the framework repo) got "fixture not found". +""" + +from __future__ import annotations + +# The fixtures the plugin promises. Treat this set as the public fixture +# surface — dropping one breaks external modules' suites. +_HEADLINE_FIXTURES = ( + "fake_event_bus", + "build_test_app", + "settings", + "db_state", + "engine", + "db_session", + "app", + "client", + "authenticated_client", +) + + +def _is_pytest_fixture(obj) -> bool: + """True if ``obj`` was produced by ``@pytest.fixture`` (version-robust). + + pytest >=8.4 returns a ``FixtureFunctionDefinition`` instance; older + versions left the function in place with a ``_pytestfixturefunction`` + marker attribute. Cover both so this test doesn't pin a pytest version. + """ + return hasattr(obj, "_pytestfixturefunction") or ( + type(obj).__name__ == "FixtureFunctionDefinition" + ) + + +def test_plugin_module_exports_every_headline_fixture(): + """Each advertised fixture is a real ``@pytest.fixture`` on the plugin module.""" + import simple_module_test.plugin as plugin + + for name in _HEADLINE_FIXTURES: + obj = getattr(plugin, name, None) + assert obj is not None, f"{name!r} is not exported by simple_module_test.plugin" + # A plain function imported by mistake would not satisfy this. + assert _is_pytest_fixture(obj), f"{name!r} is not a pytest fixture" + + +def test_plugin_is_registered_via_pytest11_entry_point(pytestconfig): + """pytest discovered the plugin through its ``pytest11`` entry point, so a + consumer gets the fixtures by installing the package alone.""" + assert pytestconfig.pluginmanager.hasplugin("simple_module_test") + + +async def test_db_session_fixture_injects_without_a_local_conftest(db_session): + """``db_session`` resolves in this test dir, which has no conftest defining + it — it can only come from the installed plugin (root conftest no longer + provides it). Proves the moved fixture is genuinely shipped, not just + re-exported on paper.""" + from sqlalchemy import text + + result = await db_session.execute(text("SELECT 1")) + assert result.scalar_one() == 1 diff --git a/modules/background_tasks/background_tasks/celery_app.py b/modules/background_tasks/background_tasks/celery_app.py index bc15b1dec..272fc474d 100644 --- a/modules/background_tasks/background_tasks/celery_app.py +++ b/modules/background_tasks/background_tasks/celery_app.py @@ -15,6 +15,7 @@ from __future__ import annotations +import importlib import logging from importlib.metadata import entry_points @@ -47,10 +48,71 @@ def _discover_task_packages() -> list[str]: return sorted(packages) +def _collect_module_beat_schedules(packages: list[str]) -> dict: + """Merge every installed module's optional ``tasks.BEAT_SCHEDULE`` mapping. + + A module registers periodic work the same way it registers tasks — by + shipping a ``tasks.py`` — and additionally exporting a module-level + ``BEAT_SCHEDULE`` dict of ``{entry_name: celery beat entry}``. Because + ``build_celery`` runs identically in the web and worker processes, this is + worker-safe and deterministic, unlike runtime registration via + ``add_periodic_task`` / the ``on_after_finalize`` signal (and unlike the + commonly-cited ``on_after_configure``, which never fires here — see GH #199). + + Importing ``.tasks`` is idempotent (autodiscover imports the same + modules), so doing it eagerly here has no extra side effects. + """ + merged: dict = {} + for pkg in packages: + try: + tasks_mod = importlib.import_module(f"{pkg}.tasks") + except ModuleNotFoundError as exc: + # The module simply ships no tasks.py — skip. But a tasks.py that + # *exists* and fails to import (its own dependency is missing) must + # surface loudly, not be mistaken for "no tasks.py" and silently + # drop the module's periodic work. + if exc.name in (pkg, f"{pkg}.tasks"): + continue + raise + schedule_dict = getattr(tasks_mod, "BEAT_SCHEDULE", None) + if not isinstance(schedule_dict, dict): + continue + for name, entry in schedule_dict.items(): + if name in merged: + logger.warning( + "Beat entry %r from %s.tasks overrides an earlier module's entry", name, pkg + ) + merged[name] = entry + return merged + + def build_celery(settings: BackgroundTasksSettings) -> Celery: """Construct a Celery app wired to the project's Redis broker.""" celery = Celery(MODULE_NAME) + packages = _discover_task_packages() + + # The two internal entries always ship; modules contribute more via their + # own ``tasks.BEAT_SCHEDULE``. ``setdefault`` keeps the built-ins authoritative + # if a module reuses one of their entry names. See GH #199. + beat_schedule = { + "background-tasks-sweep-stuck": { + "task": INTERNAL_TASK_SWEEP_STUCK, + "schedule": schedule(settings.stuck_sweep_interval_seconds), + }, + "background-tasks-purge-old": { + "task": INTERNAL_TASK_PURGE_OLD, + "schedule": schedule(settings.purge_interval_seconds), + }, + } + for name, entry in _collect_module_beat_schedules(packages).items(): + if name in beat_schedule: + logger.warning( + "Module beat entry %r clashes with a built-in entry; the built-in wins", name + ) + continue + beat_schedule[name] = entry + celery.conf.update( broker_url=settings.broker_url, result_backend=settings.result_backend, @@ -74,22 +136,12 @@ def build_celery(settings: BackgroundTasksSettings) -> Celery: timezone="UTC", enable_utc=True, broker_connection_retry_on_startup=True, - beat_schedule={ - "background-tasks-sweep-stuck": { - "task": INTERNAL_TASK_SWEEP_STUCK, - "schedule": schedule(settings.stuck_sweep_interval_seconds), - }, - "background-tasks-purge-old": { - "task": INTERNAL_TASK_PURGE_OLD, - "schedule": schedule(settings.purge_interval_seconds), - }, - }, + beat_schedule=beat_schedule, beat_scheduler="celery.beat:PersistentScheduler", ) # Autodiscover across every installed simple_module — a module just ships # a `tasks.py` and the worker picks it up. - packages = _discover_task_packages() logger.info("Celery autodiscover_tasks across: %s", packages) celery.autodiscover_tasks(packages, related_name="tasks", force=True) diff --git a/modules/background_tasks/tests/test_beat_schedule.py b/modules/background_tasks/tests/test_beat_schedule.py new file mode 100644 index 000000000..bb3d29921 --- /dev/null +++ b/modules/background_tasks/tests/test_beat_schedule.py @@ -0,0 +1,105 @@ +"""GH #199: a consumer module can register Celery beat entries by shipping a +``tasks.py`` that exports a module-level ``BEAT_SCHEDULE`` dict — ``build_celery`` +merges them into the beat schedule, worker-safe and declarative (no reliance on +the broken ``on_after_configure`` signal). +""" + +from __future__ import annotations + +import importlib +import logging +import sys +import types + +import pytest +from background_tasks import celery_app +from background_tasks.constants import INTERNAL_TASK_SWEEP_STUCK +from background_tasks.settings import BackgroundTasksSettings + + +def _install_fake_module(monkeypatch, pkg: str, beat_schedule: dict | None) -> None: + """Register ``pkg`` + ``pkg.tasks`` in sys.modules for the test duration. + + When ``beat_schedule`` is None the tasks module ships no ``BEAT_SCHEDULE`` + attribute (a module that has tasks but no periodic work). + """ + parent = types.ModuleType(pkg) + tasks = types.ModuleType(f"{pkg}.tasks") + if beat_schedule is not None: + tasks.BEAT_SCHEDULE = beat_schedule + monkeypatch.setitem(sys.modules, pkg, parent) + monkeypatch.setitem(sys.modules, f"{pkg}.tasks", tasks) + + +class TestCollectModuleBeatSchedules: + def test_merges_module_beat_schedule(self, monkeypatch): + entry = {"task": "fakepkg.daily", "schedule": 3600} + _install_fake_module(monkeypatch, "fakepkg", {"fakepkg-daily": entry}) + + merged = celery_app._collect_module_beat_schedules(["fakepkg"]) + assert merged == {"fakepkg-daily": entry} + + def test_skips_package_without_beat_schedule_attr(self, monkeypatch): + _install_fake_module(monkeypatch, "nobeat", None) + assert celery_app._collect_module_beat_schedules(["nobeat"]) == {} + + def test_skips_package_without_tasks_module(self): + # A package with no importable ``.tasks`` submodule is ignored, not fatal. + assert celery_app._collect_module_beat_schedules(["definitely_not_a_real_pkg"]) == {} + + def test_ignores_non_dict_beat_schedule(self, monkeypatch): + _install_fake_module(monkeypatch, "weird", None) + sys.modules["weird.tasks"].BEAT_SCHEDULE = ["not", "a", "dict"] + assert celery_app._collect_module_beat_schedules(["weird"]) == {} + + def test_propagates_error_from_a_broken_tasks_module(self, monkeypatch): + """A tasks.py that *exists* but fails its own import (missing dependency) + must surface, not be mistaken for 'module ships no tasks.py' and silently + drop its periodic work.""" + real_import = importlib.import_module + + def fake_import(name, *args, **kwargs): + if name == "brokenpkg.tasks": + # The module's own `import some_dep` fails — note the missing + # name is NOT brokenpkg/brokenpkg.tasks. + raise ModuleNotFoundError("No module named 'some_dep'", name="some_dep") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(importlib, "import_module", fake_import) + with pytest.raises(ModuleNotFoundError, match="some_dep"): + celery_app._collect_module_beat_schedules(["brokenpkg"]) + + +class TestBuildCeleryBeatSchedule: + def test_module_beat_entry_lands_in_schedule(self, monkeypatch): + entry = {"task": "fakeinv.generate_recurring", "schedule": 3600} + _install_fake_module(monkeypatch, "fakeinv", {"fakeinv-gen-daily": entry}) + monkeypatch.setattr( + celery_app, "_discover_task_packages", lambda: ["background_tasks", "fakeinv"] + ) + + app = celery_app.build_celery(BackgroundTasksSettings(task_always_eager=True)) + schedule = app.conf.beat_schedule + + assert schedule["fakeinv-gen-daily"] == entry + # The two built-in entries still ship. + assert "background-tasks-sweep-stuck" in schedule + assert "background-tasks-purge-old" in schedule + + def test_builtin_entries_win_on_name_clash_and_warn(self, monkeypatch, caplog): + # A module that reuses a built-in entry name must not clobber it, and the + # drop must be diagnosable (not silent). + rogue = {"task": "rogue.evil", "schedule": 1} + _install_fake_module(monkeypatch, "rogue", {"background-tasks-sweep-stuck": rogue}) + monkeypatch.setattr( + celery_app, "_discover_task_packages", lambda: ["background_tasks", "rogue"] + ) + + with caplog.at_level(logging.WARNING, logger=celery_app.__name__): + app = celery_app.build_celery(BackgroundTasksSettings(task_always_eager=True)) + + assert ( + app.conf.beat_schedule["background-tasks-sweep-stuck"]["task"] + == INTERNAL_TASK_SWEEP_STUCK + ) + assert any("clashes with a built-in entry" in r.message for r in caplog.records)